-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Fix layout shift caused by video
tag in Video block lacking width
and height
#70293
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
Open
westonruter
wants to merge
6
commits into
trunk
Choose a base branch
from
fix/video-block-layout-shift
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+95
−1
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
362ee04
Prevent layout shift in Video block when width/height metadata is ava…
westonruter 7fb986d
Add todo for capturing dimensions in JS to persist in block attributes
westonruter 87eca02
Only set auto:height on videos with width and height set
westonruter a89350f
Use height:auto regardless of whether the width/height attributes wer…
westonruter c048eb8
Add explanation for why inline style is added
westonruter cf37f1c
Fix syntax of attr()
westonruter File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
<?php | ||
/** | ||
* Server-side rendering of the `core/video` block. | ||
* | ||
* @package WordPress | ||
*/ | ||
|
||
/** | ||
* Renders the `core/video` block on the server to supply the width and height attributes from the attachment metadata. | ||
* | ||
* @since 0.0.1 | ||
* | ||
* @phpstan-param array{ "id"?: positive-int } $attributes | ||
* | ||
* @param array $attributes The block attributes. | ||
* @param string $content The block content. | ||
* @return string The block content with the dimensions added. | ||
*/ | ||
function render_block_core_video( array $attributes, string $content ): string { | ||
// if the content lacks any video tag, abort. | ||
if ( ! str_contains( $content, '<video' ) ) { | ||
return $content; | ||
} | ||
|
||
// If the 'id' attribute is not populated for a video attachment, abort. | ||
if ( | ||
! isset( $attributes['id'] ) || | ||
! is_int( $attributes['id'] ) || | ||
$attributes['id'] <= 0 | ||
) { | ||
return $content; | ||
} | ||
|
||
// If the 'id' attribute wasn't for an attachment, abort. | ||
if ( get_post_type( $attributes['id'] ) !== 'attachment' ) { | ||
return $content; | ||
} | ||
|
||
// Get the width and height metadata for the video, and abort if absent or invalid. | ||
$metadata = wp_get_attachment_metadata( $attributes['id'] ); | ||
if ( | ||
! isset( $metadata['width'], $metadata['height'] ) || | ||
! ( is_int( $metadata['width'] ) && is_int( $metadata['height'] ) ) || | ||
! ( $metadata['width'] > 0 && $metadata['height'] > 0 ) | ||
) { | ||
return $content; | ||
} | ||
|
||
// Locate the VIDEO tag to add the dimensions. | ||
$p = new WP_HTML_Tag_Processor( $content ); | ||
if ( ! $p->next_tag( array( 'tag_name' => 'VIDEO' ) ) ) { | ||
return $content; | ||
} | ||
|
||
$p->set_attribute( 'width', (string) $metadata['width'] ); | ||
$p->set_attribute( 'height', (string) $metadata['height'] ); | ||
|
||
/* | ||
* The aspect-ratio style is needed due to an issue with the CSS spec: <https://github.com/w3c/csswg-drafts/issues/7524>. | ||
* Note that a style rule using attr() like the following cannot currently be used: | ||
* | ||
* .wp-block-video video[width][height] { | ||
* aspect-ratio: attr(width type(<number>)) / attr(height type(<number>)); | ||
* } | ||
* | ||
* This is because this attr() is yet only implemented in Chromium: <https://caniuse.com/css3-attr>. | ||
*/ | ||
$style = $p->get_attribute( 'style' ); | ||
if ( ! is_string( $style ) ) { | ||
$style = ''; | ||
} | ||
$aspect_ratio_style = sprintf( 'aspect-ratio: %d / %d;', $metadata['width'], $metadata['height'] ); | ||
$p->set_attribute( 'style', $aspect_ratio_style . $style ); | ||
|
||
return $p->get_updated_html(); | ||
} | ||
|
||
/** | ||
* Registers the `core/video` block on server. | ||
* | ||
* @since 0.0.1 | ||
*/ | ||
function register_block_core_video(): void { | ||
register_block_type_from_metadata( | ||
__DIR__ . '/video', | ||
array( | ||
'render_callback' => 'render_block_core_video', | ||
) | ||
); | ||
} | ||
add_action( 'init', 'register_block_core_video' ); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
|
||
.video { | ||
width: 100%; | ||
height: auto; | ||
} | ||
|
||
.videoContainer { | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
box-sizing: border-box; | ||
video { | ||
width: 100%; | ||
height: auto; | ||
vertical-align: middle; | ||
} | ||
|
||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this, the
aspect-ratio
doesn't cause the height of the element to be the intrinsic height based on the resized width. Notice the white bars on the top and bottom of the video, causes by the defaultobject-fit:contain
styles whenheight:auto
is absent:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally proposed in #37052 as a follow-up to #30092