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

Video block #687

Closed
Tug opened this issue Feb 28, 2019 · 3 comments

Comments

@Tug
Copy link
Contributor

commented Feb 28, 2019

Add support for Video block

@Tug Tug created this issue from a note in Mobile Gutenberg (More blocks) Feb 28, 2019

@Tug Tug added the Blocks label Feb 28, 2019

@koke koke moved this from More blocks to Sprint #28 in Mobile Gutenberg Mar 21, 2019

@pinarol pinarol moved this from Sprint #28 to In progress in Mobile Gutenberg Mar 27, 2019

@pinarol

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

TO DO items to go publicly available:

  • (First iteration)Make a basic video block that can upload & insert media - merged #855
  • UX/UI enhancements - #966 PR: WordPress/gutenberg#15551 merged
  • Handle invalid html content resulting from the Jetpack plugin (more context available in the first iteration PR ☝️) - merged
  • Remove the unregistration of video block

TODO items next:

  • Update sidebar at: https://mobilegutenberg.wordpress.com to show that it is included in next version
  • [Android] Video disappears from video block after turning off the device screen: #974 <<not reproducible on every device
  • Android only issue: there is a split second before the placeholder/uploading state shows, where the inner block collapses. Can we make sure the placeholder shows immediately, so there isn't a jump details here: #979 (comment) Opened issue to track this #1005
  • Solve react-native-video bug about the poster. If you set a poster url then this happens: react-native-community/react-native-video#1509 We have opened an issue to track it on our side: #965 we are planning to solve this on our fork. But I think this is not a blocker for making the video block publicly available. Video settings are not yet available on wp.com so users won't be able to set posters yet. And even if they do, we can avoid the anomaly by simply not using the poster url on mobile as a workaround. Currently we removed poster setting on mobile side to avoid any possible issues.
  • Add unit-tests for touched areas( both image and video block ) - ready for review but need to merge the main PR first #919
  • Add some simple ui tests to cover basic upload scenarios
  • Add support for Video settings
  • Enhance video block to show videos with better aspect ratio rather than a fixed one
  • Retry options should have a cancel button (details :#855)
  • Stopping an upload-in-progress, quote: I would expect when the upload finishes behind the ActionSheet that it would simply close the action sheet because those actions are no longer relevant.(details :#855)
  • Don't save local uri in attributes, details: #854 (review)
  • Tech debt: media-upload-progress.native.js can be moved to a better place WordPress/gutenberg#14912 (comment)
  • Try and look for ways to make it cross platform
@pinarol

This comment has been minimized.

Copy link
Contributor

commented May 10, 2019

Note about video playing differences between platforms

Currently on iOS we have a pull page video play, on Android we have inline.

This is because we have limitations on react-native-video level. Having control buttons(like play, pause etc) in the video block are messing up the layout on iOS, so I had to hide them and add our own play button which opens the video on full page. Once the video opens full page then there's no problem about control buttons. A bit of a technical insight: I think this is the line responsible for it, they are setting the frame (as a workaround for some issue) when controls=true
https://github.com/react-native-community/react-native-video/blob/c30f246f5694452f4f081ff30ac4523dfd89f41d/ios/Video/RCTVideo.m#L593
And by doing that they are messing up the y position of our page.

On the other hand on Android the full page preview just doesn’t work, the method call does nothing. But control buttons do work correctly. The video can play inline.

So now, we have different user experiences in both platforms in terms of video playing. If we want to make this equal in the future we'll need to either solve one of this:

  1. Android the full page preview doesn’t work
  2. iOS controls=true messing up the layout
@hypest

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2019

The block has been released for both platforms. Closing this.

@hypest hypest closed this Jul 22, 2019

Mobile Gutenberg automation moved this from In progress to Done Jul 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.