Skip to content
This repository was archived by the owner on Dec 8, 2023. It is now read-only.

update the image component to output the image width #1

Merged
merged 2 commits into from
Feb 23, 2023

Conversation

SirNovi
Copy link
Contributor

@SirNovi SirNovi commented Jan 9, 2023

Browsers need to be told the image width of every srcset file in order to choose correctly.

P.S. Thanks for your project

Browsers need to be told the image width of every srcset file in order to choose correctly.

P.S. Thanks for your project
@vercel
Copy link

vercel bot commented Jan 9, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
sveltekit-imagetools ✅ Ready (Inspect) Visit Preview Jan 9, 2023 at 10:46PM (UTC)

@rdela
Copy link
Owner

rdela commented Jan 16, 2023

Unsure about this since (like Ben’s pseudo-code) on MDN the relevant example:

<picture>
  <source srcset="photo.avif" type="image/avif" />
  <source srcset="photo.webp" type="image/webp" />
  <img src="photo.jpg" alt="photo" />
</picture>

…does not specify width even though in the srcset section directly above we have:

Each image descriptor is composed of a URL of the image, and either:

  • a width descriptor, followed by a w (such as 300w); OR
  • a pixel density descriptor, followed by an x (such as 2x) to serve a high-res image for high-DPI screens.

Yet the source element spec:

If the srcset attribute has any image candidate strings using a width descriptor, the sizes attribute must also be present, and is a sizes attribute.

…implies a width descriptor is not mandatory.

It definitely doesn't do any harm by being there and makes this a more useful example in certain cases, but it does add a small bit of complexity. I am inclined to merge it but would love any thoughts from others who might care: @benmccann, @rchrdnsh, @eur2, @elibenton, @sawyerclick, @crypticwasp254, and @alohaquando etc…even just a 👍 or 👎 on this comment. This is a rare invitation to bikeshed, don't squander it 😆

@SirNovi
Copy link
Contributor Author

SirNovi commented Jan 16, 2023

I think it's a very common use case to provide multiple images and let the browser decide which one to use in regard to the browser size.

Also see the examples in here: https://developer.mozilla.org/en-US/docs/Learn/HTML/Multimedia_and_embedding/Responsive_images

<img
  srcset="elva-fairy-480w.jpg 480w, elva-fairy-800w.jpg 800w"
  sizes="(max-width: 600px) 480px,
         800px"
  src="elva-fairy-800w.jpg"
  alt="Elva dressed as a fairy" />

I also added the sizes attribute to my component but having the widths available is useful either way as sizes defaults to '100vw' if not supplied.

In the example you shared, the browser can pick based on file-type and in other examples it can pick based on pixel density but it certainly needs something to go on and width should be an obvious one.

@benmccann
Copy link

I don't think the width is required. You can provide different sized images, but you can also provide a single image size in different formats

@SirNovi
Copy link
Contributor Author

SirNovi commented Jan 17, 2023

I don't think the width is required. You can provide different sized images, but you can also provide a single image size in different formats

I used the provided code and piped a bunch of different resolutions in and luckily checked if the browser was using an appropriate size. It doesn't if there's no way, i.e. no width attribute, to differentiate the images.

If the width attribute doesn't hurt when using images in different formats, why not have it for occasions when you want to use multiple images?

@eur2
Copy link

eur2 commented Jan 17, 2023

@rdela Indeed, "srcset" and "width" attributes would be great!
Maybe let's follow the model of next-image or gatsby-image, a kind of standard to manage and optimize image?

@eur2
Copy link

eur2 commented Jan 22, 2023

It seems that this new package will solve all the image optimization issues:
https://github.com/divriots/jampack

@rdela
Copy link
Owner

rdela commented Feb 23, 2023

I need to add some documentation for this but I will do that in a separate PR and let everyone know here when that is merged. Thanks @SirNovi you'll always be #1 here! 🥇

@rdela rdela merged commit be2b7d7 into rdela:trunk Feb 23, 2023
rdela added a commit that referenced this pull request Feb 23, 2023
* trunk:
  update image component to output image width (#1)
rdela added a commit that referenced this pull request Feb 23, 2023
* trunk:
  update image component to output image width (#1)
@rdela rdela mentioned this pull request Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants