[image_picker_macos] macOS native image picker

[EchoEllet]

Adds support for macOS native image picker for image_picker_macos.

An alternative implementation that is based on PHPickerViewController instead of file_selector.

FlutterNativeMacOSImagePicker.mp4

Usage

Similar to the Android Photo Picker in image_picker_android:

import 'package:image_picker_macos/image_picker_macos.dart';
import 'package:image_picker_platform_interface/image_picker_platform_interface.dart';
// ···
  final ImagePickerPlatform imagePickerImplementation =
      ImagePickerPlatform.instance;
  if (imagePickerImplementation is ImagePickerMacOS) {
    imagePickerImplementation.useMacOSPHPicker = true;
  }

Note

It's only supported on macOS +13, on macOS 12 and older versions, will fallback to the file_selector_macos implementation.

Related Issues

• Fix [image_picker] Use PHPicker for macOS 13+ flutter#125829
• Fix [image_picker] Support image/video restrictions on macOS flutter#150063
• Related Build native UI testing support for macOS plugins flutter#70234
• Related [image_picker] Add desktop support - implementations #4172
• Related [image_picker] Add desktop support #3882
• Related [macOS] Provide support for image_picker flutter#85100

Limiations

• Supports maxWidth, maxHeight, imageQuality, and limit when using the PHPicker implementation.
• Supports picking image, multi-image, video, media, multi-media. The multi-media supports picking images and videos.
• Doesn't support maxDuration of video selection, and requestFullMetadata (iOS-specific).
• The user can only pick photos from the macOS Photos app. The user will see a No Photos message if they don't have photos within the app.

Motivation

The macOS native picker only supports picking images, videos, and media from the Photos for macOS app.

If the user has photos in a directory, they will need to open the app and import the images they need, however, this app integrates with other macOS apps and the Apple ecosystem, and it supports features such as importing pictures seamlessly from an iOS device, albums, favorites, recently deleted, duplicates.

Screenshot

Tip

It doesn't require file read-only access entitlement.

IMO

it's better to keep this implementation optional instead of making it as default, the developer can provide the option to switch between implementations based on the user preference since it's not common on desktops to have a photos app like mobile platforms, maybe provide a method to check if it's supported:

ImagePicker().supportsPHPicker();

// OR

final ImagePickerPlatform imagePickerImplementation =
      ImagePickerPlatform.instance;
  if (imagePickerImplementation is ImagePickerMacOS) {
    imagePickerImplementation.supportsPHPicker();
  }

Testing

This branch has been manually tested on macOS 14.6.1.
Native and Dart Unit tests have been added, though unfortunately, native UI tests are missing (see #70234). I left a TODO since currently it doesn't seem to be possible.

Additional Context

Related Discord Threads:

• Unable to create native macOS Unit/UI tests due to incompatibility between Xcode 16 and CocoaPods
• Unable to tap a Flutter widget button from macOS XCUITest

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I linked to 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, or this PR is exempt from version changes.
• I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this 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.

[EchoEllet]

I left TODOs in the code that should be addressed in this PR before continuing further except the TODO of adding macOS native UI tests.

I welcome any feedback and suggestions.

[EchoEllet]

Probably image-conversion-error?

[EchoEllet]

Is there a better way to handle error codes? Maybe an enum in the pigeons/messages.dart file. Currently, they are not being handled from the Dart side as it's pretty rare to encounter any of them (e.g., not enough storage or an unknown error while writing the temp file).

[tarrinneal]

@stuartmorgan for conventions.

Any error thrown should be handled on the dart side as per policy.

[stuartmorgan]

https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#platform-exception-handling

Known string constants that for the error code (matching on both sides of the communication channel) are an option, although of course that's the kind of thing that we switched to Pigeon to avoid so it's not ideal. In some packages where we've started moving to the new guidance, we do indeed use a message.dart enum so that we can do exhaustive, type-safe handling on the Dart side. In that model we use a return class that can include error information, and don't use PigeonError at all. (url_launcher_macos is a simple example of that approach.)

[EchoEllet]

All errors are handled on the Swift side and are thrown as PlatformException to Dart. According to the Platform Exception Handling docs, exceptions should be converted to the appropriate error types defined by the interface. Though which types should I use in this case? This comment is outdated.

[EchoEllet]

In that model we use a return class that can include error information, and don't use PigeonError at all. (url_launcher_macos is a simple example of that approach.)

In case the API call is @async, the completion expects a Swift Result that's either a failure or success. The failure requires a class that conforms to Error (such as PigeonError), so the return class generated by Pigeon can't be used. Is it okay to use success instead to return the error result?

Throwing the error from Swift:

completion(.success(ImagePickerErrorResult(error: .phpickerUnsupported)))

Handling the error from Dart:

final ImagePickerResult result = await _hostApi.pickImages(
    ImageSelectionOptions(quality: 100), GeneralOptions(limit: 1));
switch (result) {
  case ImagePickerSuccessResult():
    final List<String> filePaths = result.filePaths;
  case ImagePickerErrorResult():
    switch (result.error) {
      case ImagePickerError.phpickerUnsupported:
        throw UnsupportedError(
            result.platformErrorMessage.toString());
      // Exhaustive type-safe handling...
    }
}

I'm still uncertain what to throw after handling those platform errors in a type-safe way from the Dart side. Should I throw built-in exceptions from Dart whenever possible (e.g., ArgumentError, UnsupportedError)? As I didn't find any appropriate error types defined by the platform interface package.

[stuartmorgan]

Is it okay to use success instead to return the error result?

That is currently the only way to do error handling without string matching, yes. (TBD if we will eventually build some alternate error system into Pigeon for this pattern.)

I'm still uncertain what to throw after handling those platform errors in a type-safe way from the Dart side. Should I throw built-in exceptions from Dart whenever possible (e.g., ArgumentError, UnsupportedError)?

ArgumentError and UnsupportedError are errors, not exceptions. It would be incorrect in Dart to throw an error unless the package client did something wrong that should have been fixed at the time of authoring the client code. Runtime failures that are not the developer's fault (e.g., bad image data on a user's device) must be exceptions, not errors.

As I didn't find any appropriate error types defined by the platform interface package.

For now, it's fine to throw a PlatformException on the Dart side, with a TODO comment saying that this is for compatibility with the existing behavior of other implementations of image_picker, and that it should be replaced with a plugin-specific exception as part of a cross-platform update.

[EchoEllet]

it's fine to throw a PlatformException on the Dart side
In that model we use a return class that can include error information, and don't use PigeonError at all.

Done in 2a101a9.

Known string constants that for the error code (matching on both sides of the communication channel) are an option, although of course that's the kind of thing that we switched to Pigeon to avoid so it's not ideal.

The PigeonError is no longer used and the error handling is type-safe now.

[EchoEllet]

Mac_arm64 macos_repo_checks
Tasks failed: Swift format,validate iOS and macOS podspecs

All Swift files are formatted except the generated Messages.g.swift.

I ran the script dart run script/tool/bin/flutter_plugin_tools.dart format --packages image_picker_macos to format the Messages.g.swift though the output produced some warnings. Should they ignored?

Output

image_picker/image_picker_macos/macos/image_picker_macos/Sources/image_picker_macos/Messages.g.swift:166:15: warning: [TypeNamesShouldBeCapitalized] rename the class 'messagesPigeonCodecReader' using UpperCamelCase; for example, 'MessagesPigeonCodecReader'
image_picker/image_picker_macos/macos/image_picker_macos/Sources/image_picker_macos/Messages.g.swift:183:15: warning: [TypeNamesShouldBeCapitalized] rename the class 'messagesPigeonCodecWriter' using UpperCamelCase; for example, 'MessagesPigeonCodecWriter'
image_picker/image_picker_macos/macos/image_picker_macos/Sources/image_picker_macos/Messages.g.swift:203:15: warning: [TypeNamesShouldBeCapitalized] rename the class 'messagesPigeonCodecReaderWriter' using UpperCamelCase; for example, 'MessagesPigeonCodecReaderWriter'
image_picker/image_picker_macos/macos/image_picker_macos/Sources/image_picker_macos/Messages.g.swift:213:7: warning: [TypeNamesShouldBeCapitalized] rename the class 'messagesPigeonCodec' using UpperCamelCase; for example, 'MessagesPigeonCodec'
Swift linter found issues. See above for linter output.

These files are not formatted correctly (see diff below):
packages/image_picker/image_picker_macos/lib/src/messages.g.dart
packages/image_picker/image_picker_macos/test/test_api.g.dart

Those Dart files are generated, should generated files be formatted on this repo?
Formatted using dart format since the format tool within this repo didn't format them.

image_picker_macos/macos/image_picker_macos/Sources/image_picker_macos/ImagePickerImpl.swift:462:31: error: file contains invalid or unrecognized Swift syntax.
Failed to format Swift files: exit code 1.

I'm using Xcode 16.1 with Swift 6, maybe the CI uses Swift 5?

- NOTE | [OSX] xcodebuild: Pods.xcodeproj: warning: The macOS deployment target 'MACOSX_DEPLOYMENT_TARGET' is set to 10.11, but the range of supported deployment target versions is 10.13 to 14.0.99. (in target 'image_picker_macos' from project 'Pods')

Should I update the minimum to 10.13 instead of 10.11 since file_selector_macos requires at least 10.14 which is a dependency of image_picker_macos? Updated to 10.14 since that's the minimum supported version by Flutter stable.

- WARN | [OSX] xcodebuild: /Volumes/Work/s/w/ir/x/w/packages/packages/image_picker/image_picker_macos/macos/image_picker_macos/Sources/image_picker_macos/ImagePickerImpl.swift:279:15: warning: passing argument of non-sendable type 'NSItemProvider' outside of main actor-isolated context may introduce data

Should I only mark processAndSave with @MainActor (likely to be inefficient) or instead load the data representation and pass it to PickVideoHandler and PickImageHandler or maybe annotate only the part that uses NSItemProvider?

[stuartmorgan]

@cbracken Is the triage-macos label being reviewed in triage?

[cbracken]

@cbracken Is the triage-macos label being reviewed in triage?

Yes, but we're unfortunately quite backlogged and we prioritise the iOS review. Given how far back this is, it's probably worth me scheduling an extra catch-up triage.

I realise this isn't a viable long-term solution, but in the meantime, if there are any specific patches you need to get unblocked, feel free to ping me directly on them.

[jmagman]

Adding @cbracken as a reviewer. Sorry again for the delay on this one.

[cbracken]

Given the current state of Flutter desktop prioritisation, our policy has been to continue to keep desktop stable add quickly address P0 issues, but not to land new feature work or expand scope.

I want to be clear that this is the sort of patch we'd have been absolutely thrilled to have when desktop was a much higher priority, and personally this is a pretty exciting patch, but at present our policy is to avoid expanding scope or landing new feature work for desktop.

@stuartmorgan from the ecosystem perspective, is this something that might be better maintained as a community package we could point people to in place of our existing implementation?

[stuartmorgan]

Apologies @EchoEllet; I didn't realize this was no longer considered in scope or I definitely would have said something much earlier in the process!

If this implementation were published as a new unendorsed image_picker implementation package, we could certainly reference it as a potential alternative in the issue about native support and in the limitations section of the README.

[EchoEllet]

It was great working with you, even if it was just for a short time. Thanks!

If this implementation were published as a new unendorsed image_picker implementation package
is this something that might be better maintained as a community package

I’m more than willing to create a separate package—would you be open to reviewing the PR first? It might help decide the best path forward.

keep desktop stable add quickly address P0 issues, but not to land new feature work or expand scope.
avoid expanding scope or landing new feature work for desktop.

It seems we're aligned in our opinion.

[stuartmorgan]

I’m more than willing to create a separate package—would you be open to reviewing the PR first? It might help decide the best path forward.

Can you elaborate on what you mean by "decide the best path forward"? Code doesn't need to be reviewed by the Flutter team to be published on pub.dev.

[EchoEllet]

I have extracted this functionality into native_image_picker_macos as an unendorsed platform implementation, which has not yet been published to pub.dev.

Can you elaborate on what you mean by "decide the best path forward"?

I'm unfamiliar with native macOS and Flutter macOS integration and there is only one issue that's critical to fix before releasing the package. Fixed.

I have previously added a comment and a TODO for this issue.

Should we close this PR?

[stuartmorgan]

Should we close this PR?

Yes, sorry for the delay on this. I was waiting on an offline discussion to make sure I understand the current scoping decisions.

I'll follow up on the issue.

[EchoEllet]

No worries. Thank you all for your help with this PR!

[EchoEllet]

I'll follow up on the issue.

I appreciate your help in resolving the issue!

is this something that might be better maintained as a community package we could point people to in place of our existing implementation?

If this implementation were published as a new unendorsed image_picker implementation package, we could certainly reference it as a potential alternative in the issue about native support and in the limitations section of the README.

The package native_image_picker_macos has been published.