Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[image_picker] Multiple image support #3783

Merged

Conversation

danielroek
Copy link
Contributor

@danielroek danielroek commented Apr 2, 2021

Implements functionality to pick multiple images in iOS14+ and Android.

Fixes #16054
Fixes #82519

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@cyanglaz
Copy link
Contributor

cyanglaz commented Apr 2, 2021

This is really nice! Thanks. Since it is still a draft I will hold off reviewing it. Please ping me when it's ready and you want me to review it :)

# Conflicts:
#	packages/image_picker/image_picker/CHANGELOG.md
#	packages/image_picker/image_picker/pubspec.yaml
Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

LGTM

@cyanglaz
Copy link
Contributor

Please fix CI.

@cyanglaz
Copy link
Contributor

@renefloor Looks like your comments have been addressed. Do you want to take another look?

Copy link
Contributor

@renefloor renefloor left a comment

Choose a reason for hiding this comment

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

I have 1 serious question in the Android code and some nits.

packages/image_picker/image_picker/CHANGELOG.md Outdated Show resolved Hide resolved
@@ -11,6 +11,9 @@ First, add `image_picker` as a [dependency in your pubspec.yaml file](https://fl

### iOS

Starting with version **0.8.1** the iOS implementation uses PHPicker to pick (multiple) images on iOS 14 or higher.
As a result of implementing PHPicker it becomes impossible to pick HEIC images on the iOS simulator in iOS 14+. This is a known issue. Please test this on a real device, or test with non-HEIC images until Apple solves this issue.[63426347 - Apple known issue](https://www.google.com/search?q=63426347+apple&sxsrf=ALeKk01YnTMid5S0PYvhL8GbgXJ40ZS[…]t=gws-wiz&ved=0ahUKEwjKh8XH_5HwAhWL_rsIHUmHDN8Q4dUDCA8&uact=5)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a Google Search URL, is there a direct link to the Apple issue tracker? I can only find forum threads like this: https://developer.apple.com/forums/thread/658135

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, there is not an issue tracker. Not one publicly available. It seems those issues can contain private data which is why they are private.

Copy link
Contributor

Choose a reason for hiding this comment

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

packages/image_picker/image_picker/README.md Outdated Show resolved Hide resolved
@@ -1,3 +1,8 @@
## 0.8.1

* Add a new method `getMultiImage` to allow picking multiple images on iOS 14 or higher and Android 4.3 or higher.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe nice to mention what this function does on older devices. Does it pick 1 or does it not work at all?

@@ -564,6 +625,17 @@ private void finishWithSuccess(String imagePath) {
clearMethodCallAndResult();
}

private void finishWithListSuccess(ArrayList<String> imagePaths) {
if (pendingResult == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we also call clearMethodCallAndResult(); in this case?

Copy link
Contributor

@renefloor renefloor left a comment

Choose a reason for hiding this comment

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

LGTM

@mvanbeusekom mvanbeusekom added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Jun 22, 2021
@fluttergithubbot fluttergithubbot merged commit e864172 into flutter:master Jun 22, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 22, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 22, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 22, 2021
amantoux pushed a commit to amantoux/plugins that referenced this pull request Jul 10, 2021
fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021
jolmiracle added a commit to miracle-as/webview_flutter that referenced this pull request Sep 24, 2021
# Conflicts:
#	packages/image_picker/image_picker/CHANGELOG.md
#	packages/image_picker/image_picker/README.md
#	packages/image_picker/image_picker/example/android/app/src/test/java/io/flutter/plugins/imagepicker/ImagePickerDelegateTest.java
#	packages/image_picker/image_picker/lib/image_picker.dart
#	packages/image_picker/image_picker/pubspec.yaml
#	packages/image_picker/image_picker/test/image_picker_test.dart
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes p: image_picker platform-android platform-ios waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
9 participants