[Mobile]Update video player to open the URL by browser on Android #16089
Conversation
pinarol
added
the
Mobile App - i.e. Android or iOS
pinarol
changed the title
| @@ -13,7 +13,7 @@ $play-icon-size: 50; | |||
| align-items: center; | |||
| align-self: center; | |||
| position: absolute; | |||
| background-color: #e9eff3; | |||
| background-color: transparent; | |||
There was a problem hiding this comment.
not directly related with this PR but updated this intentionally, there was a whitish color with 0.3 opacity on the videos which made videos look like they lost saturation.
| openURL( url ) { | ||
| Linking.canOpenURL(url).then(( supported ) => { | ||
| if ( !supported ) { | ||
| console.warn("Can't open the video URL: " + url); |
There was a problem hiding this comment.
On Android there is also an option to present the bottom sheet with the list of apps that can open the video, maybe that could be a good fall back scenario?
There was a problem hiding this comment.
Got the explanation from @pinarol, so we are good with the current solution.
There was a problem hiding this comment.
AFAIU, if there are apps that can open the url then supported flag won't be false ever. https://facebook.github.io/react-native/docs/linking#canopenurl
There was a problem hiding this comment.
Btw, on WPAndroid Aztec, when Chrome is disabled, it pops up the message for the user: "No application can handle this request. Please install a Web browser", maybe we can do the same so that user knows the reason why video can't be opened?
There was a problem hiding this comment.
Sounds good 👍 added here 941917f
|
Ready for another look @marecar3
I had to manipulate the code to be able to see the alert messages, but the best way to test might be removing the browser from phone. |
|
I added a last commit to prevent play button getting visible before our player has a height
I also added this as a code comment but I'll put more info here. Normally we wouldn't expect such a thing happening if we could add the play button as a child component to the |
|
I updated the base branch because of this wordpress-mobile/gutenberg-mobile#1117 (comment) |
…rnmobile/open-video-by-browser-for-android * 'master' of https://github.com/WordPress/gutenberg: (34 commits) [RNMobile] Native mobile release v1.7.0 (#16156) Scripts: Document and simplify changing file entry and output of build scripts (#15982) Block library: Refactor Heading block to use class names for text align (#16035) Make calendar block resilient against editor module not being present (#16161) Bump plugin version to 5.9.1 Editor: Fix the issue where statics for deprecated components were not hoisted (#16152) Build Tooling: Use "full" `npm install` for Build Artifacts Travis task (#16166) Scripts: Fix naming conventions for function containing CLI keyword (#16091) Add native support for Inserter (Ported from gutenberg-mobile) (#16114) docs(components/higher-order/with-spoken-messages): fix issue in example code (#16144) docs(components/with-focus-return): fix typo in README.md (#16143) docs(block-editor/components/inspector-controls): fix image path in README.md (#16145) Add mention of on Figma to CONTRIBUTING.md (#16140) Bring greater consistency to placeholder text for media blocks. (#16135) Add descriptive text and a link to embed documentation in embed blocks (#16101) Update toolbar-text.png (#16102) Updating changelogs for the Gutenberg 5.9 packages release chore(release): publish [RNMobile] Fix pasting text on Post Title (#16116) Bump plugin version to 5.9.0 ... # Conflicts: # packages/block-library/src/video/video-player.android.js
Description
To Fix: wordpress-mobile/gutenberg-mobile#1119
How has this been tested?
Tested with the steps on gutenberg-mobile PR: wordpress-mobile/gutenberg-mobile#1117
Screenshots
Types of changes
This should change only Android. We are canceling out the
react-native-video's player controls and replacing it with system browser.Since Android and iOS players got very close to each other I got rid of video-player.android.js.
Checklist: