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

WP Stories: changed flow for remote media from WP site to be copied locally #13548

Merged
merged 3 commits into from
Dec 4, 2020

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Dec 4, 2020

With this change, instead of passing a remote URL to StoryComposerActivity through onActivityResult(), we're first making a local copy of it and then passing it over to the composer.

It has proven to have better results and avoid metadata extractor not to be ready when needed in order to calculate the video size and rotation and so on.

I only made this test in the old PhotoPicker. We should follow up on this on the new media picker when it's also used in Stories. EDIT - tracked in #13549

To test:

  1. download this video to a your emulator / handset. (this one has proven to show especial trouble on a Pixel 3 emulator, a Pixel 2 actual device, and a Samsung S5e tablet).
  2. go to the Media section of the app and upload that video from your handset. Follow the next steps when ready.
  3. from the Posts list or from the My Site screen on the app, tap on the FAB and tap on new Story
  4. when the media picker appears, tap on the W option to see the site's media items
  5. you should see the video you recently uploaded. Select that one and then tap on the ✔️ on the top-right corner of the screen
  6. observe a dialog reading "Uploading..." appears briefly (for as long as it takes for the video to get copied), then the Story composer appears and the video starts playing in a loop.
  7. go ahead and add some text to your slide
  8. publish your story
  9. verify it gets saved correctly and it gets published correctly.

Before this change, it would randomly err or crash when trying to save the video.

nice_video

PR submission checklist:

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

@mzorz mzorz added this to the 16.3 ❄️ milestone Dec 4, 2020
@mzorz mzorz requested a review from aforcier December 4, 2020 22:40
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Dec 4, 2020

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Dec 4, 2020

You can test the changes on this Pull Request by downloading the APK here.

@aforcier
Copy link
Contributor

aforcier commented Dec 4, 2020

@mzorz looks like some unit tests need to be updated.

@aforcier
Copy link
Contributor

aforcier commented Dec 4, 2020

This is working well, I followed the test steps and tried various local/wp library image/video uploads for good measure.

:shipit: once CI is green.

@aforcier aforcier merged commit 41d7e74 into fix/stories-for-16.3 Dec 4, 2020
@aforcier aforcier deleted the fix/remote-video-copy branch December 4, 2020 23:12
@mzorz mzorz mentioned this pull request Dec 4, 2020
3 tasks
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.

2 participants