Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task
devguerreiro opened this issue Apr 30, 2024 · 5 comments
Labels
- P2: has workaround Bug, but has workaround (priority) feat: assets Related to the Assets feature (scope)

Comments

@devguerreiro
Copy link

devguerreiro commented Apr 30, 2024

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

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Apr 30, 2024
@Princesseuh
Copy link
Member

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)

@Princesseuh Princesseuh added needs response Issue needs response from OP and removed needs triage Issue needs to be triaged labels Apr 30, 2024
@devguerreiro
Copy link
Author

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
Copy link
Member

Princesseuh commented May 1, 2024

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
Copy link
Author

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
Copy link
Member

Princesseuh commented May 2, 2024

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.

@Princesseuh Princesseuh added feat: assets Related to the Assets feature (scope) - P2: has workaround Bug, but has workaround (priority) and removed needs response Issue needs response from OP labels May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: has workaround Bug, but has workaround (priority) feat: assets Related to the Assets feature (scope)
Projects
None yet
Development

No branches or pull requests

2 participants