Shopify / theme-check

The Ultimate Shopify Theme Linter
Other
333 stars 96 forks source link

Check the width against the srcset values #660

Open ludoboludo opened 1 year ago

ludoboludo commented 1 year ago

Is your feature request related to a problem? Please describe.

When you output an image like this:

{{ section.settings.image_2 | image_url: width: 3840 | image_tag:
      loading: 'lazy',
      width: section.settings.image_2.width,
      height: image_height_2,
      class: image_class_2,
      sizes: sizes,
      widths: '375, 550, 750, 1100, 1500, 1780, 2000, 3000, 3840',
      alt: section.settings.image_2.alt | escape
 }}

The image_url: width: value is going to limit the what widths you can output/generate. So ideally the image_url: width: value should be similar to the highest value declared in the widths:.

Describe the solution you'd like

There could be a warning showing up when the value is different or lower 🤔

Describe alternatives you've considered

Manual checking 😅

Additional context

This is something we ran into in Dawn. We realized we were using 1500 on the image banner image and it would never output an image bigger than 1500 despite having some widths set up to 3840.

bakura10 commented 1 year ago

@ludoboludo , the proper fix is to make the width optional for image_url and make sure it generates the highest possible value:

{{ section.settings.image_2 | image_url | image_tag: xxx }}

The width should be implicitly the image max width. I remember we had this discussion already, has it been refused?

ludoboludo commented 1 year ago

Hey @bakura10, I think there might be a blocker to do with potential future features. Not too sure but I'll double check with the team to confirm.