Skip to content

Catalog image lazy load #27033

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Mar 2, 2020
Merged

Catalog image lazy load #27033

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c00cef4
Fix deprecated method usage
Feb 26, 2020
1a79775
Add loading="lazy" to image_with_borders.phtml
Feb 26, 2020
182a474
Get rid of max-width and max-height in image_with_borders.phtml
Feb 26, 2020
0bd7a87
Fix deprecated method usage
Feb 26, 2020
56fde6c
Add loading="lazy" to image.phtml
Feb 26, 2020
463c460
Add escaping to alt tags
Feb 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions app/code/Magento/Catalog/view/frontend/templates/product/image.phtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@
* See COPYING.txt for license details.
*/
?>
<?php /** @var $block \Magento\Catalog\Block\Product\Image */ ?>
<?php
/** @var $block \Magento\Catalog\Block\Product\Image */
/** @var $escaper \Magento\Framework\Escaper */
?>

<img class="photo image <?= $block->escapeHtmlAttr($block->getClass()) ?>"
<?= $block->escapeHtml($block->getCustomAttributes()) ?>
src="<?= $block->escapeUrl($block->getImageUrl()) ?>"
width="<?= $block->escapeHtmlAttr($block->getWidth()) ?>"
height="<?= $block->escapeHtmlAttr($block->getHeight()) ?>"
alt="<?= /* @noEscape */ $block->stripTags($block->getLabel(), null, true) ?>" />
<img class="photo image <?= $escaper->escapeHtmlAttr($block->getClass()) ?>"
<?= $escaper->escapeHtml($block->getCustomAttributes()) ?>
src="<?= $escaper->escapeUrl($block->getImageUrl()) ?>"
loading="lazy"
width="<?= $escaper->escapeHtmlAttr($block->getWidth()) ?>"
height="<?= $escaper->escapeHtmlAttr($block->getHeight()) ?>"
alt="<?= /* @noEscape */ $block->stripTags($block->getLabel(), null, true) ?>" />
20 changes: 12 additions & 8 deletions app/code/Magento/Catalog/view/frontend/templates/product/image_with_borders.phtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,20 @@
* See COPYING.txt for license details.
*/
?>
<?php /** @var $block \Magento\Catalog\Block\Product\Image */ ?>
<?php
/** @var $block \Magento\Catalog\Block\Product\Image */
/** @var $escaper \Magento\Framework\Escaper */
?>

<span class="product-image-container"
style="width:<?= $block->escapeHtmlAttr($block->getWidth()) ?>px;">
style="width:<?= $escaper->escapeHtmlAttr($block->getWidth()) ?>px;">
<span class="product-image-wrapper"
style="padding-bottom: <?= ($block->getRatio() * 100) ?>%;">
<img class="<?= $block->escapeHtmlAttr($block->getClass()) ?>"
<?= $block->escapeHtmlAttr($block->getCustomAttributes()) ?>
src="<?= $block->escapeUrl($block->getImageUrl()) ?>"
max-width="<?= $block->escapeHtmlAttr($block->getWidth()) ?>"
max-height="<?= $block->escapeHtmlAttr($block->getHeight()) ?>"
alt="<?= /* @noEscape */ $block->stripTags($block->getLabel(), null, true) ?>"/></span>
<img class="<?= $escaper->escapeHtmlAttr($block->getClass()) ?>"
<?= $escaper->escapeHtmlAttr($block->getCustomAttributes()) ?>
src="<?= $escaper->escapeUrl($block->getImageUrl()) ?>"
loading="lazy"
width="<?= $escaper->escapeHtmlAttr($block->getWidth()) ?>"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why you changed max-width and max-height to just width and height? It looks not related to this PR.

As max width and max height are not valid attributes - seems like we could just remove them

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The width and height attributes are needed to preserve the aspect ratio of the image when it is not loaded yet.

Copy link
Contributor

@ihor-sviziev ihor-sviziev Feb 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, no problem, but need just test it carefully on different devices, because it could do regression

height="<?= $escaper->escapeHtmlAttr($block->getHeight()) ?>"
alt="<?= /* @noEscape */ $block->stripTags($block->getLabel(), null, true) ?>"/></span>
</span>