Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Conversation

@ened
Copy link
Contributor

@ened ened commented Dec 7, 2018

Most Android phones have, despite a single hardware storage, an internal/external partition layout.
Additionally, some phones support external SD cards for storing files.

Apps may store files onto the internal and external storage folders. For external storage, permissions can not be strongly enforced (hence Android's requirement for the WRITE_EXTERNAL_STORAGE permission) - yet if an App stores files into the app-specific folder, that folder can be cleaned up when an App is uninstalled.

The path_provider was missing 3 parts to work this out which have been addressed by the PR:

  1. Devices with multiple external storage options (like a Sony XZ performance w/ SD card) are now supported and developers can choose to store files on either storage option.
  2. A direct link into the apps specific files & cache folders was missing, thus requiring developers to build it themselves. E.g. /storage/emulated/0 is different than /storage/emulated/0/Android/data/.... Developers should by default use the latter because it should be cleaned up automatically by the Android system when an App is uninstalled
  3. The App specific folder actually supports standard folder types like Music, Photos, Podcasts. Developers can now request this type in the API.

Screenshot on Pixel 2:

Screenshot on Sony XZ performance w/ additional SD card inserted:

@ened ened changed the title Add support for Android devices with multiple external storage options. [path_provider] Add support for Android devices with multiple external storage options. Dec 7, 2018
@ened ened changed the title [path_provider] Add support for Android devices with multiple external storage options. [path_provider] Android: Support multiple external storage options. Dec 7, 2018
@jaggs6
Copy link

jaggs6 commented Jan 10, 2019

@ened can we get this merged please?

@jaggs6
Copy link

jaggs6 commented Jan 10, 2019

@dnfield can we get this merged please?

@dnfield
Copy link
Contributor

dnfield commented Jan 10, 2019

Can we get any tests for this? /Cc @amirh

@amirh
Copy link
Contributor

amirh commented Jan 16, 2019

This is great, thanks for the contribution!

As we currently don't have a harness for testing the platform side of plugins, current and new features are not properly covered by tests.
In order to maintain a quality bar for flutter/plugins we are going to avoid adding new features that aren't critical until they can be properly tested.

The test harness work is tracked in flutter/flutter#10824 and flutter/flutter#26080, your feedback on the testing work will be really appreciated (will it properly support the testing needs of this PR?) some design details should be available on flutter/flutter#10824 soon(contributions to that effort are welcomed!).

Would you mind re-opening this PR with e2e tests once a test harness is ready?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants