getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.27k stars 167 forks source link

Panel loads huge images in cards layout #2488

Closed manuelmoreale closed 2 years ago

manuelmoreale commented 4 years ago

Describe the bug
Currently, when using a card layout in the panel, even though there's both an srcset and a sizes attribute set, the browser is still loading the biggest possible crop rather than one at a smaller size.

This was supposed to be fixed in #1807 but the bug is still there.

Unless my understanding of sizes and srcset is completely wrong, the problem with the current implementation is mostly a matter of css/html.

Right now, there's 3 crops sizes and 3 sizes set up

srcset="img-352x.jpg 352w, img-864x.jpg 864w, img-1408x.jpg 1408w" sizes="(min-width: 30em) and (max-width: 65em) 59em, (min-width: 65em) 88em, 27em"

The problem with this is that the current sizes attribute is telling the browser that when the browser window is at least 65em wide (so 1040px) the space available for the image is 88em.

Again, unless I got this completely wrong, sizes defines how big the image in the browser actually is displayed and with that information the browser then selects the appropriate image from the ones available inside srcset.

To Reproduce
Steps to reproduce the behavior:

  1. Create a Pages section
  2. Set layout to cards and size to small or tiny
  3. Open dev tools and check which image has been loaded by the browser

Screenshots
75891226-c16f6b80-5e2f-11ea-8e33-07fd72c0566d

distantnative commented 4 years ago

sizes defines a set of media conditions (e.g. screen widths) and indicates what image size would be best to choose, when certain media conditions are true — these are the hints we talked about earlier. In this case, before each comma we write:

A media condition ((max-width:600px)) — you'll learn more about these in the CSS topic, but for now let's just say that a media condition describes a possible state that the screen can be in. In this case, we are saying "when the viewport width is 600 pixels or less". A space The width of the slot the image will fill when the media condition is true (480px)

Our main problem here is, that the srcset and sizes are loaded from the backend without awareness where they are placed. So the screen width isn't really a helpful fact to determine the images size, as long as we don't know if it is in a 1/4 column or 1/1 or 3/4 but with a nested 1/3. Nesting columns (or field width) makes this hard.

Right now, I am wondering whether the backend should provide 3-4 abstract sensible sizes for srcset. And then we might be able to work on the Vue image component with a computed prop that actually reads the element width and creates the sizes attribute. Not sure about it.

I think we had it covered when we had huuuuuuuge attributes covering pretty much all column/size configurations. But people got mad cause it generated 12= thumbs per image.

moritzebeling commented 3 years ago

Is this still open? As of v3.5.4 files sections seem to have srcsets in both list and cards layout...

manuelmoreale commented 3 years ago

@moritzebeling it Is. in 3.5.4 Kirby is still loading images that are way too big. That is not for a lack of srcset but because of the way srcset is configured.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. This is for us to prioritize issues that are still relevant to our community. It will be closed if no further activity occurs within the next 14 days. If this issue is still relevant to you, please leave a comment.