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

Do not crop after pick cancelled #171

Merged
merged 4 commits into from
Jul 29, 2021
Merged

Do not crop after pick cancelled #171

merged 4 commits into from
Jul 29, 2021

Conversation

tikitu
Copy link
Contributor

@tikitu tikitu commented Jul 29, 2021

close #162

Bug

Cause:

After cropping a file is left on the file system for later use. The picker contract doesn't respect cancellation, so if it finds that file it will (incorrectly) return it for cropping even though the user intended to cancel file selection.

Reproduce

  1. Open Camera
  2. Capture a Image
  3. Crop a image
  4. Open crop selector again
  5. Don't select any image
  6. return

How the bug was solved:

Discussion on #162 suggested removing the file after cropping. However it seems to me that we can't know when to do this safely: because we return a URI to this file, we can't know whether the library's client has finished using it or not. Instead I made the picker contract explicitly respect cancellation: when cancelled, it returns no URI even if there is a file available.

@tikitu tikitu requested a review from a team as a code owner July 29, 2021 11:00
@tikitu
Copy link
Contributor Author

tikitu commented Jul 29, 2021

The test is rather low-level, but I believe I've confirmed using the Sample app that this indeed solves the user-level problem.

@Canato
Copy link
Member

Canato commented Jul 29, 2021

This PR is beauty 🤩

Will check later today/tomorrow.

@Canato Canato self-assigned this Jul 29, 2021
@tikitu tikitu requested a review from Canato July 29, 2021 12:19
@Canato Canato merged commit 3496b54 into CanHub:main Jul 29, 2021
@tikitu tikitu deleted the do-not-crop-after-pick-cancelled branch July 30, 2021 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] - Camera crop image activity is not removed
2 participants