Skip to content
This repository was archived by the owner on Feb 24, 2025. It is now read-only.

supply privacy info for image permission dialog - #185

Merged
brindy merged 1 commit into
developfrom
feature/fix_save_image_crash
Nov 6, 2017
Merged

brindy merged 1 commit into
developfrom
feature/fix_save_image_crash

Conversation

@brindy

@brindy brindy commented Nov 6, 2017 •

Copy link
Copy Markdown
Contributor

Reviewer: Mia
Asana: https://app.asana.com/0/414235014887631/470959007490761
CC: Cain

Description:

Simply add the privacy statement for adding images to the user's photo library.

Note that once the user grants or denies permission they will have to change this in Settings -> Privacy -> Photo Library. We should probably check for permission before showing the menu and hide "Save Image" if it is denied (or show some messaging to tell the user to update their permissions in settings) but that seems low priority for now.

Steps to test this PR:

Test 1

  1. Navigate to an image
  2. Long press the image and choose save
  3. Note the privacy reason in the permission dialog and deny the request
  4. Confirm the image is NOT in the photo library

Test 2

  1. Reinstall the app (to clear any stored permissions)
  2. Navigate to an image
  3. Long press the image and choose save
  4. Note the privacy reason in the permission dialog and accept the request
  5. Confirm the image is in the photo library
Reviewer Checklist:
  • Ensure the PR solves the problem
  • Review every line of code
  • Ensure the PR does no harm by testing the changes thoroughly
  • Get help if you're uncomfortable with any of the above!
  • Determine if there are any quick wins that improve the implementation
PR DRI Checklist:
  • Get advice or leverage existing code
  • Agree on technical approach with reviewer (if the changes are nuanced)
  • Ensure that there is a testing strategy (and documented non-automated tests)
  • Ensure there is a documented monitoring strategy (if necessary)
  • Consider systems implications (Database connections, Grafana stats, CPU)

@subsymbolic subsymbolic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants