ausi / respimagelint

Linter for Responsive Images - https://ausi.github.io/respimagelint/
MIT License
778 stars 29 forks source link

suggested srcset sizes are failing the linter #43

Open goldmerc opened 4 years ago

goldmerc commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.