Images: Fix dynamic size loading - #40
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts responsive image loading for product cards on the search and collection pages by updating the widths/sizes configuration to avoid over-requesting “retina” sizes while still allowing higher-resolution sources when needed.
Changes:
- Updated product card image
widthsto include smaller and larger candidates (250, 500, 800). - Replaced fixed
sizesvalues with a more layout-driven calculation for small screens (calc(100vw - 66px)), and a fixed fallback size for larger screens.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
sections/search.liquid |
Updates image_tag widths/sizes for search result product images. |
sections/collection-page.liquid |
Updates image_tag widths/sizes for collection product grid images. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| widths: '500, 700', | ||
| sizes: '(max-width: 489px) 700px, 500px', | ||
| widths: '300, 600, 900', | ||
| sizes: '(max-width: 489px) calc(100vw - 66px), 286px', |
There was a problem hiding this comment.
This is how browser works with sizes:
- Figure out the total browser width. Lets say it is a phone and width is 480px.
- Use
sizesto determine the image size given the screen size. In this case the rule(max-width: 489px)will match and we calculate the image size to: 414px. For larger screens we will fall back to287px. - We have 3 candidate widths
300, 600, 900. Divide each width with our calculated width. We get:0,721,44and2,17 - Pick the image that has ratio closest to the screen density. If it is a retina screen (2x) browser will pick the
2,17ratio or the 900px image. If it is a regular screen it is going to pick the 600px image as this is closest to1xratio.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
sections/search.liquid:41
- The
sizesvalue switches to a fixed286pxabove 489px, but in the 490–575px range the grid becomes 2 columns (CSS usesgrid-template-columns: repeat(auto-fill, minmax(220px, 1fr))and only appliesmax-width: 320pxstarting at 576px). That means the rendered image is closer tocalc(50vw - 59px)in this range; using286pxoverestimates and can cause larger-than-needed image downloads on small tablets.
sizes: '(max-width: 489px) calc(100vw - 66px), 286px',
sections/collection-page.liquid:78
- The
sizesvalue switches to a fixed286pxabove 489px, but between 490–575px the grid is typically 2 columns and.producthas nomax-widthuntil 576px. In that range the rendered image width is closer tocalc(50vw - 59px); using286pxoverestimates and can lead to unnecessarily large image downloads on small tablets.
sizes: '(max-width: 489px) calc(100vw - 66px), 286px',
Why
Current image sizes configuration is not correct. We assumed that we need to declare double size images in the
sizesin order for retina pages to have high enough resolution - this is not correct. Browser will automatically load double size image depending on the screen density.This PR updates the
sizessettings to more conservative values, but increases the maximum image width we provide in order to enable loading higher resolution images when needed.Media