Poor image quality possibly due to bad Liquid code in srcset attributes on img tags

[ADTC]

Describe the current behavior

Images for products and collections turn out to be blurry.

Describe the expected behavior

Images must be clear on any screen size.

Version information (Dawn, browsers and operating systems)

• Dawn Version: 2.1.0
• Firefox Version 92.0
• Windows 10 Version 20H2

Possible solution

Perhaps we're doing responsive images wrong. These guidelines on MDN as well as the HTML Standard could help. 🙂

Additional context/screenshots

It's my first time using Dawn theme as I wanted to explore OS 2.0 and upgrade my skill-set as a Shopify web developer. My client for whom I used the theme (with some of my own modifications) complained that the product images are too blurry. Upon testing, I found out that the srcset attribute value is not rendered correctly, so the browser is unable to access the highest possible resolution. I had to do this crazy fix below to fix the problem.

diff --git a/snippets/product-thumbnail.liquid b/snippets/product-thumbnail.liquid
index 3b9aaac..8ebbb7e 100644
--- a/snippets/product-thumbnail.liquid
+++ b/snippets/product-thumbnail.liquid
@@ -73,11 +73,11 @@

   <div class="product__media media media--transparent" style="padding-top: {{ 1 | divided_by: media.preview_image.aspect_ratio | times: 100 }}%;">
     <img
-      srcset="{% if media.preview_image.width >= 288 %}{{ media.preview_image | img_url: '288x' }} 288w,{% endif %}
-        {% if media.preview_image.width >= 576 %}{{ media.preview_image | img_url: '576x' }} 576w,{% endif %}
-        {% if media.preview_image.width >= 750 %}{{ media.preview_image | img_url: '750x' }} 750w,{% endif %}
-        {% if media.preview_image.width >= 1100 %}{{ media.preview_image | img_url: '1100x' }} 1100w,{% endif %}
-        {% if media.preview_image.width >= 1500 %}{{ media.preview_image | img_url: '1500x' }} 1500w{% endif %}"
+      srcset="{{ media.preview_image | img_url: '288x' }} 288w,
+        {% if media.preview_image.width >= 288 %}{{ media.preview_image | img_url: '576x' }} 576w,{% endif %}
+        {% if media.preview_image.width >= 576 %}{{ media.preview_image | img_url: '750x' }} 750w,{% endif %}
+        {% if media.preview_image.width >= 750 %}{{ media.preview_image | img_url: '1100x' }} 1100w,{% endif %}
+        {% if media.preview_image.width >= 1100 %}{{ media.preview_image | img_url: '1500x' }} 1500w{% endif %}"
       src="{{ media | img_url: '1500x' }}"
       sizes="(min-width: {{ settings.page_width }}px) {{ settings.page_width | minus: 100 | times: 0.64 | round }}px, (min-width: 750px) calc((100vw - 11.5rem) / 2), calc(100vw - 4rem)"
       loading="lazy"

I had to push the width check one step up, because with the original code, if the image is not wide enough, a smaller version of it is shown. This meant the clarity was already compromised at the source no matter what the viewport width was.

The problem with the original Liquid code here is that it doesn't add a line to the srcset attribute for the original width of the image. As an example, take an image that is 900 pixels wide. In the original code, it stops at 750w with the condition media.preview_image.width >= 750 and the browser never sees the original image that's 900px wide.

In the modified one, the condition media.preview_image.width >= 750 adds the image for 1100w as well. This has two advantages:

• The browser will see and use the image that is 900px wide when the viewport is greater than 750w, although the image is accessed through an image URL with 1100x suffix.
• Shopify never up-scales the image, so the image served through the image URL with 1100x suffix is the original image that is 900px wide.

Can we apply this modification for the Liquid code on all srcset declarations in the Dawn theme?

[maxime-memtell]

Hey @ADTC thanks for working on this! I have the same issue and I'm looking for solutions. A little googling seems to show we're not alone and one dude posts everywhere to say the "Craft" version doesn't have the issue. I tried but still felt like things were blurry.

I tested your fix but couldn't see a difference on my collections pages, if anything, maybe it got worse? I'll try tinkering around and see if I can get it working for me and report back in case I do.

Also I think this issue is related #532

[ludoboludo]

Hey folks 👋
Thanks for flagging this issue on the repo!
I'd be curious to know if the fix applied in this PR has fixed the issue for you or not.

It should have I believe but if you could confirm, that'd be great! Thank you 🙂

[ADTC]

Wow, that's a cool idea I didn't think of! 🤦‍♂️ I think that will fix the issue as it will always include the original image. However I have not yet tested it!

[ludoboludo]

Let me know when you have a chance to test it, if it works out for in your use case @ADTC 🙂
And if not we can have another look at that solution.

[ludoboludo]

Also it looks like in some cases (the product card) we need to update the way we calculate the sizes for the sizes attribute on the images. As if you have 2 products only their image will be of lower quality because in the sizes calculation we calculate the size like if there was always 4 products on one row on desktop.

Instead we should try to dynamically change that number or only divide by 2 at all time.

sizes="(min-width: {{ settings.page_width }}px) {{ settings.page_width | minus: 130 | divided_by: 4 }}px, (min-width: 990px) calc((100vw - 130px) / 4), ...

[shawnr42]

I just tested, changing the calculations Ludo pointed out, to use divided by 2, instead of 4, and it made the image better.

[klawrz89]

Just had a shop where product images were showing as blurry, and the 'divide by 2' fix didn't work. What did work however, was changing this line to multiply by 2:

sizes="(min-width: {{ settings.page_width }}px) {{ settings.page_width | minus: 130 | divided_by: 2 }}px, (min-width: 990px) calc((100vw - 130px) / 2), (min-width: 750px) calc((100vw - 120px) / 3), calc((100vw - 35px) / 2)"

With this line (note that we are multiplying at the end:

sizes="(min-width: {{ settings.page_width }}px) {{ settings.page_width | minus: 130 | divided_by: 2 }}px, (min-width: 990px) calc((100vw - 130px) / 2), (min-width: 750px) calc((100vw - 120px) / 3), calc((100vw - 35px) * 2)"

This is in combination with the original /2 fix for large screens

[shawnr42]

I did more digging after seeing @klawrz89's comment about mobile issues, and I think I found the general solution for all screen sizes.

Before the <img tag add these two lines

{% assign columns_desktop = section.settings.columns_desktop | at_least: 1 %}
{% assign columns_mobile = section.settings.columns_mobile | at_least: 1 %}

Replace the sizes line in the <img tags with this

sizes="(min-width: {{ settings.page_width }}px) {{ settings.page_width | minus: 130 | divided_by: columns_desktop }}px, (min-width: 990px) calc((100vw - 130px) / {{ columns_desktop }}), (min-width: 750px) calc((100vw - 120px) / {{ columns_desktop }}), calc((100vw - 35px) / {{ columns_mobile }})"

This solution uses the correct number of columns for the image sizes, instead of assuming all stores will have 4 products per row, on computer screens, and 2 columns per row on mobile.

In the end, you get this full block of code.

{% assign columns_desktop = section.settings.columns_desktop %}
{% assign columns_mobile = section.settings.columns_mobile %}
<img
  srcset="
  {%- if card_product.featured_media.width >= 165 -%}{{ card_product.featured_media | image_url: width: 165 }} 165w,{%- endif -%}
  {%- if card_product.featured_media.width >= 360 -%}{{ card_product.featured_media | image_url: width: 360 }} 360w,{%- endif -%}
  {%- if card_product.featured_media.width >= 533 -%}{{ card_product.featured_media | image_url: width: 533 }} 533w,{%- endif -%}
  {%- if card_product.featured_media.width >= 720 -%}{{ card_product.featured_media | image_url: width: 720 }} 720w,{%- endif -%}
  {%- if card_product.featured_media.width >= 940 -%}{{ card_product.featured_media | image_url: width: 940 }} 940w,{%- endif -%}
  {%- if card_product.featured_media.width >= 1066 -%}{{ card_product.featured_media | image_url: width: 1066 }} 1066w,{%- endif -%}
  {{ card_product.featured_media | image_url }} {{ card_product.featured_media.width }}w
  "
  src="{{ card_product.featured_media | image_url: width: 533 }}"
  sizes="(min-width: {{ settings.page_width }}px) {{ settings.page_width | minus: 130 | divided_by: columns_desktop }}px, (min-width: 990px) calc((100vw - 130px) / {{ columns_desktop }}), (min-width: 750px) calc((100vw - 120px) / {{ columns_desktop }}), calc((100vw - 35px) / {{ columns_mobile }})"
  alt="{{ card_product.featured_media.alt | escape }}"
  class="motion-reduce"
  {% unless lazy_load == false %}
  loading="lazy"
  {% endunless %}
  width="{{ card_product.featured_media.width }}"
  height="{{ card_product.featured_media.height }}"
>

Do this for the second image too, but you don't need to add the assign blocks.

[ludoboludo]

Something similar is being discussed in #2277