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

Save media capture path during orientation changes #16037

Merged

Conversation

mkevins
Copy link
Contributor

@mkevins mkevins commented Mar 3, 2022

Fixes #11296

Description

This PR saves the media capture path when the activity is destroyed, and restores it when it is recreated. This allows the processing of a captured photo to resume when the activity returns to the foreground after an orientation change has taken place. See #11296 (comment) for more details.

To test:

  1. Start in Portrait mode.
  2. Add an Image block and tap "Take a Photo" from the Choose image options.
  3. Once the camera app has loaded, turn the device to Landscape mode.
  4. Tap the on-screen button to take the photo, still in Landscape.
  5. Tap the on-screen "OK" button to accept the photo, still in Landscape.

The photo should upload and be displayed correctly in the editor.

Regression Notes

  1. Potential unintended areas of impact
    N/A

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    N/A

  3. What automated tests I added (or what prevented me from doing so)
    N/A

PR submission checklist:

  • I have completed the Regression Notes.
  • 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.

@mkevins mkevins added the Media label Mar 3, 2022
@mkevins mkevins added this to the 19.4 milestone Mar 3, 2022
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Mar 3, 2022

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

@mkevins mkevins requested a review from ovitrif March 3, 2022 05:15
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Mar 3, 2022

You can test the changes on this Pull Request by downloading the APKs:

Copy link
Contributor

@ovitrif ovitrif left a comment

Choose a reason for hiding this comment

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

Hey @mkevins !

Thank you for fixing this 🙇‍♂️

The code changes LGTM & the fix is working as expected ✅

I'll go ahead and merge the pull request 🚀

@ovitrif
Copy link
Contributor

ovitrif commented Mar 3, 2022

@mkevins I merged latest trunk into the PR branch fix/save-media-capture-path-during-orientation-change to fix the merge conflicts. No manual change was needed from me, so I pushed this to origin.

Let me know if you'd prefer that I don't do this, for me it's "collaboration" but I want to develop the right sensitivity to how such contributions are seen. I already checked with the team lead but I also need your feedback 🙇‍♂️

I'll leave the PR as approved, so you can enable auto-merge and get it merged 👍

Thank you

@mkevins mkevins merged commit 7500809 into trunk Mar 3, 2022
@mkevins mkevins deleted the fix/save-media-capture-path-during-orientation-change branch March 3, 2022 21:40
@mkevins
Copy link
Contributor Author

mkevins commented Mar 3, 2022

to fix the merge conflicts. No manual change was needed from me, so I pushed this to origin.

Thanks Ovi, this is fine, especially since I believe the conflict was only in the release notes. Thanks for reviewing and testing 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GlobalStep] Rotating the device while taking a picture causes it not to be successfully uploaded
2 participants