-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[file_selector_android] Create initial Android implementation of the file_selector package #3814
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
[file_selector_android] Create initial Android implementation of the file_selector package #3814
Conversation
| apply plugin: 'com.android.library' | ||
|
|
||
| android { | ||
| namespace 'io.flutter.plugins.file_selector_android' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stuartmorgan Is there a preference for the org of new Android implementations? This is the standard, but I think new ones should be dev.flutter.packages.<pluginname>.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we should definitely use dev.flutter going forward. A while ago I fixed all the legacy references to io.flutter in the plugins repo, except for Android because changing namespaces is non-trivial so I punted on that part.
|
@stuartmorgan This is ready for another review. I was able to find the problem with integration tests on Android. We use different entry-point target files when we run the tests locally vs when we run it on FTL. I was able to get a proper run on FTL and locally by adding /// Entry point for integration tests that require espresso.
void integrationTestMain() {
enableFlutterDriverExtension();
app.main();
}to both I was thinking of landing this PR as is and following up with a fix to the tooling that will set the entry-point target file based on whether we are running the Dart integration tests vs the Java integration tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just small small things, LGTM with those changes!
| # Name/Organization <email address> | ||
|
|
||
| Google Inc. | ||
| The Chromium Authors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line can be removed.
| // If the size is unknown, the value stored is null. But because an | ||
| // int can't be null, the behavior is implementation-specific, | ||
| // and unpredictable. So as | ||
| // a rule, check if it's null before assigning to an int. This will |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is pretty verbose, can we just remove these two sentences? Keeping just the first and last one seems like plenty of explanation.
| } | ||
| } | ||
|
|
||
| if (bytes == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can only happen if size was null, so it would be simpler to just early return above if size is null, and then initialize bytes directly to a value instead of having it be possibly null.
packages/file_selector/file_selector_android/lib/src/file_selector_android.dart
Outdated
Show resolved
Hide resolved
| String? initialDirectory, | ||
| String? confirmButtonText, | ||
| }) async { | ||
| assert( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same.
… of the file_selector package (flutter/packages#3814)
flutter/packages@6b70804...f89ce02 2023-06-27 hansmuller@google.com Updated rfw/test/material_widgets_test.dart for M3 (flutter/packages#4316) 2023-06-27 tarrinneal@gmail.com [shared_preferences] Adds new `clearWithParameters` and `getAllWithParameters` methods to platform interface. (flutter/packages#4261) 2023-06-26 engine-flutter-autoroll@skia.org Roll Flutter from 042c036 to 96a2c05 (60 revisions) (flutter/packages#4313) 2023-06-26 10687576+bparrishMines@users.noreply.github.com [file_selector_android] Create initial Android implementation of the file_selector package (flutter/packages#3814) 2023-06-26 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump org.json:json from 20230227 to 20230618 in /packages/in_app_purchase/in_app_purchase/example/android/app (flutter/packages#4244) 2023-06-26 me@ghyeok.io [webview_flutter_wkwebview] Adds the `isInspectable` to `WebKitWebViewController`. (flutter/packages#3984) 2023-06-26 stuartmorgan@google.com [ci] Remove jcenter from legacy project (flutter/packages#4306) 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
Android implementation of the file_selector package
Related Links:
flutter/plugins#6468
flutter/flutter#25659
Part of flutter/flutter#110098
Useful Resources:
https://developer.android.com/guide/topics/providers/document-provider
https://developer.android.com/training/data-storage/shared/documents-files
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.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.