Android: add pickDirectory #414
Conversation
|
Tested on a device but it does not have a slot for SD card, so that part I have only tested in simulator |
|
Ping? |
|
hi, I am sorry, I'll test this out this week! :) |
android/src/main/java/io/github/elyx0/reactnativedocumentpicker/DocumentPickerModule.java
Show resolved
Hide resolved
android/src/main/java/io/github/elyx0/reactnativedocumentpicker/DocumentPickerModule.java
Outdated
Show resolved
Hide resolved
| // internal: content://com.android.externalstorage.documents/tree/primary%3Atest | ||
| // sd card: content://com.android.externalstorage.documents/tree/1DEA-0313%3Atest | ||
| List<String> pathSegments = uri.getPathSegments(); | ||
| if (pathSegments.get(0).equalsIgnoreCase("tree")) { |
There was a problem hiding this comment.
did you write this code by yourself, or is it copied from somewhere (where?)
I'm worried this can vary a lot among android versions and I'm not sure this would work reliably... 🤔
There was a problem hiding this comment.
I wrote it myself. I could only test it with emulator and my phone (Honor 9) which does not have external sd card.
There was a problem hiding this comment.
Hello and thanks for the reply :) I think I need more context to understand.
What is the reason for returning the path from this call? Why not return only the uri?
If the reason is that you want to write a file to that path later on, then I believe the right approach to do this is outlined here https://stackoverflow.com/a/61120265/2070942 and should not be handled in this library.
The approach taken here seems like a heuristic that works for some cases, but may not work in others so I'm hesitant to merging this. We should be using android-system-provided tools to work with uris.
If the only thing returned from the call were the uri, then I'd be happy with this.
There was a problem hiding this comment.
What is the reason for returning the path from this call? Why not return only the uri?
This is so that you can then use react-native-fs to access the directory. It may stop working in Android 12 but I suppose it's better than nothing.
If the reason is that you want to write a file to that path later on, then I believe the right approach to do this is outlined here https://stackoverflow.com/a/61120265/2070942 and should not be handled in this library.
Sure, but then you won't be using react-native any more and would be just writing a native app.
If the only thing returned from the call were the uri, then I'd be happy with this.
As far as I'm aware you can't do anything with that URI from RN, without going native.
There was a problem hiding this comment.
The approach taken here seems like a heuristic that works for some cases, but may not work in others so I'm hesitant to merging this. We should be using android-system-provided tools to work with uris.
Ok, fair enough.
|
Hello @roman-r-m, sorry if I made it sound like I am against merging this PR - I am not. The only thing I do not fancy is the part about coverting the uri to path. I do believe it makes sense to choose a folder using this lib and return its uri. Whatever happens with the uri next is up to the library user. I do not believe we should do anything more - eg. If you want to write a file to the uri using some library (eg. React Native fs) then that library should handle the job of converting that uri to a path eventually. Hope this makes sense, thanks! |
|
Ok, thanks. I'll resubmit without the path. |
|
thank you for the PR! :) |
Resolves #302
Adds a method to show system directory picker on Android. Returns the URI of the directory and if possible resolves the URI to the actual path.
README.md