First-party support for a Picture component RFC#715
Conversation
|
@Princesseuh thank you for letting me try it out! I’m getting a type error: I did not expect this warning: I figured I would be able to use widths and tack on a density. Say, Another unexpected result is that it upscales images. I would expect that if a source image is only Lastly, while:
What do you think about sensible defaults? WordPress for example uses:
Anyway, just food for thought. |
Not sure about that one, probably just something I didn't finish.
This is actually a limitation of HTML itself, you cannot mix pixel and width descriptors in the same
Hmm, we could definitely add a way to specify the original size as a widths, that should cover the first use case nicely. There are cases where you do actually want it to upscale, so it makes sense to support it.
Still thinking about that one because every framework kinda has a different take on it and I'm not sure which one would be the best for Astro. Nuxt notably has an entire system where you set up screens and then there's a DSL to generate sizes, it's a lot. For now I'm still in the camp for it being a non-goal, but would be happy to revisit in another proposal perhaps. |
I was able to find the source of type error when the Image component released, but not sure about where this one is coming from.
Interesting. I usually use a library for this kind of thing, so that’s nice to know.
I’ve been racking my brain, but I can’t think of a use case for that. I manually use AI upscalers if I need to upscale an image. Or, for things like pixel art, I usually use css image-rendering. It’d be great to have the option to disable that completely; otherwise, I’ll just be shipping a lower-quality image while using more bandwidth.
I’ll probably end up making a general Picture component wrapper with default settings then |
Hmm, I'm not necessarily against not having it, l think I might just not be sure how it would exactly manifest. Let's say you have an original image that is 1280x720, so classic 720p. You write this: Similarly, should |
|
|
||
| ## For the built-in experience | ||
|
|
||
| - Automatic generation or API to make writing `sizes` easier |
There was a problem hiding this comment.
You mean we don't append media, right? What would someone do who needed this?
There was a problem hiding this comment.
No, this line is about sizes for both <Image />, <Picture />. Certain frameworks have automatic generation for this, or actually uses sizes to generate the srcset instead of a separate attribute (typically through a DSL or through a normal sizes attribute, convoluted syntax and all)
media is also not supported in the base Picture component, because art direction is not supported. (But, it's of course very possible using your own Picture component)
There was a problem hiding this comment.
media and art direction might be a good one to list as alternatives considered
As much as I've reached for that approach over the years, it's pretty cumbersome to work with and is way too easy to silently break your media breakpoints later. Marking that as not supported here sounds like a smart move to me
|
Published a new preview release of the implementation PR: withastro/astro#8620 with the following changes that I collected from feedback on this PR:
I will be updating the RFC with those changes soon. |
|
Is it possible to apply the type PictureProps = AllTheProps & ({ widths: ...; } | { densities: ...; }) |
Hmm, yes I believe that would be possible! |
If I wrote
Anyway, thank you for disabling automatic image upscale 😄 I will try out the new prerelease again tomorrow—I’m going to Mt. Evans today 🏔️ |
|
Thanks for working on this @Princesseuh! It’s all looking very promising. If I understand correctly, this doesn’t allow to produce images in different aspect ratios than the original one. Is this just out of scope for this implementation and will be added later? If that’s the case it might be worth adding a sentence on that in the non-goals section of the RFC. |
Yes, this is out of scope for this proposal. This is what is meant by "Complex art direction with the built-in component" in the Non-goals section. It could be added in the future, for sure! Right now our base image services don't support cropping, which is kinda required for this |
I didn’t mean art direction (different source images or aspect ratios served using media queries) and I understand that this makes things complex. A very common use case though is to serve an image in a different aspect ratio than the one of the original image. You can of course use CSS to crop / hide parts of the image, but I would rather not send pixels that are never seen by anyone. |
Ah yes, you mean just for |
|
Published a draft of docs for this feature here: withastro/docs#4866 |
Is there a setting I need to use to avoid upscaling? |
Hmm, no it should normally work by default! I mainly tested with densities, so it's possible there's a mistake for widths. Is it possible that it's still generating the widths, but the image paths only refer to the right sizes? (We'll of course do tests and stuff before actually releasing it) |
Ahh, I was using widths—I believe it still includes and upscales widths larger than the image in the srcset |
Hmm, weird. I'll take a look next week when I'm back to work, sorry for the inconvenience! |
|
Awesome work guys. |
|
Posted a new preview release:
Unless something else comes up, this is likely to be the version that we'll have in experimental mode on next Thursday. Feedback is of course still welcome, we're just trying to get it into more hands. |
|
Hello, Thank you for making this component available again. I just upgrade to Astro 3 and it was easy to update the Picture component. However, I encountered an error: MissingImageDimension: Missing width attribute for /content/uploads/anchor-text-html.png. When using remote images, both dimensions are always required in order to avoid CLS.The component crashes if the width or height attributes are missing, even though standard HTML doesn't have this behavior. Of course CLS is important, but I'm wondering if it's appropriate for Astro to enforce this. What do you think about throwing the warning but still rendering the component ? |
This is the same behaviour as |
|
Hey, awesome work! |
That would be nice! Definitely something I'd be open to add. I assume people would love to see a |
|
Hello everyone! We're entering the final phase of comments on this RFC. If you have any final feedback or concerns, please leave them here. Of course, this does not mean the feature is set in stone, but more so that this initial design makes sense. In the future, it is very possible that we would add some of the missing features here, such as art direction for instance. |
|
Thank you everyone for your comments! This Stage 3 proposal is now accepted, and the feature is available in Astro. We'll be improving and maintaining it for the future with fixes and new features (probably including some of the non-goals described in this RFC) |
Summary
The current
<Image />component andgetImageAPIs are great to show a single image, however it doesn't answer some of the common needs of the web, such as fallback formats and responsive images. This RFC hopes to define the addition of two features:srcsetrequestLinks