[image_picker_macos] macOS native image picker #8079

EchoEllet · 2024-11-14T08:51:58Z

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:

Pre-launch Checklist

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 · 2024-11-14T09:06:43Z

packages/image_picker/image_picker_macos/example/macos/RunnerTests/ImageCompressTests.swift

+  private func createTestImage(size: NSSize) -> NSImage {
+    let image = NSImage(size: size)
+    image.lockFocus()
+    NSColor.white.set()
+    NSBezierPath(rect: NSRect(origin: .zero, size: size)).fill()
+    image.unlockFocus()
+    return image
+  }


This is duplicated in both ImageCompressTests and ImageResizeTests, should I extract it?
Most unit tests of plugins on this repo are implemented with one file RunnerTests.

packages/image_picker/image_picker_macos/example/macos/Runner/AppDelegate.swift

EchoEllet · 2024-11-14T09:08:02Z

packages/image_picker/image_picker_macos/example/macos/RunnerTests/ImageResizeTests.swift

+  private func createTestImage(size: NSSize) -> NSImage {
+    let image = NSImage(size: size)
+    image.lockFocus()
+    NSColor.black.set()
+    NSBezierPath(rect: NSRect(origin: .zero, size: size)).fill()
+    image.unlockFocus()
+    return image
+  }


Duplicated.

packages/image_picker/image_picker_macos/lib/image_picker_macos.dart

EchoEllet · 2024-11-14T09:11:30Z

packages/image_picker/image_picker_macos/lib/image_picker_macos.dart

+  ///  * [useMacOSPHPicker] to check whether **PHPicker** should be preferred over [file_selector_macos](https://pub.dev/packages/file_selector_macos).
+  ///  * [supportsPHPicker] to verify if the current macOS version supports **PHPicker**.
+  @visibleForTesting
+  Future<bool> shouldUsePHPicker() async =>


https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md#avoid-using-visiblefortesting

Does mean that this should be public?

EchoEllet · 2024-11-14T09:13:18Z

...r/image_picker_macos/macos/image_picker_macos/Sources/image_picker_macos/ImageCompress.swift

+      // TODO(EchoEllet): Is there a convention for the error code? ImageConversionError or IMAGE_CONVERSION_ERROR or image-conversion-error. Update all codes.
+      throw PigeonError(
+        code: "ImageConversionError", message: "Failed to convert NSImage to TIFF data.",


Probably image-conversion-error?

packages/image_picker/image_picker_macos/macos/image_picker_macos.podspec

EchoEllet · 2024-11-14T09:15:42Z

...r/image_picker_macos/macos/image_picker_macos/Sources/image_picker_macos/ImageCompress.swift

+    assert(quality != 100, "Quality 100 means no compression.")
+    assert(quality >= 0, "Quality can't be negative.")


Should I remove the asserts from the platform side since it will crash the app on failure when not running using Xcode (flutter run -d macos)?

packages/image_picker/image_picker_macos/README.md

...ker/image_picker_macos/example/macos/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme

EchoEllet · 2024-11-14T09:34:07Z

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

I'm new to Apple/Xcode/Swift ecosystem, 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?

EchoEllet · 2024-11-14T10:23:42Z

The doc comments of image_picker have not been updated.

temporary file could not be created (iOS only)
Future<XFile?> pickImage

This is also possible with the native macOS implementation (when PHPicker is used). Should I update those parts of the docs to reflect the change?

EchoEllet · 2024-11-14T10:37:20Z

The ImageSource is unsupported for both implementations, should override the supportsImageSource?

@override
bool supportsImageSource(ImageSource source) {
  if (source == ImageSource.camera) {
    return cameraDelegate != null;
  }
  return false;
}

The arguments maxWidth, maxHeight, imageQuality, and limit are only supported when using the PHPicker implementation;

It's also possible to support those on file_selector implementation using the same platform API, should we support resizing and compressing for file_selector implementation?

EchoEllet · 2024-11-14T11:53:52Z

...image_picker_macos/macos/image_picker_macos/Sources/image_picker_macos/ImagePickerImpl.swift

+      noActiveWindow()
+      return
+    }
+    // TODO(EchoEllet): IMPORTANT The window size of the picker is smaller than expected, see the video in https://discord.com/channels/608014603317936148/1295165633931120642/1295470850283147335


