[image_picker_android] Adds Android 13 photo picker functionality #3267

FXschwartz · 2023-02-22T18:27:10Z

NOTE: This PR was moved over from the flutter/plugins repo - flutter/plugins#7043

Description
This PR adds the new Android 13 photo picker functionality to Android devices running SDK version 33 or later as well as bumps the compileSdkVersion from 31 to 33. If an Android device is older, it will use the previous standard image picker. Below are two devices, the left running SDK 30 and the right running SDK 33.

SDK 30	SDK 33

List which issues are fixed by this PR. You must list at least one issue.
Fixes flutter/flutter#104250

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

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

gnprice · 2023-02-22T19:52:39Z

...er_android/android/src/test/java/io/flutter/plugins/imagepicker/ImagePickerDelegateTest.java

+    verify(mockActivity)
+        .startActivityForResult(
+            any(Intent.class),
+            eq(ImagePickerDelegate.REQUEST_CODE_CHOOSE_IMAGE_FROM_GALLERY_USING_PHOTO_PICKER));


(Continuing from flutter/plugins#7043 (comment) on the pre-migration version of this PR, and from Discord.)

This is checking what "request code" we passed to startActivityForResult. But the request code doesn't have any effect on what UI the system presents to the user, or anything like that — it's a number we make up for our own internal tracking, so that our onActivityResult can know what to do with the result it gets.

The thing that controls whether the system shows the spiffy new photo picker, or does something else, is the "action" in the Intent. So that would be the thing to inspect if writing this style of unit test.

Here's probably the clearest upstream docs on that — in particular the "Action" subsection:
https://developer.android.com/guide/components/intents-filters

Then in the reference docs there's
https://developer.android.com/reference/android/app/Activity#startActivityForResult(android.content.Intent,%20int)
https://developer.android.com/reference/android/app/Activity#onActivityResult(int,%20int,%20android.content.Intent)
and the voluminous intro of
https://developer.android.com/reference/android/content/Intent.html
.

Here's probably the clearest upstream docs on that — in particular the "Action" subsection: https://developer.android.com/guide/components/intents-filters

Is this something we want want to implement for this PR then? Looks like all the tests currently only check that the intent has the correct tag so I imagine we'd want to refactor those tests to also confirm the correct action.

I imagine we'd want to refactor those tests to also confirm the correct action.

I think that would be a good improvement, but my suggestion for this PR would be to leave the tests for other functionality as they are.

…Contracts to decide which photo_picker should be used based on SDK version instead of handling the checks manually.

tarrinneal · 2023-02-24T21:25:43Z

the error causing the failing test:

/imagepicker/ImagePickerDelegate.java:347: Error: Call requires API level 19 (current min is 16): PickMultipleVisualMedia [NewApi]
new ActivityResultContracts.PickMultipleVisualMedia()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

FXschwartz · 2023-02-24T21:30:55Z

the error causing the failing test:

/imagepicker/ImagePickerDelegate.java:347: Error: Call requires API level 19 (current min is 16): PickMultipleVisualMedia [NewApi]

Is there any kind of approval process to bump the min SDK or am I good to make that change?

gnprice · 2023-02-24T23:01:50Z

Flutter in general supports API 16, so I imagine folks will want to keep these plugins supporting API 16 too.

It looks like both of these ActivityResultContracts.Pick… classes require API 19. I guess that's because they fall back only as far as Intent.ACTION_OPEN_DOCUMENT, which was new in that version.

I'm a bit surprised that the AndroidX folks didn't add a fallback to Intent.ACTION_GET_CONTENT (what the existing code here has been using) in order to support older versions — I'd thought supporting older Android versions smoothly was a big part of what AndroidX was for — but it seems they didn't.

So I think you'll need to check for API 19, and below that version fall back to the same code that's currently here. Fortunately I don't think that makes things much more complicated; it basically just means adding the same code this version adds, but not deleting the old code. And the old code is pretty short.

FXschwartz · 2023-02-27T14:49:42Z

I'm a bit surprised that the AndroidX folks didn't add a fallback to Intent.ACTION_GET_CONTENT (what the existing code here has been using) in order to support older versions — I'd thought supporting older Android versions smoothly was a big part of what AndroidX was for — but it seems they didn't.

Yeah, it definitely seems odd, especially since they are already doing some SDK checks with the photo picker. I'll implement this change today and ping you for another review!

reidbaker · 2023-02-27T16:30:56Z

Fyi flutter is in the process of bumping support to a min of api 19.
flutter.dev/go/android-ndk-version

FWIW I am waiting on terrinneal to give this a pass before digging in.

gnprice · 2023-02-27T17:07:28Z

Fyi flutter is in the process of bumping support to a min of api 19.
flutter.dev/go/android-ndk-version

Ah, good to know, thanks!

If I owned this code I think I'd want to keep the API 16 support until that process completes and Flutter itself actually does bump the minimum to 19. At least I would in this case, where the cost of continued support is just to keep a few lines of existing code, wrapped in a conditional where it only runs on the old versions. But I wouldn't feel strongly about it.

FXschwartz · 2023-02-27T17:37:42Z

If I owned this code I think I'd want to keep the API 16 support until that process completes and Flutter itself actually does bump the minimum to 19. At least I would in this case, where the cost of continued support is just to keep a few lines of existing code, wrapped in a conditional where it only runs on the old versions. But I wouldn't feel strongly about it.

I agree, I'll continue with the original plan of checking the current API version before deciding what code to run.

@reidbaker Appreciate you bringing that up!

tarrinneal · 2023-02-28T00:24:22Z

Flutter in general supports API 16, so I imagine folks will want to keep these plugins supporting API 16 too.

We will need to maintain support for API 16 for now. My intention with surfacing that error wasn't to suggest that it needed to be updated in that way. Sorry for the confusion.

I've been digging in to future intent with this plugin moving forward, so it is taking me a little longer than expected to be able to approve this pr. Hoping to be able to move forward with it asap.

tarrinneal

Flutter in general supports API 16, so I imagine folks will want to keep these plugins supporting API 16 too.

We will need to maintain support for API 16 for now. My intention with surfacing that error wasn't to suggest that it needed to be updated in that way. Sorry for the confusion.

…used, otherwise defaults to old Intent.ACTION_GET_CONTENT behavior.

FXschwartz · 2023-02-28T20:21:29Z

Flutter in general supports API 16, so I imagine folks will want to keep these plugins supporting API 16 too.

We will need to maintain support for API 16 for now. My intention with surfacing that error wasn't to suggest that it needed to be updated in that way. Sorry for the confusion.

No apologies necessary, really appreciate the review!

Wrapped functionality in an if statement to check the SDK version. Ready for another review!

...picker_android/android/src/main/java/io/flutter/plugins/imagepicker/ImagePickerDelegate.java

tarrinneal

This all seems good to me, thanks for sending this in, and handling all of the revisions as well!

auto-submit · 2023-02-28T21:22:56Z

auto label is removed for flutter/packages, pr: 3267, due to - This commit is not mergeable and has conflicts. Please rebase your PR and fix all the conflicts.

reidbaker · 2023-02-28T21:23:47Z

I think I fixed the conflicts by removing a version that didnt have a changelog message.

gnprice · 2023-02-28T21:54:30Z

(This looks great to me! Thanks @FXschwartz for all your work on it.)

auto-submit · 2023-03-01T00:50:18Z

auto label is removed for flutter/packages, pr: 3267, due to - The status or check suite repo_checks has failed. Please fix the issues identified (or deflake) before re-applying this label.

…lity (flutter/packages#3267)

* main: (3910 commits) [various] Align Flutter and Dart SDK constraints (flutter#3349) Roll Flutter from c590086 to f2f8005 (14 revisions) (flutter#3373) [webview_flutter] Enable warnings-as-errors on Android (flutter#3356) [ci] Increase Android platform test sharding (flutter#3365) Roll Flutter from f032a4d to c590086 (69 revisions) (flutter#3366) [Espresso] Update truth package to 1.1.3 (flutter#3358) [google_maps] Relax the Android renderer requset test (flutter#3364) [pigeon] Only check generated files on master (flutter#3357) [webview]: Bump androidx.webkit:webkit from 1.5.0 to 1.6.0 in /packages/webview_flutter/webview_flutter_android/android (flutter#3243) [ci+various] Partially enable javac warning checks (flutter#3293) [webview_flutter] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3336) [local_auth] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3335) [google_sign_in] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3330) [google_maps_flutter] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3329) [video_player] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3328) [file_selector] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3325) [go_router_builder] Fix the example for default values in the README (flutter#3231) Update annotation and espresso dependencies (flutter#3271) [tool] Provide a --base-branch flag (flutter#3322) [image_picker_android] Adds Android 13 photo picker functionality (flutter#3267) ...

…utter#3267) [image_picker_android] Adds Android 13 photo picker functionality

Recreating PR from flutter/plugins

d351ca8

github-actions bot added p: image_picker platform-android labels Feb 22, 2023

gnprice reviewed Feb 22, 2023

View reviewed changes

FXschwartz added 2 commits February 22, 2023 14:21

Removes duplicate intent tags, uses android.x.activity ActivityResult…

5e1a035

…Contracts to decide which photo_picker should be used based on SDK version instead of handling the checks manually.

Formats java file

da4b4d0

FXschwartz marked this pull request as ready for review February 22, 2023 20:25

FXschwartz requested a review from gmackall as a code owner February 22, 2023 20:25

gmackall requested review from tarrinneal and reidbaker and removed request for gmackall February 23, 2023 19:36

Bumps minSdkVersion from 16 to 19.

f41bd09

tarrinneal requested changes Feb 28, 2023

View reviewed changes

Adds SDK version check to decide if the newer PickVisualMedia can be …

16530a8

…used, otherwise defaults to old Intent.ACTION_GET_CONTENT behavior.

FXschwartz requested review from tarrinneal and removed request for reidbaker February 28, 2023 20:20

tarrinneal requested changes Feb 28, 2023

View reviewed changes

...picker_android/android/src/main/java/io/flutter/plugins/imagepicker/ImagePickerDelegate.java Show resolved Hide resolved

tarrinneal self-requested a review February 28, 2023 20:44

tarrinneal approved these changes Feb 28, 2023

View reviewed changes

tarrinneal requested review from gnprice and removed request for gnprice February 28, 2023 20:51

tarrinneal requested a review from stuartmorgan-g February 28, 2023 20:51

reidbaker approved these changes Feb 28, 2023

View reviewed changes

reidbaker added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 28, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 28, 2023

Merge branch 'main' into photo-picker

b6dcf6d

fix changelog

dad6094

tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 1, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 1, 2023

pubspec

28be6bd

tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 1, 2023

auto-submit bot merged commit 7ec6a77 into flutter:main Mar 1, 2023

engine-flutter-autoroll mentioned this pull request Mar 1, 2023

Roll Packages from f39c97dbfb95 to 09544999b221 (8 revisions) flutter/flutter#121695

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 1, 2023

7ec6a77 [image_picker_android] Adds Android 13 photo picker functiona…

6a7b5b7

…lity (flutter/packages#3267)

danagbemava-nc mentioned this pull request Mar 3, 2023

[image_picker] No longer able to select photos from Google Photos flutter/flutter#121744

Closed

reidbaker mentioned this pull request Mar 20, 2023

[Many] i123017 integration annotation documentation #3495

Merged

10 tasks

nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023

[image_picker_android] Adds Android 13 photo picker functionality (fl…

71ed274

…utter#3267) [image_picker_android] Adds Android 13 photo picker functionality

camsim99 mentioned this pull request Feb 10, 2025

[ci] Manually roll master, set -Xmx4G #8586

Merged

11 tasks

[image_picker_android] Adds Android 13 photo picker functionality #3267

[image_picker_android] Adds Android 13 photo picker functionality #3267

Uh oh!

Conversation

FXschwartz commented Feb 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gnprice Feb 22, 2023

Choose a reason for hiding this comment

Uh oh!

gnprice Feb 22, 2023

Choose a reason for hiding this comment

Uh oh!

FXschwartz Feb 22, 2023

Choose a reason for hiding this comment

Uh oh!

gnprice Feb 22, 2023

Choose a reason for hiding this comment

Uh oh!

tarrinneal commented Feb 24, 2023

Uh oh!

FXschwartz commented Feb 24, 2023

Uh oh!

gnprice commented Feb 24, 2023

Uh oh!

FXschwartz commented Feb 27, 2023

Uh oh!

reidbaker commented Feb 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnprice commented Feb 27, 2023

Uh oh!

FXschwartz commented Feb 27, 2023

Uh oh!

tarrinneal commented Feb 28, 2023

Uh oh!

tarrinneal left a comment

Choose a reason for hiding this comment

Uh oh!

FXschwartz commented Feb 28, 2023

Uh oh!

Uh oh!

tarrinneal left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Feb 28, 2023

Uh oh!

reidbaker commented Feb 28, 2023

Uh oh!

gnprice commented Feb 28, 2023

Uh oh!

auto-submit bot commented Mar 1, 2023

Uh oh!

Uh oh!

FXschwartz commented Feb 22, 2023 •

edited

Loading

reidbaker commented Feb 27, 2023 •

edited

Loading