janeczku / calibre-web

:books: Web app for browsing, reading and downloading eBooks stored in a Calibre database
GNU General Public License v3.0
12.89k stars 1.38k forks source link

Caliblur has squished covers - fix inside #1314

Open lyonzy opened 4 years ago

lyonzy commented 4 years ago

Describe the bug/problem The CaliBlur theme (which looks great by the way!) squishes the covers of some/most books, e.g.: book cover with changed aspect ratio

To Reproduce Steps to reproduce the behavior:

  1. Load some books - square covers are a good example
  2. Use CaliBlur theme
  3. Observe squashed cover images

Expected behavior Book covers at full width but height appropriate to the aspect ratio: same book cover but square as desired

Screenshots See above.

Environment (please complete the following information):

Fix This CSS fixes it, at least on a desktop view:

.book:not(body) {
  height: 295px;
  display: flex;
  justify-content: end;
  flex-direction: column;
}

.container-fluid .book .cover {
    height: auto;`
}

Basically this sets div.cover to auto height (not 275px) and sets the parent (div.book) to 295px to compensate. It then uses the three flex properties to align div.cover and div.meta to the bottom of div.book - so that books are aligned at the bottom: row of books of various heights, aligned on their bottom edge

This also maintains hover border and shading alignment with the cover: book cover with aligned hover fade and yellow border

OzzieIsaacs commented 4 years ago

@hexeth could you have a look at this one, can we merge the suggestion without any sideffects?

hexeth commented 4 years ago

This is a little more complicated than the above code solves for. @leram84 did the CSS. I know a little, but it's definitely not my wheelhouse.

Not all elements are covered by above code: image

Hover icon is no longer centered: image

lyonzy commented 4 years ago

That's weird, it looks like yours has kept div.book width at 225px - mine is overriden somewhere else at 180px: css screenshot

Maybe @leram84 might know more about how this all works (and what the significance of :not(body) is).

Also I tested it in Chromium and it seems like it should actually be justify-content: flex-end; instead of just end.

hexeth commented 4 years ago

I noticed that removing display: flex made it all line up but I am missing any square book covers to see what it does to them

hexeth commented 4 years ago

Okay, think I am starting to see what's going on. The issue was that I was replacing the CSS with what you put above, but in reality it just needs to be added to the existing spots.

In caliblur.css

.container-fluid .book .cover {
    width: auto;
    display: inline-block;
    height: Auto;
}
.book:not(body) {
    width: 180px!important;
    max-width: 180px!important;
    min-width: 180px!important;
    height: 295px;
    display: flex;
    justify-content: end;
    flex-direction: column;
}

Results in: image

Not sure why yours line up at the bottom and mine do not though.

Edit:

Looks like the above code works in Firefox, but not in Chrome for aligning on the bottom

hexeth commented 4 years ago

It looks like

.book:not(body) {
    width: 180px!important;
    max-width: 180px!important;
    min-width: 180px!important;
    height: 295px;
    display: flex;
    flex-direction: column;
    justify-content: flex-end
}

Solves this in Chrome for me

Knepherbird commented 4 years ago

"end", "start", etc., were originally developed for css grid contexts, and while there is an intention to replace flex-end, flex-start, etc., only FF and Safari have implemented support for them in flexbox context, so it's advised to stick with the flex-specific properties at this time. (See: https://caniuse.com/#feat=mdn-css_properties_justify-content_flex_context_start_end)

hexeth commented 4 years ago

Using flex-endinstead of end does fix the problem in a way, but creates another strange one in Chrome.

Some book cover heights are super squished until hovered over, or until the screen is repositioned. It additionally doesn't play super well with the margins on tall book covers.

squished

Knepherbird commented 4 years ago

I haven't dug through the whole stylesheet, but based on what's been posted here and some tinkering in dev inspector, I think these modified rules appear to provide a solution, including the Chrome issue just above. These are for desktop, of course, so may need modification for responsive breakpoints. As a side note, I'd love to see the bootstrap grid replaced with a modern css grid / flexbox solution. (Edited the last rule to remove unnecessary properties)

#books>.cover>a {
    display: inline-block;
    width: auto;
    height: auto;
}

.book:not(body) {
    width: 180px!important;
    max-width: 180px!important;
    min-width: 180px!important;
    height: 295px;
    display: flex;
    flex-direction: column;
    justify-content: flex-end;
}

.container-fluid .book .cover {
    width: auto;
    display: inline-block;
    align-self: center;
    height: auto;
}

#books .cover img {
    width: auto !important;
    height: auto;
    max-height: 225px;
    max-width: 150px;
}
chrisdma commented 1 year ago

@OzzieIsaacs @hexeth Is there any reason why this hasn't been merged? I am using the Caliblur theme and have been experiencing this issue for quite a while and would love for it to be fixed.

IceflowRE commented 7 months ago

The latest suggested fix is not entirely working, it is still overlapping other elements when having a long series name

IceflowRE commented 7 months ago

This is a quick fix:

.cover > a > span > img {
    object-fit: contain;
}

This wont scale the hover effect, but i like having a bigger hover effect anyway, makes it easier to click the covers.