[RNMobile] Update native Video block to check if MediaPlaceholder should be displayed#49858
Merged
derekblank merged 2 commits intotrunkfrom Apr 19, 2023
Merged
Conversation
derekblank
added
Mobile App - i.e. Android or iOS
|
Size Change: -171 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
dcalhoun
reviewed
Apr 18, 2023
Member
dcalhoun
left a comment
dcalhoun
left a comment
There was a problem hiding this comment.
Thanks for putting this together! The test plan succeeded when using a iPhone SE and Samsung Galaxy S20 FE 5G.
I left a comment for us to consider before merging.
dcalhoun
approved these changes
Apr 19, 2023
Member
dcalhoun
left a comment
dcalhoun
left a comment
There was a problem hiding this comment.
Looks good! Thank you. I tested each Video block media selection option with an iPhone SE and Samsung Galaxy S20 FE 5G, I did not encounter any regressions.
This was referenced Apr 19, 2023
3 tasks
This was referenced Jan 18, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Related:
Fixes:
Updates a conditional on the native Video block to determine if a video source exists in order to display MediaPlaceholder under the correct logic.
Why?
Currently, the web and native versions of the Video block use different properties to determine if the MediaPlaceholder component should be displayed:
gutenberg/packages/block-library/src/video/edit.js
Lines 142 to 145 in 4ed1c11
gutenberg/packages/block-library/src/video/edit.native.js
Lines 239 to 242 in 4ed1c11
Although the edit.native.js Video block file is setting the
idattribute to be the new video URL here, the web equivalent edit.js file does not set theidas the URL. In fact, it sets theidandposterattributes asundefined:Currently, when a user adds a video from a URL via mobile and saves the post, no placeholder image will be displayed. The lack of an id results in the display of the early return function in the native file, and perceived content loss for the user, even though the video will be displayed correctly on the web editor and on the post itself. Further detail referenced here: #42515 (comment)
How?
To preserve the web behavior and also eliminate the perceived content loss on mobile, this PR modifies the logic of the early return function to check for a
srcattribute as well, which matches the web behavior more closely.Testing Instructions
To test #42515: Saving a Video block with a file URL displays placeholder text in the editor
http://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ForBiggerFun.mp4To test #42443: Showing the play button after inserting URL that doesn't exist
aaaaaaPlaceholder vs Poster image
Note: this change will allow the mobile app to display a placeholder image, which is distinct from a poster image. The placeholder image will always be the first frame from a given video source URL. A poster image is a specific frame of a video that can be selected from the web editor. Currently, the mobile editor does not allow a user to select the poster image, or display the selected poster image. That work is captured in a separate issue:
Also note that when testing this issue with the common community example of using Big Buck Bunny as a test source, the first frame of Big Buck Bunny is a black screen, and is what will be displayed as the placeholder image. To test with a video URL example that does not have a black screen as the first frame, the following URL can be used:
Screenshots or screencast
Before ❌
Video.Block.Not.Working.mov
After ✅
Video.Block.Working.mov