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.

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] Simple View for video view on Android #16136

Conversation

hypest
Copy link
Contributor

@hypest hypest commented Jun 12, 2019

Description

Additional proposal on top of #16089 that demotes the in-block video preview to a simple View instead of the full react-native-video view. This is a temporary workaround until some issues with the view overflowing its boundaries are fixed.

How has this been tested?

Tested using wordpress-mobile/gutenberg-mobile#1117

simple-view-preview

Types of changes

  • Introduced the VideoPlayButton component to render the play button
  • Introduced the video-player.android.js to implement the Android side of the video component, rendering a simple View instead of the proper video preview.
  • Reduced the video-player.native.js module into video-player.ios.js to only hold the iOS side implementation

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@hypest hypest added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Jun 12, 2019
@hypest hypest added this to the Gutenberg 5.10 milestone Jun 12, 2019
@hypest hypest requested a review from pinarol June 12, 2019 23:08
@Soean Soean removed this from the Gutenberg 5.10 milestone Jun 13, 2019
const { isSelected, style } = this.props;

return (
<View style={ styles.videoContainer }>
Copy link
Contributor

Choose a reason for hiding this comment

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

On Gutenberg Web, there is an option to set a poster for the video. Probably we can use that prop this.props.attributes.poster and pass it here to show it as <ImageBackground> so that users can see the poster instead of black view?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, that seems to be part of the extra block settings functionality though, to manually set a poster image. I can be wrong but, it doesn't seem useable for our case where we want to have a preview picture automatically shown, based on a frame from the video.

Copy link
Contributor

Choose a reason for hiding this comment

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

Poster setting opened for public usage pretty recently, so I expect only a tiny amount of videos would come with a poster if any :(

@gziolo
Copy link
Member

gziolo commented Sep 26, 2019

What’s the status of this one? Is it still in active development?

@pinarol
Copy link
Contributor

pinarol commented Dec 13, 2019

@hypest I think we can close this PR as it is not needed anymore?

@hypest
Copy link
Contributor Author

hypest commented Apr 28, 2020

Closing as not relevant at this point.

@hypest hypest closed this Apr 28, 2020
@hypest hypest deleted the rnmobile/open-video-by-browser-for-android-simple-view branch April 28, 2020 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants