[image_picker] Add desktop support - implementations #4172
Conversation
Platform implementation portion of flutter#3882 Updates the Windows implementation to use the new base class for camera delegation, and creates new macOS and Linux implementations that are near-duplicates. These are separate packages, rather than a single shared package, because it's likely that they will diverge over time (e.g., the TODO for macOS to use a system image picker control on newer versions of macOS), and the amount of code that could be shared is minimal anyway. Part of flutter/flutter#102115 Part of flutter/flutter#102320 Part of flutter/flutter#85100
github-actions
bot
added
p: image_picker
platform-linux
platform-macos
platform-windows
labels
| /// Linux. | ||
| class ImagePickerLinux extends CameraDelegatingImagePickerPlatform { | ||
| /// Constructs a platform implementation. | ||
| ImagePickerLinux(); |
There was a problem hiding this comment.
We don't generally make our platform implementation classes const. I don't think there's much benefit to it, and it would lock them into being const forever without a breaking change. Is there any real upside to it?
| /// macOS. | ||
| class ImagePickerMacOS extends CameraDelegatingImagePickerPlatform { | ||
| /// Constructs a platform implementation. | ||
| ImagePickerMacOS(); |
There was a problem hiding this comment.
Similar question here (and probably for Windows).
| # FOR TESTING AND INITIAL REVIEW ONLY. DO NOT MERGE. | ||
| # See https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins | ||
| dependency_overrides: | ||
| {image_picker_platform_interface: {path: ../../../image_picker/image_picker_platform_interface}} |
There was a problem hiding this comment.
You're probably already aware this is still here.
There was a problem hiding this comment.
the others as well, the platform changes have landed.
There was a problem hiding this comment.
Oops, apparently I screwed up my search (and didn't review the final diff). Fixed.
| } | ||
|
|
||
| // `preferredCameraDevice` and `maxDuration` arguments are not currently | ||
| // supported. If any of these arguments is supplied, they will be silently |
There was a problem hiding this comment.
optional nit: any vs either. Technically any is fine though.
is should be are though, as these are countable, plural, options.
There was a problem hiding this comment.
Fixed everywhere.
| } | ||
|
|
||
| // `maxWidth`, `maxHeight`, and `imageQuality` arguments are not currently | ||
| // supported. If any of these arguments is supplied, they will be silently |
There was a problem hiding this comment.
Fixed everywhere.
| // arguments are not supported on Windows. If any of these arguments | ||
| // is supplied, it'll be silently ignored by the Windows version of | ||
| // the plugin. `source` is not implemented for `ImageSource.camera` | ||
| // and will throw an exception. |
There was a problem hiding this comment.
Shouldn't these deleted comments still be here in some capacity?
There was a problem hiding this comment.
or rather, "deprecation" comments, if there is a newer method.
There was a problem hiding this comment.
Added short comments explaining the soft-deprecation and pointing to the new methods, for all three platforms.
stuartmorgan
added
the
autosubmit
auto-submit
bot
removed
the
autosubmit
|
auto label is removed for flutter/packages, pr: 4172, due to - The status or check suite Mac_arm64 ios_platform_tests_shard_5 master has failed. Please fix the issues identified (or deflake) before re-applying this label.
|
stuartmorgan
added
the
autosubmit
flutter/packages@afe2f05...c9865e8 2023-06-11 engine-flutter-autoroll@skia.org Roll Flutter from da127f1 to 3df163f (25 revisions) (flutter/packages#4183) 2023-06-10 stuartmorgan@google.com [image_picker] Add desktop support (flutter/packages#3882) 2023-06-09 tarrinneal@gmail.com [image_picker] getMedia platform changes (flutter/packages#4174) 2023-06-09 stuartmorgan@google.com [image_picker] Add desktop support - implementations (flutter/packages#4172) 2023-06-09 engine-flutter-autoroll@skia.org Roll Flutter from 6e254a3 to da127f1 (28 revisions) (flutter/packages#4170) 2023-06-09 32538273+ValentinVignal@users.noreply.github.com [go_router_builder] Include required and positional query parameters in the location (flutter/packages#4163) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Platform implementation portion of
#3882
Updates the Windows implementation to use the new base class for camera delegation, and creates new macOS and Linux implementations that are near-duplicates.
These are separate packages, rather than a single shared package, because it's likely that they will diverge over time (e.g., the TODO for macOS to use a system image picker control on newer versions of macOS), and the amount of code that could be shared is minimal anyway.
Part of flutter/flutter#102115 Part of flutter/flutter#102320 Part of flutter/flutter#85100
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).