This repository was archived by the owner on Feb 22, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[image_picker] add requestFullMetadata for iOS (optional permissions) #4638
[image_picker] add requestFullMetadata for iOS (optional permissions) #4638
Changes from 8 commits
5d3a9aa
4195dca
7d255de
3fd7383
686bc22
1f73191
24d0679
1ba0c73
1084763
a89ad0a
8ff7492
193bbe9
95eef20
e10b24e
f2c2e68
f5f94b2
97efdca
bd3dfeb
b99ba07
3d6c45c
b3e9166
bf3f55c
742b259
d41b390
ead2f62
7b6ace9
0f43653
c6dcc49
9ba71b9
c1fc9da
7f958a6
c825d09
a2ca361
0120c47
0827c80
994e45f
9b39e31
933254e
66f14b2
47098ec
0a811d8
0789a7b
c5966d4
6f32405
217237b
7a4d250
4ee04cb
b3fb220
4d884de
c2240fa
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Nit: remove the comma after "control".
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.
Nit: This shouldn't specifically say it's for iOS, as it might apply to other platforms in the future. I would remove iOS here, and add it to the sentence below (e.g., "... which may require extra permission requests on some platforms, such as iOS").
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.
You need to require the version of the platform interface that has this method 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.
Re-adds
(Per style guide)
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.
You don't need to explain in the changelog that it's non-breaking; that's indicated by the version.
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.
Okay, I only wanted to explicitly show the difference from previous implementation
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.
Please put this next to
getImage
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.
Nit: Missing final period.
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.
What is the reason behind making source not a part of
ImagePickerOptions
? Can we also movesource
toImagePickerOptions
?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 parameter is needed whenever the picking method is called. All parameters inside
ImagePickerOptions
are more optional, so the whole options object can be skipped if a caller doesn't need any specific adjustments. It was @stuartmorgan's suggestion in this PR that I've agreed with, as calling the picking method doesn't require creating the options object for every use