Cover: add support for background videos as embeds. #73023
Merged
Conversation
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 extends background video support to other core video embeds, like YouTube, Vimeo, etc.
mtias
requested review from
ajitbohra,
fabiankaegy and
juanmaguitar
as code owners
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @amandablum, @ivcreative, @badlydrawnben, @mammothsolutions, @miriamardent, @kblizeck, @losbenos, @LetterAfterZ, @scrybbler, @nebmotion. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
|
Size Change: +1.4 kB (+0.06%) Total Size: 2.47 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 26d9e4c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19214336552
|
youknowriad
reviewed
Nov 5, 2025
youknowriad
reviewed
Nov 5, 2025
youknowriad
reviewed
Nov 5, 2025
| ); | ||
|
|
||
| setAttributes( { | ||
| embedSrc: backgroundVideoSrc, |
Contributor
There was a problem hiding this comment.
This is a bit weird, why are we loading the iframe of the embed extraction the src ... Can't we just render the iframe directly (like we do in the frontend I guess)
Contributor
There was a problem hiding this comment.
I guess we should check how the embed preview works in the embed block and do the same.
Contributor
There was a problem hiding this comment.
Looking at the Embed block looks the logic is a bit involved and implemented in EmbedPreview. I wonder if we should have a similar logic using the Sandbox component.
Member
There was a problem hiding this comment.
Alternatively, maybe all this processing can happen in EmbedVideoUrlInput or in the submit callback.
The embed block has to do it during rendering as a side effect, because it deals with embeddable URLs and converts them into iframes, which mimics how WordPress handles this on the front end.
youknowriad
reviewed
Nov 5, 2025
youknowriad
reviewed
Nov 5, 2025
|
Loving it so far! The default overlay opacity is 100, making it impossible to see the video if you don't know how to adjust it. |
|
Once the video is added, replacing it with a new one becomes difficult because the video URL field stays empty. The user can swap a video for another by first resetting the cover block. |
mtias
added
the
[Block] Cover
youknowriad
reviewed
Nov 6, 2025
| // Use WordPress's native oEmbed processing (includes caching). | ||
| $oembed_html = wp_oembed_get( $url ); | ||
|
|
||
| if ( $oembed_html ) { |
Contributor
There was a problem hiding this comment.
I like that this logic is contained now, I think probably we could also guess the embedProvider from the html like we do in frontend to avoid the extra attribute.
Contributor
|
Looks like YouTube is broken on the editor but works on the frontend. |
Contributor
Do we need to add an option to the placeholder? |
youknowriad
reviewed
Nov 9, 2025
youknowriad
reviewed
Nov 9, 2025
| { isEmbedVideoBackground && url && ( | ||
| <figure | ||
| className={ clsx( | ||
| 'wp-block-cover__video-background', |
Contributor
There was a problem hiding this comment.
Is this classname needed or should we clearly separate the "embed" from the "video" background rendering/classes?
youknowriad
reviewed
Nov 9, 2025
Contributor
youknowriad
left a comment
youknowriad
left a comment
There was a problem hiding this comment.
This is looking good to me in terms of flow / code / behavior. I don't know if we could fix the YouTube preview in the editor. Other than that I left small comments but no big things.
Member
Author
I thought it was a bit much, so we could start by adding the support in the dropdown only. |
Contributor
|
Ok I tested this on playground and it works great. Youtube has issues when it comes to localhost. |
youknowriad
approved these changes
Nov 10, 2025
Contributor
youknowriad
left a comment
youknowriad
left a comment
There was a problem hiding this comment.
I think this is working great for me. IT would have been great if oembed's supported the "no buttons" approach more easily but the code is contained and can be improved later.
Member
Author
|
Thanks for the review! |
bph
added
[Type] Enhancement
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #28860
This extends background video support in
core/coverto other core video embeds, like YouTube, Vimeo, etc.Implementation notes: