withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
45.91k stars 2.42k forks source link

The image size isn't changing when viewport changes (Image/Picture built-in component) #10920

Closed devguerreiro closed 2 weeks ago

devguerreiro commented 5 months ago

Astro Info

Astro                    v4.7.0
Node                     v20.11.0
System                   Linux (x64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             @astrojs/tailwind

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Both Image and Picture built-in components are generating the img tag with static width and height, which unables the browser to render the intrinsic size of the image when using "widths" and "sizes" properties

image

What's the expected result?

The rendered image size should be the same as the intrinsic size and should switch sizes as normally does when viewport's width changes

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-ote2fp-dm1slu?file=src%2Fpages%2Findex.astro

Participation

Princesseuh commented 5 months ago

How do you believe this should be instead? The current markup is in line with the recommendation regarding avoiding CLS on responsive images, ex https://web.dev/articles/optimize-cls#responsive-images

Other sources I'm finding also mention adding the dimensions even when using sizes and srcset for images that all have the same aspect ratio (which is always the case in our Picture component)

devguerreiro commented 5 months ago

I do believe there is no need to set width and height when working with srcset, as you can see here https://developer.mozilla.org/en-US/docs/Learn/HTML/Multimedia_and_embedding/Responsive_images#resolution_switching_different_sizes

When I removed those attributes through devtools > inspect, it worked as expected.

Princesseuh commented 5 months ago

I do believe there is no need to set width and height when working with srcset, as you can see here https://developer.mozilla.org/en-US/docs/Learn/HTML/Multimedia_and_embedding/Responsive_images#resolution_switching_different_sizes

When I removed those attributes through devtools > inspect, it worked as expected.

The example in that section has CLS. Ideally we have a solution without CLS, which width and height seems to be in the article I linked. Maybe just using aspect-ratio would work? In which case, you may be able to make it work like you'd expect using CSS

devguerreiro commented 5 months ago

The point of this issue is that the Image and Picture built-in component does not work as expected, even following the example on the docs -> https://docs.astro.build/en/guides/images/#widths

The rendered image does not changes its size when viewport changes. I fixed this removing both width and height attributes.

I still believe that when working with "srcset" and "sizes" there is no need to provide those attributes (width/height).

Princesseuh commented 5 months ago

I understand that, but I don't know how to fix it in a way that doesn't create CLS. I can't just remove the attributes and let every Astro website lose 20 points on Lighthouse every time they use an image with multiple sizes.

My suggestion right now would be to use CSS to do this, using width: auto; height: auto; on the image.

matthewp commented 3 months ago

@ascorbic Do you think this is fixable?

ascorbic commented 3 months ago

The way this is handled with Unpic is to use CSS, which is the only way to do this without CLS. The problem about handling this automatically is that there are different behaviours that are needed for different situations. Unpic has three different layout types, for example. If we were to handle it, we'd need to either be opinionated or offer different options, at which point we're basically recreating Unpic. The Unpic inline style logic is here