-
Notifications
You must be signed in to change notification settings - Fork 800
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
[RNMobile] Add MVP for VideoPress block v5 #29256
Conversation
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped. Jetpack plugin:
|
Are you an Automattician? You can now test your Pull Request on WordPress.com. On your sandbox, run
to get started. More details: p9dueE-5Nn-p2 |
4f6b00e
to
70e9d3f
Compare
70e9d3f
to
cc05da0
Compare
The metadata is used to transform the Video block into v5, as well as for calculate the video URL with the VideoPress token for private videos.
Component `edit-common-settings` is only defined for the native platform in Jetpack, hence we no longer need the logic related to web inherited from the core Video block.
cc05da0
to
64537c9
Compare
This PR has been marked as stale. This happened because:
No further action is needed. But it's worth checking if this PR has clear testing instructions, is it up to date with trunk, and it is still valid. Feel free to close this PR if you think it's not valid anymore — if you do, please add a brief explanation. |
This PR has been automatically closed as it has not been updated in some time. If you want to resume work on the PR, feel free to restore the branch and reopen the PR. |
Related PRs:
Fixes wordpress-mobile/gutenberg-mobile#5497.
Proposed changes:
The idea of this PR is to support VideoPress block v5 to address wordpress-mobile/gutenberg-mobile#5497 by reusing as much as possible the current implementation of the core Video block. Since the issue only affects iOS, this version will only be enabled on iOS for now. On Android, we'll keep using the core Video block.
The main changes introduced in this PR are:
guid
(i.e. VideoPress video ID) based if it can be inferred from the URL insrc
attribute. This is done upon block mounting or when thesrc
attributes changes. The idea behind this logic is to automatically migrate from the core Video block format to VideoPress when using a VideoPress URL.metadata_token
query parameter in the video URL.Other information:
Jetpack product discussion
N/A
Does this pull request change what data or activity we track or use?
N/A
Testing instructions:
Part of the testing requires to check the HTML produced by the block. Here are examples of the block's HTML by version:
Video block V5 - (Simple site/Self-hosted) [🍎 iOS]:
Core Video block - Simple site [🤖 Android]:
Core Video block - Self-hosted [🍎 iOS/ 🤖 Android]:
1 - Simple sites
Preparation:
Since the PR addresses the issue with private videos, we need to ensure to test using private videos.
NOTE: Alternatively, a public site can be used by changing the privacy of videos to Private after upload.
1.1 - 🍎 iOS - v5
Upload within the editor:
Upload outside the editor:
Take a video:
Insert from Media library:
Replace video:
Transform block:
NOTE: When the post is saved, the
src
attribute is removed from the block. This means that transforming the block into Cover/File/Media & Text will result in an empty block.1.2 - 🤖 Android - core Video block
Follow the same test cases of iOS, but after adding the video check follow these steps:
2 - Self-hosted sites with Jetpack
Preparation - attachment and VideoPress videos:
Once VideoPress is enabled, all video uploads will use VideoPress. In order to test the block, it's important to also use videos uploaded to the site.
Preparation - private videos:
Similar to Simple site, we need to ensure to test using private videos. However, since self-hosted sites can't be private, we need to enable video privacy:
NOTE: Alternatively, the video privacy can be changed on each video via editing video details after upload.
2.1 - 🍎 iOS - v5
NOTE: Free users can only have one video in VideoPress, so keep this in mind when testing. In case you already uploaded a video, you can delete it and upload a new one. Alternatively, you can purchase the VideoPress plan to have unlimited uploads.
Upload within the editor - success:
Upload within the editor - error:
Upload outside the editor - success:
Upload outside the editor - error:
Take a video:
Insert from Media library - VideoPress video:
Insert from Media library - Attachment:
Replace video:
Transform block:
2.2 - 🤖 Android - core Video block
Follow the same test cases of iOS, but after adding the video check follow these steps:
3 - Self-hosted sites without Jetpack
VideoPress is only available with Jetpack, so in all test cases we should check that the block's format is Core Video block - Self-hosted.