-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[image_picker] Add desktop support #3882
[image_picker] Add desktop support #3882
Conversation
As usual for adding new platforms, this looks like a ton more code to review than it actually is; most of it by volume is example boilerplate:
|
I did this in a single PR because I wanted to approach it from the standpoint of "what do we need to get to something end-to-end workable to endorse desktop. It was useful for discovering things like, it works much better for everyone if the camera delegation goes into the platform interface as an option, rather than duplicated in each platform. Now that I have that though, I'm happy to carve it up into a series of PRs if you would prefer smaller reviews:
|
Platform interface portion of #3882 Adds CameraDelegatingImagePickerPlatform and ImagePickerCameraDelegate, and supportsImageSource Part of flutter/flutter#102115 Part of flutter/flutter#102320 Part of flutter/flutter#85100
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
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
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
Overriding changelog: The remaining macOS changes in this PR are just the binary files from the example app that I accidentally didn't transfer to the other branch because I created it using a patch file. |
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
(This is the combination PR for overall review.)
This adds initial desktop support across all three desktop platforms:
image_picker
in a world of dynamic camera support viable. (In particular this is critical for published packages that useimage_picker
, as they won't know in advance if a client app will have a delegate set up).getImageFromSource
API as an opportunistic fix while changing the code (if not implemented, it would fall back togetImage
)Since in my opinion this level of support is sufficient to allow basic real-world use of the desktop implementations, due to having a structure in place for handling camera support, this includes endorsing the desktop implementations. It is still the case that desktop does not support image resizing, which isn't ideal, but I don't think we should block on that. It is clearly documented in the README.
The desktop implementations are separate packages even though they are mostly the same right now because:
PHPicker
, meaning we can later add native code for that to the macOS implementation, with just a fallback to the currentfile_selector
wrapper. It's plausible that better native options for Windows and/or Linux may become available in the future as well.Fixes flutter/flutter#102115
Fixes flutter/flutter#102320
Fixes flutter/flutter#85100
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style.///
).