Skip to content

Check hero image media attribute before srcset #136

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
Apr 9, 2021
Merged

Check hero image media attribute before srcset #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5140655
Move media check further up
schlessera Apr 8, 2021
ae3161e
Always remove lazy loading on hero image
schlessera Apr 8, 2021
765419c
Use separate method for lazy loading removal
schlessera Apr 8, 2021
47d4f2e
media attribute to test for throwing scrscet error
schlessera Apr 8, 2021
d0dc1c4
Adapt test for preload
schlessera Apr 8, 2021
d6dfe43
Add test for when preloading is enabled for srcset images
westonruter Apr 9, 2021
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
35 changes: 21 additions & 14 deletions src/Optimizer/Transformer/PreloadHeroImage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ public function transform(Document $document, ErrorCollection $errors)
}

for ($index = 0; $index < $heroImageCount; $index++) {
$this->removeLazyLoading($heroImages[$index]);
$this->generatePreload($heroImages[$index], $document, $errors);
$this->generateImg($heroImages[$index], $document);
}
Expand Down Expand Up @@ -470,22 +471,12 @@ private function getPlaceholderImage(Element $element)
}

/**
* Generate the preload link for a given hero image.
* Remove the lazy loading from the hero image.
*
* @param HeroImage $heroImage Hero image to generate the preload link for.
* @param Document $document Document to generate the preload link in.
* @param ErrorCollection $errors Collection of errors that are collected during transformation.
* @param HeroImage $heroImage Hero image to remove the lazy loading for.
*/
private function generatePreload(
HeroImage $heroImage,
Document $document,
ErrorCollection $errors
) {
if ($heroImage->getSrcset() && ! $this->supportsSrcset()) {
$errors->add(Error\CannotPreloadImage::fromImageWithSrcsetAttribute($heroImage->getAmpImg()));
return;
}

private function removeLazyLoading(HeroImage $heroImage)
{
$img = $heroImage->getAmpImg();

if (
Expand All @@ -495,13 +486,28 @@ private function generatePreload(
) {
$img->removeAttribute(Attribute::LOADING);
}
}

/**
* Generate the preload link for a given hero image.
*
* @param HeroImage $heroImage Hero image to generate the preload link for.
* @param Document $document Document to generate the preload link in.
* @param ErrorCollection $errors Collection of errors that are collected during transformation.
*/
private function generatePreload(HeroImage $heroImage, Document $document, ErrorCollection $errors)
{
if (empty($heroImage->getMedia())) {
// We can only safely preload a hero image if there's a media attribute
// as we can't detect whether it's hidden on certain viewport sizes otherwise.
return;
}

if ($heroImage->getSrcset() && ! $this->supportsSrcset()) {
$errors->add(Error\CannotPreloadImage::fromImageWithSrcsetAttribute($heroImage->getAmpImg()));
return;
}

if ($this->hasExistingImagePreload($document, $heroImage->getSrc())) {
return;
}
Expand All @@ -517,6 +523,7 @@ private function generatePreload(
$preload->appendChild($document->createAttribute(Attribute::DATA_HERO));
if ($heroImage->getSrcset()) {
$preload->setAttribute(Attribute::IMAGESRCSET, $heroImage->getSrcset());
$img = $heroImage->getAmpImg();
if ($img && $img->hasAttribute(Attribute::SIZES)) {
$preload->setAttribute(Attribute::IMAGESIZES, $img->getAttribute(Attribute::SIZES));
}
Expand Down
29 changes: 23 additions & 6 deletions tests/Optimizer/Transformer/PreloadHeroImageTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,26 +98,43 @@ public function dataTransform()

'throws error when scrset detected on image to be preloaded' => [
$input(
'<amp-img data-hero width="500" height="400" src="https://example-com.cdn.ampproject.org/hero1.png" srcset="test 100w test2 3dpr"></amp-img>'
. '<amp-img data-hero width="500" height="400" src="https://example-com.cdn.ampproject.org/hero2.png"></amp-img>'
'<amp-img data-hero media="(min-width: 250px)" width="500" height="400" src="https://example-com.cdn.ampproject.org/hero1.png" srcset="test 100w test2 3dpr"></amp-img>'
. '<amp-img data-hero media="(min-width: 250px)" width="500" height="400" src="https://example-com.cdn.ampproject.org/hero2.png"></amp-img>'
),
$output(
'<amp-img data-hero width="500" height="400" i-amphtml-ssr src="https://example-com.cdn.ampproject.org/hero1.png" srcset="test 100w test2 3dpr">'
'<amp-img data-hero media="(min-width: 250px)" width="500" height="400" i-amphtml-ssr src="https://example-com.cdn.ampproject.org/hero1.png" srcset="test 100w test2 3dpr">'
. '<img class="i-amphtml-fill-content i-amphtml-replaced-content" decoding="async" src="https://example-com.cdn.ampproject.org/hero1.png" srcset="test 100w test2 3dpr">'
. '</amp-img>'
. '<amp-img data-hero width="500" height="400" i-amphtml-ssr src="https://example-com.cdn.ampproject.org/hero2.png">'
. '<amp-img data-hero media="(min-width: 250px)" width="500" height="400" i-amphtml-ssr src="https://example-com.cdn.ampproject.org/hero2.png">'
. '<img class="i-amphtml-fill-content i-amphtml-replaced-content" decoding="async" src="https://example-com.cdn.ampproject.org/hero2.png">'
. '</amp-img>'
. '</amp-img>',
'<link as="image" data-hero href="https://example-com.cdn.ampproject.org/hero2.png" media="(min-width: 250px)" rel="preload">'
),
[
Error\CannotPreloadImage::fromImageWithSrcsetAttribute(
Document::fromHtmlFragment(
'<amp-img data-hero width="500" height="400" src="https://example-com.cdn.ampproject.org/hero1.png" srcset="test 100w test2 3dpr"></amp-img>'
'<amp-img data-hero media="(min-width: 250px)" width="500" height="400" src="https://example-com.cdn.ampproject.org/hero1.png" srcset="test 100w test2 3dpr"></amp-img>'
)->body->firstChild
),
],
],

'preloads scrset image when configured to do so' => [
$input(
'<amp-img data-hero media="(min-width: 250px)" width="500" height="400" src="https://example-com.cdn.ampproject.org/hero.png" srcset="test 100w test2 3dpr" sizes="100vw"></amp-img>'
),
$output(
'<amp-img data-hero media="(min-width: 250px)" width="500" height="400" i-amphtml-ssr src="https://example-com.cdn.ampproject.org/hero.png" srcset="test 100w test2 3dpr" sizes="100vw">'
. '<img class="i-amphtml-fill-content i-amphtml-replaced-content" decoding="async" src="https://example-com.cdn.ampproject.org/hero.png" srcset="test 100w test2 3dpr" sizes="100vw">'
. '</amp-img>',
'<link as="image" data-hero href="https://example-com.cdn.ampproject.org/hero.png" imagesizes="100vw" imagesrcset="test 100w test2 3dpr" media="(min-width: 250px)" rel="preload">'
),
[],
[
PreloadHeroImageConfiguration::PRELOAD_SRCSET => true,
]
],

'fetches placeholders for animations' => [
$input(
'<amp-anim data-hero width="500" height="400" src="/foo.gif">'
Expand Down