This is an important issue to fix since it affects the user experience, see the video in this PR.

EchoEllet · 2024-11-14T12:11:56Z

packages/image_picker/image_picker_macos/lib/image_picker_macos.dart

+              _imageOptionsToImageSelectionOptions(options.imageOptions),
+        ),
+        GeneralOptions(
+          limit: options.limit ?? (options.allowMultiple ? 0 : 1),


This seems to be incorrect and needs validation similarly to image_picker_android. The current code doesn't check allowMultiple in case limit was provided.

It looks like there is validation from the app-facing package (image_picker) for all methods, do we still need to verify from the platform implementation side?

Signed-off-by: Ellet <[email protected]>

…mated), deprecated @NSApplicationMain attribute Signed-off-by: Ellet <[email protected]>

…acos - Updates the `image_picke_macos`'s `pubspec.yaml` to add `pigeon` as dev dependency and add the `pluginClass` for Swift native code - Adds the `ImagePickerPlugin` in `image_picker_macos/macos` for native macOS plugin with support for SPM and CocoaPods with basic native unit tests - Uses the steps in https://github.com/flutter/flutter/blob/master/docs/ecosystem/testing/Plugin-Tests.md#enabling-xctests-or-xcuitests to enable `XCTests` and `XCUITests` - Updates the `image_picker_macos_test.dart` to fix the test failure and ensure PHPicker is disabled by default - Adds a new button in the example to enable/disable PHPicker macOS implementation and enable the PHPicker by default - Updates the `README.md` of `image_picker_macos` and `image_picker` to document the usage - Removes two TODOs in `image_picker_macos.dart` as they are done with this PR - Adds TODOs that need to be done before merging the PR, some of them are questions, will be removed - Implement the getMultiImageWithOptions() since the getMultiImage is deprecated, updates getMultiImage() to delegate to getMultiImageWithOptions() since getMultiImageWithOptions() is required to access the limit property - Updates the Dart unit tests of image_picker_macos - Adds simple integration test for the example - Updates `pubspec.yaml` and `CHANGELOG.md` of `image_picker` and `image_picker_macos` Signed-off-by: Ellet <[email protected]>

Signed-off-by: Ellet <[email protected]>

…icker_macos plugin Signed-off-by: Ellet <[email protected]>

…nit test for consistency Signed-off-by: Ellet <[email protected]>

Signed-off-by: Ellet <[email protected]>

github-actions bot added p: image_picker platform-macos labels Nov 14, 2024

EchoEllet force-pushed the feat/phpicker-image-picker-macos branch from 56235d8 to 6843278 Compare November 14, 2024 09:02

EchoEllet commented Nov 14, 2024

View reviewed changes

EchoEllet added a commit to EchoEllet/packages that referenced this pull request Nov 14, 2024

chore: update outdated TODOs as they moved to flutter#8079

78293e6

EchoEllet commented Nov 14, 2024

View reviewed changes

EchoEllet force-pushed the feat/phpicker-image-picker-macos branch 2 times, most recently from c0177fb to 277a923 Compare November 17, 2024 11:31

EchoEllet added a commit to EchoEllet/packages that referenced this pull request Nov 17, 2024

chore: update outdated TODOs as they moved to flutter#8079

5b54403

EchoEllet force-pushed the feat/phpicker-image-picker-macos branch from 277a923 to 6a489e6 Compare November 17, 2024 11:43

EchoEllet added a commit to EchoEllet/packages that referenced this pull request Nov 17, 2024

chore: update outdated TODOs as they moved to flutter#8079

be53ded

Signed-off-by: Ellet <[email protected]>

EchoEllet force-pushed the feat/phpicker-image-picker-macos branch from 6a489e6 to d3535ee Compare November 17, 2024 11:55

EchoEllet added 12 commits November 17, 2024 15:01

chore(example): update outdated macOS example using Flutter CLI (auto…

0937fca

…mated), deprecated @NSApplicationMain attribute Signed-off-by: Ellet <[email protected]>

chore: generate pigeons and mocks with the latest dev dependencies

1781a73

Signed-off-by: Ellet <[email protected]>

chore: format Swift generated file Messages.g.swift

5465142

Signed-off-by: Ellet <[email protected]>

chore: restore backticks of file_selector link

ce49360

Signed-off-by: Ellet <[email protected]>

chore: update supported macOS version from 10.11 to 10.14 for image_p…

e0c7e3a

…icker_macos plugin Signed-off-by: Ellet <[email protected]>

chore: use getVideo() instead of the soft-deprecated pickVideo() in u…

8d8eef9

…nit test for consistency Signed-off-by: Ellet <[email protected]>

chore: update outdated TODOs as they moved to flutter#8079

60035a2

Signed-off-by: Ellet <[email protected]>

test: add minor test for pickVideo

eeb3f4c

Signed-off-by: Ellet <[email protected]>

chore: run dart format for Dart generated files

a123470

Signed-off-by: Ellet <[email protected]>

chore: 'try' precede 'await' in ImagePickerImpl.swift

ce6aed8

Signed-off-by: Ellet <[email protected]>

chore: hardcode the supportsPHPicker() name to fix CI warnings

c9122f9

Signed-off-by: Ellet <[email protected]>

EchoEllet force-pushed the feat/phpicker-image-picker-macos branch from d3535ee to c9122f9 Compare November 17, 2024 12:01

EchoEllet marked this pull request as ready for review November 24, 2024 08:49

EchoEllet requested review from jmagman and tarrinneal as code owners November 24, 2024 08:49

stuartmorgan added triage-ios Should be looked at in iOS triage triage-macos Should be looked at in macOS triage and removed triage-ios Should be looked at in iOS triage labels Nov 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[image_picker_macos] macOS native image picker #8079

[image_picker_macos] macOS native image picker #8079

EchoEllet commented Nov 14, 2024 •

edited

Loading

EchoEllet left a comment

EchoEllet Nov 14, 2024 •

edited

Loading

EchoEllet Nov 14, 2024

EchoEllet Nov 14, 2024

EchoEllet Nov 14, 2024

EchoEllet Nov 14, 2024 •

edited

Loading

EchoEllet commented Nov 14, 2024 •

edited

Loading

EchoEllet commented Nov 14, 2024 •

edited

Loading

EchoEllet commented Nov 14, 2024 •

edited

Loading

EchoEllet Nov 14, 2024

EchoEllet Nov 14, 2024 •

edited

Loading

EchoEllet Nov 17, 2024

		assert(quality != 100, "Quality 100 means no compression.")
		assert(quality >= 0, "Quality can't be negative.")

[image_picker_macos] macOS native image picker #8079

Are you sure you want to change the base?

[image_picker_macos] macOS native image picker #8079

Conversation

EchoEllet commented Nov 14, 2024 • edited Loading

Usage

Related Issues

Limiations

Motivation

IMO

Testing

Additional Context

Pre-launch Checklist

EchoEllet left a comment

Choose a reason for hiding this comment

EchoEllet Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

EchoEllet Nov 14, 2024

Choose a reason for hiding this comment

EchoEllet Nov 14, 2024

Choose a reason for hiding this comment

EchoEllet Nov 14, 2024

Choose a reason for hiding this comment

EchoEllet Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

EchoEllet commented Nov 14, 2024 • edited Loading

EchoEllet commented Nov 14, 2024 • edited Loading

EchoEllet commented Nov 14, 2024 • edited Loading

EchoEllet Nov 14, 2024

Choose a reason for hiding this comment

EchoEllet Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

EchoEllet Nov 17, 2024

Choose a reason for hiding this comment

EchoEllet commented Nov 14, 2024 •

edited

Loading

EchoEllet Nov 14, 2024 •

edited

Loading

EchoEllet Nov 14, 2024 •

edited

Loading

EchoEllet commented Nov 14, 2024 •

edited

Loading

EchoEllet commented Nov 14, 2024 •

edited

Loading

EchoEllet commented Nov 14, 2024 •

edited

Loading

EchoEllet Nov 14, 2024 •

edited

Loading