Skip to content
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.

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

Optimize lazy-loading of IMG elements in Image Prioritizer #1261

Merged
merged 9 commits into from
Jun 3, 2024

Conversation

westonruter
Copy link
Member

@westonruter westonruter commented May 30, 2024

Fixes #1077

This implements optimization of lazy-loading based on the intersectionRatio of each image captured in the URL metrics. In particular, if an image is visible in the initial viewport for any viewport size, then loading=lazy should be omitted. In contrast, if an image is not visible in any viewport, then loading=lazy should be added.

This PR also improves setting the data-od- meta attributes by automatically setting them when calling OD_HTML_Tag_Walker::set_attribute() and OD_HTML_Tag_Walker::remove_attribute(). These attributes provide a window into what operations were performed to optimize the page. Additionally, an OD_HTML_Tag_Walker::set_meta_attribute() method is added which allows you to set any data-od--prefixed attribute for additional diagnostics.

This also takes an initial stab at writing the readme for Image Prioritizer, and updating the readme for Optimization Detective to account for the new dependent plugin.

@westonruter westonruter added [Type] Enhancement A suggestion for improvement of an existing feature [Plugin] Optimization Detective Issues for the Optimization Detective plugin labels May 30, 2024
@westonruter westonruter added this to the image-prioritizer 0.1.0 milestone May 30, 2024
@westonruter westonruter force-pushed the add/img-lazy-loading-optimization branch 4 times, most recently from 35850bd to 7b781fb Compare May 31, 2024 23:39
@westonruter westonruter force-pushed the add/img-lazy-loading-optimization branch from 43403a5 to c0ed712 Compare June 1, 2024 00:37
$walker->set_attribute( 'loading', 'lazy' );
}
}
// TODO: If an image is the LCP element for one breakpoint but not another, add loading=lazy. This won't hurt performance since the image is being preloaded.
Copy link
Member Author

Choose a reason for hiding this comment

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

That is, if the image is not the LCP element for all breakpoints and the image is not in the initial viewport.

$walker->remove_attribute( 'fetchpriority' );
}

// TODO: If the image is visible (intersectionRatio!=0) in any of the URL metrics, remove loading=lazy.
// TODO: Conversely, if an image is the LCP element for one breakpoint but not another, add loading=lazy. This won't hurt performance since the image is being preloaded.
// TODO: Also if the element isLCPCandidate it should never by lazy-loaded.
Copy link
Member Author

Choose a reason for hiding this comment

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

But in this case, clearly the intersectionRatio will be non-zero. So this is redundant.

@westonruter westonruter force-pushed the add/img-lazy-loading-optimization branch from c0ed712 to 9527d38 Compare June 3, 2024 18:39
Base automatically changed from add/image-prioritizer-plugin-for-reals to trunk June 3, 2024 20:25
@westonruter westonruter force-pushed the add/img-lazy-loading-optimization branch from 9527d38 to 9389a98 Compare June 3, 2024 20:26
} else {
$walker->set_attribute( 'fetchpriority', 'high' );
$walker->set_attribute( 'data-od-added-fetchpriority', true );
Copy link
Member Author

Choose a reason for hiding this comment

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

This data-od- meta attribute is now added automatically when calling set_attribute on the walker instance.

Comment on lines -55 to 52
// Never include loading=lazy on the LCP image common across all breakpoints.
if ( 'lazy' === $walker->get_attribute( 'loading' ) ) {
$walker->set_attribute( 'data-od-removed-loading', $walker->get_attribute( 'loading' ) );
$walker->remove_attribute( 'loading' );
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This code is obsolete because the loading attribute is now removed for any element which has an non-zero intersectionRatio in any viewport group.

$walker->set_attribute( 'loading', 'lazy' );
}
}
// TODO: If an image is visible in one breakpoint but not another, add loading=lazy AND add a regular-priority preload link with media queries (unless LCP in which case it should already have a fetchpriority=high link) so that the image won't be eagerly-loaded for viewports on which it is not shown.
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll be filing an issue for this. It may be a bit confusing when looking at the markup because they may well see an LCP img element which has loading=lazy, but it will still get prioritized via the high-fetchpriority preload link (with a media query). The Lighthouse audit will need to be updated to account for this case.

Copy link
Member Author

Choose a reason for hiding this comment

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

In my testing at #117 (comment) I found that adding loading=lazy to the LCP image does not degrade performance if there is a preload link with fetchpriority=high.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've opened this as #1270

Comment on lines -238 to +264
<img data-od-removed-fetchpriority="high" src="https://example.com/bar.jpg" alt="Bar" width="10" height="10" loading="lazy" >
<p>Pretend this is a super long paragraph that pushes the next image mostly out of the initial viewport.</p>
<img data-od-removed-fetchpriority="high" data-od-removed-loading="lazy" src="https://example.com/bar.jpg" alt="Bar" width="10" height="10" >
<p>Now the following image is definitely outside the initial viewport.</p>
<img data-od-added-loading data-od-removed-fetchpriority="high" loading="lazy" src="https://example.com/baz.jpg" alt="Baz" width="10" height="10" >
<img data-od-removed-fetchpriority="high" data-od-replaced-loading="eager" src="https://example.com/qux.jpg" alt="Qux" width="10" height="10" loading="lazy">
<img src="https://example.com/quux.jpg" alt="Quux" width="10" height="10" loading="lazy"><!-- This one is all good. -->
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the key test showing how the new functionality is optimizing lazy-loading. Compare this markup with the input markup above.

@@ -553,6 +579,7 @@ static function () use ( $mobile_breakpoint, $tablet_breakpoint ): array {
',
),

// TODO: Eventually the images in this test should all be lazy-loaded, leaving the prioritization to the preload links.
Copy link
Member Author

Choose a reason for hiding this comment

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

@westonruter westonruter marked this pull request as ready for review June 3, 2024 21:21
Copy link

github-actions bot commented Jun 3, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

* @param string|true $value Value.
* @return bool Whether an attribute was set.
*/
public function set_meta_attribute( string $name, $value ): bool {
Copy link
Member

Choose a reason for hiding this comment

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

Not sure I love the term "meta" here, but can't think of a better term

westonruter and others added 2 commits June 3, 2024 15:36
Co-authored-by: Pascal Birchler <pascalb@google.com>
Co-authored-by: Pascal Birchler <pascalb@google.com>
@westonruter westonruter merged commit 229fa72 into trunk Jun 3, 2024
13 checks passed
@westonruter westonruter deleted the add/img-lazy-loading-optimization branch June 3, 2024 23:17
@westonruter westonruter added the [Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) label Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lazy-load images which are not visible in any breakpoint
2 participants