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

suggested srcset sizes are failing the linter #43

Open
goldmerc opened this issue Sep 12, 2020 · 5 comments
Open

suggested srcset sizes are failing the linter #43

goldmerc opened this issue Sep 12, 2020 · 5 comments
Labels

Comments

@goldmerc
Copy link

When I run the bookmarklet on an image with a srcset using w values, it suggests a sizes attribute but when I use the suggested sizes attribute, the test still fails...

<img
	srcset="
		/images/silverware/ash-trays/l3118/xs/l3118-silver-ash-trays-1.jpg 80w,
		/images/silverware/ash-trays/l3118/s/l3118-silver-ash-trays-1.jpg 100w,
		/images/silverware/ash-trays/l3118/m/l3118-silver-ash-trays-1.jpg 200w,
		/images/silverware/ash-trays/l3118/l/l3118-silver-ash-trays-1.jpg 350w,
		/images/silverware/ash-trays/l3118/xl/l3118-silver-ash-trays-1.jpg 600w,
	"
	src="/images/silverware/ash-trays/l3118/xl/l3118-silver-ash-trays-1.jpg"
	sizes="
		(min-width: 1520px) 573px,
		(min-width: 1220px) 39.29vw,
		(min-width: 780px) calc(45.24vw - 60px),
		calc(56.3vw - 102px)
	"
>

The sizes attribute has to match the width of the image
The size of the image doesn’t match the sizes attribute (min-width: 1520px) 573px, (min-width: 1220px) 39.29vw, (min-width: 780px) calc(45.24vw - 60px), calc(56.3vw - 102px). At a viewport of 320x427 the image was 208 pixels wide instead of the specified 78 (167% difference). The affected viewports are 300x400-760x1013.

Try using sizes="(min-width: 1520px) 573px, (min-width: 1220px) 39.29vw, (min-width: 780px) calc(45.24vw - 60px), calc(56.3vw - 102px)" instead.

Are the bookmarklet generated sizes wrong? Or am I doing something wrong?

@ausi
Copy link
Owner

ausi commented Sep 12, 2020

At a viewport of 320x427 the image was 208 pixels wide instead of the specified 78 (167% difference).

The linter says that your image is 208 pixels wide if your viewport (browser window) has a width of 320.

In your sizes attribute you defined that the image has a width of calc(56.3vw - 102px) in this case which evaluates to:

56.3vw - 102px
= 0.563 * 320px - 102px
= 180px - 102px
= 78px

So it looks like your sizes attribute is wrong, or the linter detected the image width of 208 incorrectly.

@goldmerc
Copy link
Author

Hi Ausi - thanks for a great tool!

If I use chrome devtools to set the viewport to 320, the image is indeed 208 pixels wide. So, the sizes attribute appears wrong.

But I'm using the sizes attribute that respimagelint suggested...

Try using sizes="(min-width: 1520px) 573px, (min-width: 1220px) 39.29vw, (min-width: 780px) calc(45.24vw - 60px), calc(56.3vw - 102px)" instead.

The reason I'm playing with respimagelint is that I was hoping it would calculate the sizes attribute for me. But I'm not sure if I'm misunderstanding, if the i'm doing something wrong or if respimagelint is making a mistake when it calculates the sizes.

@ausi
Copy link
Owner

ausi commented Sep 12, 2020

Oh I see. It’s intresting that respimagelint itself suggests to use calc(56.3vw - 102px). That seems to be an error indeed. Can you post a link to the website so that I can have a look?

@goldmerc
Copy link
Author

Sure. I'm working on an old site, where the frontend was written by someone else. So, don't judge me! At some point I plan to rebuild it properly but at the moment I'm just patching what I've inherited. It's tricky because the image sizes seem to jump around a lot as the viewport changes which is why I'm looking for a tool to calculate the sizes attribute for me.

At the moment, the live version of the site doesn't use responsive images. So, you can see an example page here but respimagelint won't suggest the sizes attribute for you. The only change on my local machine is that I've switched out the main product img tag (the image in the div with id 'zoom_img') for one with a srcset and sizes attribute in the format of my original post. That's the image which respimagelint is suggesting the sizes attribute for, which then fails it's own test.

Thanks for taking a look.

@ausi
Copy link
Owner

ausi commented Sep 16, 2020

The width of the image on your webpage also depends on the height of the viewport. The linter can currently not detect such cases and is not able to calculate the correct sizes attribute.

@ausi ausi added the bug label Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants