-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Cover: Enable support for adding posters over video #70816
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
Cover: Enable support for adding posters over video #70816
Conversation
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: @VonZubinski, @H0yVoy, @idea--list. 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. |
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.
Very cool! I left one comment that we need to address before we can merge this but other than that I think this works as intended and solves a very old issue :)
2032b9c
to
17c52fd
Compare
I agree with this, @fabiankaegy. The current UI for previewing poster images definitely needs some iteration. That said, this has also been the case for the Video Block, where poster image support has existed for some time now. In my opinion, this PR is simply adding poster image support in line with what's already available in the Video Block. With that in mind, I think it would make sense to handle the design enhancements for both blocks as part of a follow-up issue. What do you think? |
@yogeshbhutkar Very good point! :) I hadn't realized that the video block uses the exact same control. In that case I'm happy with this 👍 :) |
I agree. The current |
+1. Makes sense as a follow-up. |
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.
Thanks for working on this, @yogeshbhutkar!
The new poster image control works as expected, and I couldn't identify any regressions when testing the previous block markup.
A small note: Maybe we should place a new control below the 'Focal point' picker. IMO, seems less distracting and secondary with this order. @fabiankaegy, @t-hamano, what do you think?
Screenshot

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.
Maybe we should place a new control below the 'Focal point' picker. IMO, seems less distracting and secondary with this order.
+1 👍
As a follow-up, it would be great to improve the PosterImage
component of the Video block based on this feedback as well (Correctly place the MediaUploadCheck
, use ref
suffix, etc.).
We got that covered in #70830. |
* Duplicates poster-image from video to cover block.
* Suffix refs with `Ref` * use string overload for `useInstanceId`
* This change should prevent the ToolsPanelItem from rendering unnecessarily.
* Updates the ordering of PosterImage to be displayed after FocalPointPicker * Adds a period to screen-reader text
589307a
to
7b20a93
Compare
The I would like to know your thoughts on whether this is a viable approach and if it requires a follow-up. |
I've not seen any reports requesting the fix for it. While a nice QoL improvement, it doesn't seem to be a significant issue for users. I would say it's a very low priority. |
Unlinked contributors: VonZubinski, H0yVoy, idea--list. Co-authored-by: yogeshbhutkar <[email protected]> Co-authored-by: fabiankaegy <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: Mamaduka <[email protected]> Co-authored-by: donalirl <[email protected]> Co-authored-by: erikjoling <[email protected]> Co-authored-by: strarsis <[email protected]> Co-authored-by: michael-sumner <[email protected]> Co-authored-by: aatospaja <[email protected]>
What?
Closes #18962
This PR adds support for displaying posters over videos within the
Core/Cover
block.Why?
For larger videos and users on slower connections, the inability to set a poster image can result in blocks displaying a blank background, leading to a poor user experience.
How?
The refactorings made in #67044 extracted the
poster-image
component, which is reused in this PR to implement a similar functionality within thecore/cover
block.Testing Instructions
Testing Instructions for Keyboard
Same.
Screencast
pr-demo.mov