Skip to content
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.

Sign up for GitHub

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

Initial support for VideoPress v5 #6634

Merged
merged 21 commits into from
Feb 23, 2024
Merged

Initial support for VideoPress v5 #6634

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
1d42425
Update Jetpack reference
Feb 13, 2024
0a56e6a
Update with bundle changes
Feb 13, 2024
b0278b6
Update Jetpack reference
Feb 13, 2024
37609f0
Update Jetpack reference
Feb 13, 2024
139d90e
Update Jetpack reference
Feb 14, 2024
9485b3b
Update Jetpack reference
Feb 14, 2024
a837182
Update Jetpack reference
Feb 16, 2024
ce21e69
Update Jetpack reference
Feb 17, 2024
4369bc9
fix: Limit VideoPress v5 to supported sites
Feb 17, 2024
c5e59b2
Update Jetpack reference
Feb 17, 2024
3479eea
refactor: Be more specific with capability
Feb 17, 2024
897f363
Update Gutenberg reference
Feb 17, 2024
d521cb2
Update Jetpack reference
Feb 20, 2024
8a55c0a
Update RELEASE-NOTES
Feb 20, 2024
e4abec2
Merge branch 'trunk' into jetpack/support-for-videopress-v5
Feb 20, 2024
049f383
Update Jetpack reference
fluiddot Feb 23, 2024
bc69c52
Update localization strings files
fluiddot Feb 23, 2024
79d1f01
Merge branch 'trunk' into jetpack/support-for-videopress-v5
Feb 23, 2024
393ad23
Fix duplicated release note
Feb 23, 2024
07f5e24
Update Gutenberg reference
Feb 23, 2024
9f223de
Update Jetpack reference
Feb 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
Unreleased
---
* [**] Introduce VideoPress v5 support, to fix issues using video block with dotcom and Jetpack sites [https://github.com/wordpress-mobile/gutenberg-mobile/pull/6634]
[*] Prevent crash when autoscrolling to blocks [https://github.com/WordPress/gutenberg/pull/59110]

1.112.0
Expand Down
7 changes: 6 additions & 1 deletion bundle/android/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
<!-- translators: displays audio file extension. e.g. MP3 audio file -->
<string name="gutenberg_native_audio_file" tools:ignore="UnusedResources">audio file</string>
<string name="gutenberg_native_audio_player" tools:ignore="UnusedResources">Audio Player</string>
<string name="gutenberg_native_autoplay_may_cause_usability_issues_for_some_users" tools:ignore="UnusedResources">Autoplay may cause usability issues for some users</string>
<string name="gutenberg_native_block_cannot_be_rendered_because_it_is_deeply_nested_tap_here_for" tools:ignore="UnusedResources">Block cannot be rendered because it is deeply nested. Tap here for more details.</string>
<!-- translators: displayed right after the block is copied. -->
<string name="gutenberg_native_block_copied" tools:ignore="UnusedResources">Block copied</string>
Expand Down Expand Up @@ -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>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 video
  • Video caption. Empty
  • Video caption. %s

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤔.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

<!-- translators: %s: name of the host app (e.g. WordPress) -->
<string name="gutenberg_native_editing_synced_patterns_is_not_yet_supported_on_s_for_android" tools:ignore="UnusedResources">Editing synced patterns is not yet supported on %s for Android</string>
<!-- translators: %s: name of the host app (e.g. WordPress) -->
Expand Down Expand Up @@ -389,10 +391,13 @@ translators: %s: Select control option value e.g: "Auto, 25%". -->
<string name="gutenberg_native_upgrade_your_plan_to_use_video_covers" tools:ignore="UnusedResources">Upgrade your plan to use video covers</string>
<string name="gutenberg_native_uploading" tools:ignore="UnusedResources">Uploading…</string>
<string name="gutenberg_native_use_icon_button" tools:ignore="UnusedResources">Use icon button</string>
<!-- translators: accessibility text. Empty video caption. -->
<string name="gutenberg_native_video_caption_empty" tools:ignore="UnusedResources">Video caption. Empty</string>
<!-- translators: accessibility text. Empty video caption. -->
<string name="gutenberg_native_video_caption_empty_663aab49" tools:ignore="UnusedResources">Video caption. Empty</string>
<!-- translators: accessibility text. %s: video caption. -->
<string name="gutenberg_native_video_caption_s" tools:ignore="UnusedResources">Video caption. %s</string>
<!-- translators: accessibility text. %s: video caption. -->
<string name="gutenberg_native_video_caption_s_0141bd20" tools:ignore="UnusedResources">Video caption. %s</string>
<string name="gutenberg_native_waiting_for_connection" tools:ignore="UnusedResources">Waiting for connection</string>
<string name="gutenberg_native_warning_message" tools:ignore="UnusedResources">Warning message</string>
<string name="gutenberg_native_we_are_working_hard_to_add_more_blocks_with_each_release" tools:ignore="UnusedResources">We are working hard to add more blocks with each release.</string>
Expand Down
1 change: 1 addition & 0 deletions bundle/ios/GutenbergNativeTranslations.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ private func dummy() {
_ = NSLocalizedString("Audio caption. Empty", comment: "translators: accessibility text. Empty Audio caption.")
_ = NSLocalizedString("audio file", comment: "translators: displays audio file extension. e.g. MP3 audio file")
_ = NSLocalizedString("Audio Player", comment: "")
_ = NSLocalizedString("Autoplay may cause usability issues for some users", comment: "")
_ = NSLocalizedString("Block cannot be rendered because it is deeply nested. Tap here for more details.", comment: "")
_ = NSLocalizedString("Block copied", comment: "translators: displayed right after the block is copied.")
_ = NSLocalizedString("Block cut", comment: "translators: displayed right after the block is cut.")
Expand Down
2 changes: 1 addition & 1 deletion gutenberg
Open in desktop
Submodule gutenberg updated 1011 files
2 changes: 1 addition & 1 deletion jetpack
Open in desktop
Submodule jetpack updated 1536 files
14 changes: 13 additions & 1 deletion src/jetpack-editor-setup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
registerLoomVariation,
registerSmartframeVariation,
} from '../jetpack/projects/plugins/jetpack/extensions/extended-blocks/core-embed';
import '../jetpack/projects/plugins/jetpack/extensions/blocks/videopress/editor';

// When adding new blocks to this list please also consider updating `./block-support/supported-blocks.json`
const supportedJetpackBlocks = {
Expand Down Expand Up @@ -129,6 +128,14 @@ export function registerJetpackEmbedVariations( { capabilities } ) {
} );
}

export function enableVideoPressV5Support( { capabilities } ) {
if ( ! isActive() || ! capabilities.videoPressV5Support ) {
return;
}

require( '../jetpack/projects/plugins/jetpack/extensions/blocks/videopress/editor' );
}
Comment on lines +131 to +137
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a side note, it would be great if we could cover this logic in a test case here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I'll add this to my list to follow up on next week.


const setupHooks = () => {
// Hook triggered before the editor is rendered
addAction( 'native.pre-render', 'gutenberg-mobile-jetpack', ( props ) => {
Expand All @@ -142,6 +149,11 @@ const setupHooks = () => {
// block type registration, so it’s required to add them before
// the core blocks are registered.
registerJetpackEmbedVariations( props );

// VideoPress v5 conversion also uses WP hooks that are attached to
// block type registration, so it’s required to add them before
// the core blocks are registered.
enableVideoPressV5Support( props );
} );

// Hook triggered after the editor is rendered
Expand Down