-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[image_picker] add forceFullMetadata for iOS #3264
Conversation
@cyanglaz I was unable to make any changes to the old PR. Please let me know if you'd like me to make any additional changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, can we add an XCUITest to make sure the permission popup didn't show if forceFullMetaData
is false
@cyanglaz Should I add a new FAB to the example with Also, I suppose I need to create a separate |
@cpboyd
|
@cpboyd It's been a while since I last commented, were you able to keep working on this? :) |
@cyanglaz I hadn't had time for this over the holidays. I'll try to make sure it's up-to-date with master and proceed |
@cpboyd Thank you so much! just FYI that we are going to migrate image picker to nnbd soon. |
@cyanglaz Great! I've been attempting to do the same with my projects |
@cpboyd the NNBD migration is done, you can safely rebase your PR. Then I'll review this and let's get this landed :) |
6d7b9b4
to
65e9045
Compare
9f20d07
to
390bd66
Compare
390bd66
to
33176ce
Compare
@cpboyd Looks like you need another rebase master to fix CI. Please ping me when it's done and I'll do another review. Let's try to land it soon :) |
33176ce
to
2706bc6
Compare
e5e720c
to
e37df4b
Compare
e37df4b
to
7c1716b
Compare
7c1716b
to
843eb99
Compare
843eb99
to
18474d6
Compare
@cyanglaz I've rebased, fixed failing tests, and added a checkbox to the example for toggling the Please advise next steps. |
5886aa1
to
ed4bcea
Compare
I had corrected the casing to |
Are there other cases where it's wrong in the public API? I was looking at the ObjC code when I made that comment; I don't remember if it extended beyond there. |
From searching |
Then a new public API should definitely use the correct capitalization. |
@cpboyd Thank you very much for this. I apologize for the delay. I just came back from a long time-off.
Once that's done, I can help landing this. |
a3ed081
to
0f20461
Compare
0f20461
to
024cc35
Compare
024cc35
to
6b54992
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that the platform_interface is updated, I think we need to update the version dependency in pubspec.yaml?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This cannot land without first fixing the interface change, which needs to be reverted.
@stuartmorgan I'm sorry, I just don't have the time to dedicate to this PR. In all honesty, I'm not fully certain about the rebases that I did previously. I stopped using this optional approach in my own app months ago in favor of simply stripping out the permissions elevation code: https://github.com/cpboyd/plugins/tree/ios-no-permissions |
Description
Originally #3090
Make iOS 11+ permissions requests optional per https://developer.apple.com/forums/thread/653414.
PHAssets aren't actually required and there was already some fallback code in place.
To keep this as a non-breaking change, I added an optional parameter of
forceFullMetaData
with a default oftrue
.Setting this to false will disable any permissions checking on iOS and simply display the image picker.
I still need to update the tests.
Related Issues
Fixes flutter/flutter#65995
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?