-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[path_provider] Android: Support multiple external storage options #2049
Conversation
…-provider-android
…-provider-android
…-provider-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.
Thank you for the PR. I left some comments.
@cyanglaz Thank you for reviewing and will update the PR tomorrow. |
@amirh @cyanglaz there may a problem that's not catch with either approach (enum, String etc): I would therefore assume that the values are not stable and thus, we should not rely on arbitrary string values. Perhaps that's what you meant with public-facing enum & string internally, but I think there needs to be double-mapping. So on dart side enum to a transfer type (like integer) and on native side from integer to the actual constant. WDYT? |
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.
@ened I think what you said makes sense. Do you mind work on that? After you done with it, we might need to add a test to test the mapping.
…-provider-android
@cyanglaz back to you, when you get a chance |
c8d7572
to
f9bca2b
Compare
...h_provider/android/src/main/java/io/flutter/plugins/pathprovider/StorageDirectoryMapper.java
Show resolved
Hide resolved
...h_provider/android/src/main/java/io/flutter/plugins/pathprovider/StorageDirectoryMapper.java
Show resolved
Hide resolved
@cyanglaz sorry to re-request - Github shows more changes required but I can not find them anywhere. |
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.
LGTM!
Description
Some Android phones support multiple external storage options to store files, e.g. SD cards.
This PR adds functionality to
path_provider
so that developers can inquire about those folders.The Android API defines a set of standard folder name constants in
android.os.Environment
which are now bridged to Dart using this PR.Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze
) does not report any problems on my PR.An existing test does not pass currently, this was introduced in #1953. Will include the fix in this PR.
Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?
This is a followup PR of #966
/cc @collinjackson