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

Force the react-native-video fork to build against same RN version as gutenberg-mobile from maven repo #1768

Merged
merged 4 commits into from
Jan 10, 2020

Conversation

hypest
Copy link
Contributor

@hypest hypest commented Jan 10, 2020

This reverts commit 4085aeb.

Fixes #1761

It's not clear yet why the issue is happening but, reverting in order to patch and unblock the v1.20 editor release.

Edit: With 39f9f34, I'm pointing to a react-native-video version (here's the branch) that builds using a specific RN version (same as gutenberg-mobile at the moment of writing) and only fetching it from our remote RN maven mirror on Bintray.

To test:

  1. On an Android 7 or 8 (possibly 9 too) device, run WPAndroid against this PR
  2. Have a post with a vertical video in a vide block. Example content:
<!-- wp:paragraph -->
<p>hello world!</p>
<!-- /wp:paragraph -->
<!-- wp:video {"autoplay":false,"id":248,"loop":false,"muted":false,"playsInline":false,"src":"https://tuggytestgmb1.wpcomstaging.com/wp-content/uploads/2020/01/VID_20200109_173921.mp4"} -->
<figure class="wp-block-video"><video controls src="https://tuggytestgmb1.wpcomstaging.com/wp-content/uploads/2020/01/VID_20200109_173921.mp4"></video></figure>
<!-- /wp:video -->
  1. Open the post
  2. Tap on the video block and then its caption
  3. the video preview should be visible and no overflow should be happening

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@hypest hypest added this to the 1.20 milestone Jan 10, 2020
@hypest hypest requested a review from Tug January 10, 2020 10:02
Copy link
Contributor

@Tug Tug left a comment

Choose a reason for hiding this comment

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

Can confirm, this revert fixes it for me 👍

Copy link
Contributor

@Tug Tug left a comment

Choose a reason for hiding this comment

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

Still works with the last commit!

@hypest hypest changed the title Revert "Upgrade the Video lib to fix #1705" Force the react-native-video fork to build against same RN version as gutenberg-mobile from maven repo Jan 10, 2020
Copy link
Contributor

@mchowning mchowning left a comment

Choose a reason for hiding this comment

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

This is fixing the video issue for me as well. :shipit:

@hypest hypest merged commit 706b981 into release/1.20.0 Jan 10, 2020
@hypest hypest deleted the issue/1761-video-overflow-on-android branch January 10, 2020 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants