Skip to content

Conversation

@mneuwert
Copy link
Contributor

@mneuwert mneuwert commented Dec 20, 2020

Description

  • Using new PHPhotoPicker introduced in iOS14 instead of our custom picker.
  • Dealing with the photo permission model introduced in iOS14 where user can grant access just to specific photo assets or albums.

Related Issue

#851

Motivation and Context

  • New photo picker in iOS14 has got an album tab and allows searching based on people faces, objects, locations etc.
  • Since the user can limit the access, we could provide a convenient UI option allowing to raise access level without the need opening the Settings app.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

QA

Test plan:

Bugs & improvements:

@mneuwert mneuwert added the enhancement New feature or request label Dec 20, 2020
@mneuwert mneuwert self-assigned this Dec 20, 2020
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Michael Neuwert seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@michaelstingl michaelstingl linked an issue Jan 5, 2021 that may be closed by this pull request
Michael Neuwert added 4 commits January 24, 2021 19:28
- If limited photo library access is active, the limited picker is not displayed automatically
- If user selects some assets and some of the are limited in access, limited photo picker is displayed giving user a chance to change pernissions
- If none of the assets are granted access, warning alert is presented
@mneuwert
Copy link
Contributor Author

@michaelstingl Improved photo selection process on iOS14 using new picker:

  • If limited access feature allowing to disable access to certain photos is not active, everything just works.
  • If user limits access to certain photos / videos, they can be selected.. And then similar picker used by the system to change permissions is presented allowing the user to revise his choice regarding limited access and to grant access.. (however it is a pity you can't actually highlight stuff which is selected for upload but is missing access permission).
  • When user confirms his new limited access selection, then assets are re-fetched and if they are available, then they are uploaded. If none of the assets are accessible, warning alert is shown.

@jesmrec jesmrec added this to the 11.5.0-Current milestone Jan 25, 2021
Copy link
Collaborator

@hosy hosy left a comment

Choose a reason for hiding this comment

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

Great work!

@mneuwert
Copy link
Contributor Author

@hosy Thanks! Appreciate!

@jesmrec
Copy link
Contributor

jesmrec commented Jan 29, 2021

I have to finish QA here it. First rounds seems pretty good.

(1)

Just one comment. Check these steps out:

  1. In Device > Settings > ownCloud > Photos, select Selected Photos
  2. Select some photos there
  3. in ownCloud app, select Upload from Photo Library
  4. Select photos that were not selected in step 2.

at this point, i expected an alert error, letting user know that it is not allowed (lack of permissions). Something similar to the error that happens when None is selected in step 1.

Instead, the selecting view (the same view that appears in step 1) is displayed to allow users to select new items to allow them for the app. This could be a little confusing, but my question is: is that way to do iOS-native? or is something implemented for the feature?

@mneuwert
Copy link
Contributor Author

I have to finish QA here it. First rounds seems pretty good.

Just one comment. Check these steps out:

  1. In Device > Settings > ownCloud > Photos, select Selected Photos
  2. Select some photos there
  3. in ownCloud app, select Upload from Photo Library
  4. Select photos that were not selected in step 2.

at this point, i expected an alert error, letting user know that it is not allowed (lack of permissions). Something similar to the error that happens when None is selected in step 1.

Instead, the selecting view (the same view that appears in step 1) is displayed to allow users to select new items to allow them for the app. This could be a little confusing, but my question is: is that way to do iOS-native? or is something implemented for the feature?

  • That's in fact confusing since new photo picker shows all pictures irrespective of photo library permissions.
  • So, if you try to fetch photos which are not available (but you have selected them), second almost identical picker is shown allowing to add more permissions. It comes from the system and there is no way to customise it.
  • So, if you don't implement special handling, this second picker is appearing only once and if user doesn't change permissions, he is never asked again by the system. I did try to improve this though.. There is a way to suppress it via setting special key in Info.plist and to trigger it manually. But again, you can just tell the system to show it and can't pass any parameters or so... The only improvement I managed to achieve is: it is always consistently shown whenever current permissions don't match the selection of assets to upload.

So there are some tradeoffs when using new photo picker...

@michaelstingl
Copy link
Contributor

Before Apple’s dialogue pops up, could we show an own thing, that tells users what to do?

@mneuwert
Copy link
Contributor Author

mneuwert commented Jan 29, 2021

Before Apple’s dialogue pops up, could we show an own thing, that tells users what to do?

yes, we could.. There is actually some text on top of Apple's dialogue, but I agree that it is eventually barely noticeable

Michael Neuwert added 2 commits January 30, 2021 16:56
- Added intermediate dialogue appearing in case not all media to be uploaded had been given appropriate permissions by the user
@mneuwert
Copy link
Contributor Author

Before Apple’s dialogue pops up, could we show an own thing, that tells users what to do?

Added an alert conveying that access to media is limited, then when user taps " Change" Apple's dialogue is shown, where photos to which access is granted are displayed with a checkmark.

@jesmrec
Copy link
Contributor

jesmrec commented Feb 1, 2021

Added an alert conveying that access to media is limited, then when user taps " Change" Apple's dialogue is shown, where photos to which access is granted are displayed with a checkmark.

that's much better

@jesmrec
Copy link
Contributor

jesmrec commented Feb 1, 2021

Approved on my side

@jesmrec jesmrec added the Approved by QA Approved by QA label Feb 1, 2021
@hosy hosy merged commit df05e49 into milestone/11.5 Feb 1, 2021
@delete-merged-branch delete-merged-branch bot deleted the feature/ios14_photo_picker branch February 1, 2021 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved by QA Approved by QA enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deal with iOS 14 Photo permissions

6 participants