Initial support for VideoPress v5 #6634
Conversation
There was a problem hiding this comment.
Approved via WordPress/gutenberg#59144 (review) (and awaiting feedback from Automattic/jetpack#35637)
There was a problem hiding this comment.
LGTM 🎊 !
I noticed we generated duplicated localization strings. This is expected because those strings are associated with the Jetpack plugin (i.e. a different GlotPress project). However, it would be interesting to review them in case we can use the translation already included in the web version (Automattic/jetpack#35637 (comment)).
| export function enableVideoPressV5Support( { capabilities } ) { | ||
| if ( ! isActive() || ! capabilities.videoPressV5Support ) { | ||
| return; | ||
| } | ||
|
|
||
| require( '../jetpack/projects/plugins/jetpack/extensions/blocks/videopress/editor' ); | ||
| } |
| @@ -160,6 +161,7 @@ | |||
| <string name="gutenberg_native_edit_media" tools:ignore="UnusedResources">Edit media</string> | |||
| <string name="gutenberg_native_edit_using_web_editor" tools:ignore="UnusedResources">Edit using web editor</string> | |||
| <string name="gutenberg_native_edit_video" tools:ignore="UnusedResources">Edit video</string> | |||
| <string name="gutenberg_native_edit_video_024aee6d" tools:ignore="UnusedResources">Edit video</string> | |||
There was a problem hiding this comment.
The following strings are duplicated on Android, however, they are not on iOS. Seems that form some reason, the i18n update scripts are identifying these strings as new. I'd like to explore this further but in the meantime, I wouldn't block the PR due to this.
Edit videoVideo caption. EmptyVideo caption. %s
There was a problem hiding this comment.
One option I'm considering is that since they are associated with the Jetpack plugin, they should be considered different from the ones provided in Gutenberg. However, I noticed that something similar is happening on the Jetpack VideoPress plugin (example) and they weren't marked as new 🤔.
There was a problem hiding this comment.
Thank you for flagging this, Carlos. I'm going to go ahead to begin the merge domino for this PR, and it's associated changes as you mentioned this is non-blocking. But, will be happy to help look further into this specific issue with you next week.
Related PRs
Gutenberg: RNMobile: Add new Video block capability WordPress/gutenberg#59144Jetpack: [RNMobile] Initial support for VideoPress v5 Automattic/jetpack#35637Android: Initial support for VideoPress v5 WordPress-Android#20181iOS: Initial support for VideoPress v5 WordPress-iOS#22602Testing
Please refer to the Jetpack PR as the central place for these changes, with the most up-to-date testing instructions.
PR submission checklist: