Prioritize loading poster image of video LCP elements#1498
Prioritize loading poster image of video LCP elements#1498westonruter merged 10 commits intotrunkfrom
Conversation
thelovekesh
added
[Type] Feature
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
TODO: Add test cases. |
|
Yeah, sounds good. |
… add/lcp-poster-image-prioritize
westonruter
added
[Type] Enhancement
| * @param string $attribute_name Attribute name. | ||
| * @return string|true|null Normalized attribute value. | ||
| */ | ||
| protected function get_attribute_value( OD_HTML_Tag_Processor $processor, string $attribute_name ) { |
There was a problem hiding this comment.
I'd like to move this eventually into OD_HTML_Tag_Processor/OD_HTML_Processor. Also @dmsnell has suggested this could eventually be part of the core class as well, as most HTML attributes have their own dedicated logic to get back a normalized value.
| * | ||
| * @return bool Whether the tag should be tracked in URL metrics. | ||
| */ | ||
| public function __invoke( OD_Tag_Visitor_Context $context ): bool { |
There was a problem hiding this comment.
@westonruter I suppose this is where I would add the logic for #1575?
|
|
||
| $xpath = $processor->get_xpath(); | ||
|
|
||
| // TODO: If $context->url_metric_group_collection->get_element_max_intersection_ratio( $xpath ) is 0.0, then the video is not in any initial viewport and the VIDEO tag could get the preload=none attribute added. |
There was a problem hiding this comment.
Thinking out loud for a future enhancement:
autoplay takes precedence over preload. Since we here know that the video is not in the initial viewport, we could remove autoplay in favor of an IntersectionObserver-based implementation that only autoplays the video when actually visible.
Thoughts? Worth creating an issue for it?
There was a problem hiding this comment.
Oh, interesting idea. Yes, I think that warrants an issue.
Related to #1035 which involves IntersectionObserver to lazy-load background images.
swissspidy
left a comment
swissspidy
left a comment
There was a problem hiding this comment.
Overall looks pretty good, seems like a great base for further enhancements like #1575
3 participants
Summary
Fixes #1183
Relevant technical choices