Update Picture.astro so it no longer passes height and width attributes

[smeevil]

Changes

The image variable of getPicture contains a width and height property, which we usually require. In this case, the image is wrapped in a picture tag, and the img tag itself should not have a width and height property as this will break the responsiveness of the image provided by the picture tag.

Testing

The picture component now renders the img tag without the width and height attributes.
I'm not sure how you would like to adjust the test files and if this is the correct way you would like to solve this issue.

Docs

This has no impact on the docs as everything stated about the element is correct.

[changeset-bot]

🦋 Changeset detected

Latest commit: 1db96b1

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Princesseuh]

Won't not having a width and height on the image cause a layout shift when the image loads in?

[smeevil]

You are correct; this will cause a layout shift.

I have no idea how to prevent this if you want to use the picture tag (or img tag, for that matter) for selecting responsive image sizes. When keeping the width/height tags, you could only replace the image with better quality, like a 2x for retina screens.

As far as I know, the only way to prevent the layout shift would be by using js to set the correct width/height for the current breakpoint (which we are trying to avoid)...

One of the most detailed articles I could find is https://stackoverflow.blog/2022/03/28/picture-perfect-images-with-the-modern-element/ which sadly does not handle the layout shift issue when not providing a width/height.

Quite an exciting conundrum...

[tony-sull]

Yeah that's definitely unfortunate...I can't find another example of an <img> inside a <picture> including the dimensions on it so we should really be stripping those off as well

I hadn't noticed this one in our use of <Picture> in astro.build, but mainly because it's always inside a flex or grid container that's handling the reponsive layout

---

I just merged in main to see if tests pass, this should be able to go out with the next release that adds WASM support

[tony-sull]

This can cause layout shift but it's still manageable with aspect-ratio (sorry Safari...). For any projects using tailwind, the aspect ratio plugin also does the trick

<Picture src={import('./hero.png') width={800} height={400} />

<style>
picture :global(img) {
  @apply aspect-[2/1];
}
</style>

[bronze]

Google is giving me warnings now that the img doesnt have a size anymore on https://web.dev/measure/.
Image elements do not have explicit width and height

Anyone else experiencing this?

[panwauu]

@bronze You are warned about the possible layout shifts when using the img without width and height, which is already discussed above. Currently, it is not possible to optimize for responsiveness and cls at the same time.

[carlcs]

I wonder what “responsiveness” issues you were running into. @smeevil

It’s currently not possible to do art direction with the Picture component. The only reason the component outputs multiple <source> elements is to serve the image in different formats (avif, webp). You would somehow have to set media queries on individual sources to do art direction, probably also want to pass multiple source images. The “Responsive pictures” section in docs is a bit misleading in this regard.

You shouldn’t use srcset to output an image in different aspect ratios (see Eric Portis: Srcset and sizes). But I don‘t think that’s even possible with the Astro Image components.

All images variants the components serve will have the same aspect ratio, the one you request via the aspect-ratio, width and height props, and therefore it should be totally fine to output width and height attributes on the <img> element.

Once art direction is possible the attributes should be output with the corresponding <source> elements. Browser support for this is already pretty good.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/source
https://caniuse.com/mdn-api_htmlsourceelement_width

[carlcs]

@tony-sull could you please consider reverting this change?

[philschonholzer]

@tony-sull This is a breaking change for our page. Img needs the hight and width for fix image sizes. 90% of our images are fixed size and not responsive (but different resolutions, for different devices). Please revert this.

See also Smashing Magazine wrote about this: https://www.smashingmagazine.com/2020/03/setting-height-width-images-important-again/

[tony-sull]

I'm open to reverting this one, gotta say I'm at a bit of a loss for solid specs on whether width/height should be included in a picture's <img>. Looking at the MDN docs for art direction, none of the examples include dimensions for the image 🤔

We had to make a couple fixes on astro.build when this update was released, so I definitely know the pain being hit here!

I'm wondering if the content-visibility: auto style added to the <picture> is part of the issue? I'll need to do some digging, but the fix here could be to add back width/height and remove the content-visibility style

[carlcs]

When doing art direction you’d want to add the width and height attributes to each individual <source> as the aspect ratio might be different for sources targeting specific viewports.

Example from the MDN article you shared might look like so with attributes added. The source for smaller viewports outputs a 4:3 image and the one for large viewports outputs in 16:9.

<picture>
  <source media="(max-width: 799px)" width="800" height="600" srcset="elva-480w-close-portrait.jpg" />
  <source media="(min-width: 800px)" width="800" height="450" srcset="elva-800w.jpg" />
  <img src="elva-800w.jpg" alt="Chris standing up holding his daughter Elva" />
</picture>

Support to add the attributes to <source> tags is new, but browser support isn’t too bad.

Specs: https://html.spec.whatwg.org/multipage/embedded-content.html#the-source-element
Support: https://caniuse.com/mdn-api_htmlsourceelement_width

The thing is that Astro’s Picture component doesn’t have a way to do art direction. So until this is a thing it’d probably be fine to just add width and height to the <img>. It worked well before this PR was merged, no layout shifts and Pagespeed Insights was happy.

Regarding CSS I would actually appreciate if the components wouldn’t output any styles or classes. But I don’t know if that’s feasible or practical.

[carlcs]

@tony-sull I just found another resource where they recommend adding width and height attributes to the <source> elements. Like in my example above, they are serving a cropped version for mobile.

https://web.dev/learn/design/picture-element/#cropping

[christian-hackyourshack]

According to the spec, the width and height on an img tag are just the intrinsic size of that image, it does not tell the user agent, how to render the image. Usually the UAs just derive the aspectRatio from it, if not otherwise specified.

@smeevil does not state, what the actual problem is. The referenced article explicitly states, that it is best practice to have width and height on your img and it does not say, you shouldn't on picturized images.

As far as I could find, all major browsers will handle responsive images just fine, even if the dimensions on the img are given.

As @carlcs states, it's now also possible to set width and height on the source element. This is already supported by all major browsers except Firefox, but at the end it just overrides the dimensions from the img tag.

Hence, I would urgently request to undo that change, until we have some good proof, that it is bad practice to add dimensions to responsive images. Currently, it renders the Picture component useless and I have to implement my own based on the getPicture method.

Maybe @smeevil could implement his own very naive wrapper around the Picture component, that removes those attributes. Adding them is much harder, because they are a result of the transformation.

[FatehAK]

Any plans to revert this? I'm facing CLS when using the Picture element since width and height aren't being included automatically.

[mtedwards]

Just following up to see if there are any plans to revert this, as it is causing CLS.

Also, they way to the fix the issue with the image not being responsive due to the set width and height is to set an override in CSS.

The width and Height on the image is used by the browser to draw out the space before the image is loaded.

Adding something like this, will then allow the image to scale as needed.

img { 
   width: 100%;
   height: auto;
}