-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Video: Add option to set a track as default #70227
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
Video: Add option to set a track as default #70227
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 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. |
Excellent work, @yogeshbhutkar! Maybe we should reduce the spacing for nested Screenshots |
1783e19
to
bd45771
Compare
Thanks for the review, @Mamaduka. I have a few more points regarding the Tracks UI that I'd like to discuss. The Ref. gutenberg/packages/block-library/src/video/tracks-editor.js Lines 118 to 123 in bd45771
In addition to reapplying existing values, the Apply button also sets default values when no input is provided. (I'd consider it to be a surprising behavior) Ref. gutenberg/packages/block-library/src/video/tracks-editor.js Lines 185 to 192 in bd45771
I think instead of language scoping the label, it's best to keep it as I believe it's best to discard the Currently, it's possible to save tracks with empty labels simply by leaving the label blank and clicking outside the popover. In Firefox, these tracks are displayed without any label, which could be confusing. We might want to consider either preventing tracks without labels from being rendered on the front-end or showing a warning if a label is missing. P.S. I plan to address these points as follow-ups, depending on their feasibility. |
Having an 'Apply' button is generally better for accessibility and UX. It's better to keep it.
This is a standard behavior for some popover forms to prevent content loss. We could refactor the track editor and keep the editing state internal, and only call It would be helpful to understand the rationale behind introducing the default values logic. As far as I can tell, the Let's merge this, and feel free to follow up on some of these points in a new PR. |
Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: Zodiac1978 <zodiac1978@git.wordpress.org>
What?
Closes #37537
This PR adds an option to mark a particular track as the default track for the
video
block.Why?
Setting a track as default ensures it's the one automatically selected for playback, providing a consistent user experience.
How?
This is done by setting the
default
property of the selected track totrue
.Testing Instructions
video
block.Testing Instructions for Keyboard
Same.
Screencast
PR-70227.720p.mov