Skip to content

Commit

Permalink
Set min-height on embed-wrapper instead of figure container
Browse files Browse the repository at this point in the history
  • Loading branch information
@westonruter
westonruter committed Aug 13, 2024
1 parent 250c4e3 commit 04120b9
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 31 deletions.
64 changes: 54 additions & 10 deletions plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,25 +26,64 @@ final class Embed_Optimizer_Tag_Visitor {
*/
protected $added_lazy_script = false;

/**
* Determines whether the processor is currently at a figure.wp-block-embed tag.
*
* @since n.e.x.t
*
* @param OD_HTML_Tag_Processor $processor Processor.
* @return bool Whether at the tag.
*/
private function is_embed_figure( OD_HTML_Tag_Processor $processor ): bool {
return (
'FIGURE' === $processor->get_tag()
&&
true === $processor->has_class( 'wp-block-embed' )
);
}

/**
* Determines whether the processor is currently at a div.wp-block-embed__wrapper tag.
*
* @since n.e.x.t
*
* @param OD_HTML_Tag_Processor $processor Processor.
* @return bool Whether the tag should be measured and stored in URL metrics
*/
private function is_embed_wrapper( OD_HTML_Tag_Processor $processor ): bool {
return (
'DIV' === $processor->get_tag()
&&
true === $processor->has_class( 'wp-block-embed__wrapper' )
);
}

/**
* Visits a tag.
*
* @since 0.2.0
*
* @param OD_Tag_Visitor_Context $context Tag visitor context.
* @return bool Whether the visit or visited the tag.
* @return bool Whether the tag should be measured and stored in URL metrics.
*/
public function __invoke( OD_Tag_Visitor_Context $context ): bool {
$processor = $context->processor;
if ( ! (
'FIGURE' === $processor->get_tag()
&&
true === $processor->has_class( 'wp-block-embed' )
) ) {

/*
* The only thing we need to do if it is a div.wp-block-embed__wrapper tag is return true so that the tag
* will get measured and stored in the URL Metrics.
*/
if ( $this->is_embed_wrapper( $processor ) ) {
return true;
}

// Short-circuit if not a figure.wp-block-embed tag.
if ( ! $this->is_embed_figure( $processor ) ) {
return false;
}

$minimum_height = $context->url_metrics_group_collection->get_element_minimum_height( $processor->get_xpath() );
$embed_wrapper_xpath = $processor->get_xpath() . '/*[1][self::DIV]';
$minimum_height = $context->url_metrics_group_collection->get_element_minimum_height( $embed_wrapper_xpath );
if ( is_float( $minimum_height ) ) {
$style = $processor->get_attribute( 'style' );
if ( is_string( $style ) ) {
Expand All @@ -56,8 +95,7 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool {
$processor->set_attribute( 'style', $style );
}

$max_intersection_ratio = $context->url_metrics_group_collection->get_element_max_intersection_ratio( $processor->get_xpath() );

$max_intersection_ratio = $context->url_metrics_group_collection->get_element_max_intersection_ratio( $embed_wrapper_xpath );
if ( $max_intersection_ratio > 0 ) {
/*
* The following embeds have been chosen for optimization due to their relative popularity among all embed types.
Expand Down Expand Up @@ -131,6 +169,12 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool {
$this->added_lazy_script = true;
}

return true;
/*
* At this point the tag is a figure.wp-block-embed, and we can return false because this does not need to be
* measured and stored in URL Metrics. Only the child div.wp-block-embed__wrapper tag is measured and stored
* so that this visitor can look up the height to set as a min-height on the figure.wp-block-embed. For more
* information on what the return values mean for tag visitors, see <https://github.com/WordPress/performance/issues/1342>.
*/
return false;
}
}
12 changes: 6 additions & 6 deletions plugins/embed-optimizer/tests/test-cases/nested-figure-embed.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => false,
'intersectionRatio' => 1,
),
Expand All @@ -14,7 +14,7 @@
'intersectionRatio' => 1,
),
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[2][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[2][self::FIGURE]/*[1][self::DIV]',
'isLCP' => false,
'intersectionRatio' => 0,
),
Expand Down Expand Up @@ -101,13 +101,13 @@ static function ( OD_Tag_Visitor_Context $context ) use ( $test_case ): bool {
<link data-od-added-tag rel="preload" as="image" href="https://example.com/poster1.jpg">
</head>
<body>
<figure data-od-added-style data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]" style="min-height: 100px;" class="wp-block-embed is-type-video">
<div class="wp-block-embed__wrapper">
<figure data-od-added-style style="min-height: 100px;" class="wp-block-embed is-type-video">
<div data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]" class="wp-block-embed__wrapper">
<video data-od-added-preload data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]/*[1][self::VIDEO]" preload="auto" src="https://example.com/video1.mp4" poster="https://example.com/poster1.jpg" width="640" height="480"></video>
</div>
</figure>
<figure data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[2][self::FIGURE]" data-od-added-style style="min-height: 100px;" class="wp-block-embed is-type-rich is-provider-figurine wp-block-embed-figurine">
<div class="wp-block-embed__wrapper">
<figure data-od-added-style style="min-height: 100px;" class="wp-block-embed is-type-rich is-provider-figurine wp-block-embed-figurine">
<div data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[2][self::FIGURE]/*[1][self::DIV]" class="wp-block-embed__wrapper">
<figure>
<p>So I heard you like <code>FIGURE</code>?</p>
<video data-od-added-preload data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[2][self::FIGURE]/*[1][self::DIV]/*[1][self::FIGURE]/*[2][self::VIDEO]" preload="none" src="https://example.com/video2.mp4" poster="https://example.com/poster2.jpg" width="640" height="480"></video>
Expand Down
6 changes: 3 additions & 3 deletions ...timizer/tests/test-cases/single-spotify-embed-outside-viewport-with-subsequent-script.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => false,
'intersectionRatio' => 0,
),
Expand Down Expand Up @@ -35,8 +35,8 @@
<title>...</title>
</head>
<body>
<figure data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]" data-od-added-style style="min-height: 100px;" class="wp-block-embed is-type-rich is-provider-spotify wp-block-embed-spotify wp-embed-aspect-21-9 wp-has-aspect-ratio">
<div class="wp-block-embed__wrapper">
<figure data-od-added-style style="min-height: 100px;" class="wp-block-embed is-type-rich is-provider-spotify wp-block-embed-spotify wp-embed-aspect-21-9 wp-has-aspect-ratio">
<div data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]" class="wp-block-embed__wrapper">
<iframe data-od-added-loading loading="lazy" title="Spotify Embed: Deep Focus" style="border-radius: 12px" width="100%" height="352" frameborder="0" allowfullscreen allow="autoplay; clipboard-write; encrypted-media; fullscreen; picture-in-picture" src="https://open.spotify.com/embed/playlist/37i9dQZF1DWZeKCadgRdKQ?utm_source=oembed"></iframe>
</div>
</figure>
Expand Down
2 changes: 1 addition & 1 deletion plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => true,
'intersectionRatio' => 1,
),
Expand Down
2 changes: 1 addition & 1 deletion plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => false,
'intersectionRatio' => 0,
),
Expand Down
6 changes: 3 additions & 3 deletions plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => true,
'intersectionRatio' => 1,
),
Expand Down Expand Up @@ -39,8 +39,8 @@
<link data-od-added-tag rel="preconnect" href="https://v0.wordpress.com">
</head>
<body>
<figure data-od-added-style data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]" style="min-height: 100px;" class="wp-block-embed is-type-video is-provider-wordpress-tv wp-block-embed-wordpress-tv wp-embed-aspect-16-9 wp-has-aspect-ratio">
<div class="wp-block-embed__wrapper">
<figure data-od-added-style style="min-height: 100px;" class="wp-block-embed is-type-video is-provider-wordpress-tv wp-block-embed-wordpress-tv wp-embed-aspect-16-9 wp-has-aspect-ratio">
<div data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]" class="wp-block-embed__wrapper">
<iframe title="VideoPress Video Player" aria-label=\'VideoPress Video Player\' width=\'750\' height=\'422\' src=\'https://video.wordpress.com/embed/vaWm9zO6?hd=1&amp;cover=1\' frameborder=\'0\' allowfullscreen allow=\'clipboard-write\'></iframe>
<script src=\'https://v0.wordpress.com/js/next/videopress-iframe.js?m=1674852142\'></script>
</div>
Expand Down
6 changes: 3 additions & 3 deletions plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-outside-viewport.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => false,
'intersectionRatio' => 0,
),
Expand Down Expand Up @@ -35,8 +35,8 @@
<title>...</title>
</head>
<body>
<figure data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]" data-od-added-style style="min-height: 100px;" class="wp-block-embed is-type-video is-provider-wordpress-tv wp-block-embed-wordpress-tv wp-embed-aspect-16-9 wp-has-aspect-ratio">
<div class="wp-block-embed__wrapper">
<figure data-od-added-style style="min-height: 100px;" class="wp-block-embed is-type-video is-provider-wordpress-tv wp-block-embed-wordpress-tv wp-embed-aspect-16-9 wp-has-aspect-ratio">
<div data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]" class="wp-block-embed__wrapper">
<iframe data-od-added-loading loading="lazy" title="VideoPress Video Player" aria-label=\'VideoPress Video Player\' width=\'750\' height=\'422\' src=\'https://video.wordpress.com/embed/vaWm9zO6?hd=1&amp;cover=1\' frameborder=\'0\' allowfullscreen allow=\'clipboard-write\'></iframe>
<script data-od-added-type type="application/vnd.embed-optimizer.javascript" src=\'https://v0.wordpress.com/js/next/videopress-iframe.js?m=1674852142\'></script>
</div>
Expand Down
2 changes: 1 addition & 1 deletion plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => true,
'intersectionRatio' => 1,
),
Expand Down
2 changes: 1 addition & 1 deletion plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => false,
'intersectionRatio' => 0,
),
Expand Down
4 changes: 2 additions & 2 deletions plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ static function ( OD_Tag_Visitor_Context $context ) use ( $test_case ): bool {
<title>...</title>
</head>
<body data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]">
<figure data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]" class="wp-block-embed is-type-video is-provider-wordpress-tv wp-block-embed-wordpress-tv wp-embed-aspect-16-9 wp-has-aspect-ratio">
<div class="wp-block-embed__wrapper">
<figure class="wp-block-embed is-type-video is-provider-wordpress-tv wp-block-embed-wordpress-tv wp-embed-aspect-16-9 wp-has-aspect-ratio">
<div data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]" class="wp-block-embed__wrapper">
<iframe title="VideoPress Video Player" aria-label=\'VideoPress Video Player\' width=\'750\' height=\'422\' src=\'https://video.wordpress.com/embed/vaWm9zO6?hd=1&amp;cover=1\' frameborder=\'0\' allowfullscreen allow=\'clipboard-write\'></iframe>
<script src=\'https://v0.wordpress.com/js/next/videopress-iframe.js?m=1674852142\'></script>
</div>
Expand Down

0 comments on commit 04120b9

Please sign in to comment.