Skip to content

[image_picker] Add the ability to pick multiple videos #9775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

stuartmorgan-g
Copy link
Contributor

Adds pickMultiVideo, to allow picking multiple videos. This fills the gap in our current support matrix, which allows single or multiple images, single or multiple image-or-video, but only single video-only.

Other changes:

  • While updating the example apps to have a new button for manual testing, I did some cleanup (capitalization fixes, inconsistent use of icons, button order), and also reduced drift between the different versions (e.g., someone had added a snackbar to Android that showed the selected paths, so I replicated that change to the other implementations).
  • On iOS, pickVideo had never been updated to the new UI-or-PH codepaths that all the other implementations used. pickMultiVideo was easily implementable with the new codepaths with only minor changes, so rather than leave pickVideo inconsistent I updated it as well.

Fixes flutter/flutter#102283
Fixes flutter/flutter#146400

Pre-Review Checklist

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Footnotes

  1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. 2 3

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds the pickMultiVideo feature, allowing users to select multiple videos from their gallery. The changes span across the platform interface and all platform implementations (Android, iOS, Web, Linux, macOS, Windows). The example apps have also been updated to demonstrate this new functionality, along with some general cleanup.

My review focuses on ensuring the new feature is implemented correctly across all platforms, checking for potential issues in the example apps, and verifying that dependency configurations are correct for merging.

Overall, the implementation is solid and the refactoring on the iOS side is a great improvement. I've identified a few issues, mainly in the example applications and pubspec.yaml files, that should be addressed before merging.

@stuartmorgan-g
Copy link
Contributor Author

stuartmorgan-g commented Aug 8, 2025

For review context, this PR started as an experiment with Jules using the new agents.md file. Jules wrote:

  • Essentially all of the non-example Dart changes
  • A first pass of the Android changes, which only needed minor changes
  • A first pass of the iOS changes, which were missing some refactoring to the shared codepath but were a good start
    • This is a useful cautionary example for reviewing AI-generated code: the new code it created looked totally reasonable in isolation, and compiled, but didn't work correctly because the shared codepath had never been written to support the case of not allowing image selection.
  • All of the desktop implementations
  • Essentially all of the Dart unit tests

I then wrote all the example app changes (I forgot to ask Jules for those, and didn't feel like going back and doing another round once I realized I needed them when manually testing), did the iOS refactoring, did some minor Android tweaks, added some native unit tests, and fixed all the changelogs and versions (Jules got confused and bumped the version of every package twice, possibly using the repo tooling).

@stuartmorgan-g stuartmorgan-g added the federated: all_changes PR that contains changes for all packages for a federated plugin change label Aug 8, 2025
stuartmorgan-g and others added 3 commits August 8, 2025 11:36
…va/io/flutter/plugins/imagepicker/ImagePickerDelegate.java

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@stuartmorgan-g stuartmorgan-g changed the title [image_picker] All picking multiple videos [image_picker] Add the ability to pick multiple videos Aug 8, 2025
@@ -121,12 +122,19 @@ - (void)launchUIImagePickerWithSource:(nonnull FLTSourceSpecification *)source
UIImagePickerController *imagePickerController = [self createImagePickerController];
imagePickerController.modalPresentationStyle = UIModalPresentationCurrentContext;
imagePickerController.delegate = self;
NSMutableArray<NSString *> *mediaTypes = [[NSMutableArray alloc] init];
if (context) {
[mediaTypes addObject:(NSString *)kUTTypeImage];
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like kUTTypeImage is included regardless of context.includeImages. Is that intentional? Shouldn't it not be included for pickMultiVideoWithMaxDuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that intentional?

Nope, that was me somehow forgetting to type .includeImages on that condition.

I missed it in manual testing because I forgot to do the test I had intended to do where I forced the UIImagePickerController codepaths; I've now done that.

I also looked at why no existing unit tests failed, and it looks like unfortunately the native unit tests aren't currently set up to force codepaths, so we aren't getting good coverage of the older codepath. It may not be worth investing time into addressing that at this point though, given that it's only for iOS <14.

Copy link
Contributor

@vashworth vashworth left a comment

Choose a reason for hiding this comment

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

iOS side LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants