-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Video: Fix track editor state saves without explicitly applying the changes #70628
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: Fix track editor state saves without explicitly applying the changes #70628
Conversation
* Also contains logic to simplify writing defaults
| const DEFAULT_TRACK = { | ||
| src: '', | ||
| label: '', | ||
| srcLang: 'en', |
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.
I intended to omit the language-specific srcLang attribute from the previous implementation; however, according to the MDN reference:
If the
kindattribute is set to "subtitles", thensrclangmust be specified.
Even when the kind attribute is omitted, it defaults to "subtitles", which still requires srclang to be defined.
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.
Makes sense 👍
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.
@Mamaduka, can we move this PR forward if the implementation looks good? 😅
|
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. |
Mamaduka
left a comment
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.
Thank you, @yogeshbhutkar!
The refactoring works as expected ✅
…hanges (#70628) Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
What?
Follow-up to #70227 (comment)
This PR implements an internal editing state for
SingleTrackEditorand uses theonChangeto write the state to attributes only whenApplyis pressed.Why?
Previously, for every change in value, the
onChangewould execute, writing the state changes to attributes as they occurred; this rendered theApplybutton useless.Removing the
Applybutton itself was previously discussed in the tagged PR, but it was decided to keep it for better Accessibility and UX.How?
An internal state is maintained, and the state is synced with attributes when the changes are applied; otherwise, they are discarded.
Testing Instructions
Media Library.Applyand then close the modal.Testing Instructions for Keyboard
Same.
Screencast
pr-demo.mov