react-native-documents / document-picker

Document Picker for React Native
https://react-native-documents.github.io/
MIT License
1.35k stars 440 forks source link

Added pickDirectory implementation for Windows #428

Closed qmatteoq closed 3 years ago

qmatteoq commented 3 years ago

Summary

This PR provides the pickDirectory() feature implementation for Windows. After selecting a folder from your Windows device, the function will return the full path of the folder.

Test Plan

  1. I've installed the module from my branch on a RNW project
  2. I've added the following function in the App.js file and I've connected it to the onPress event of a Buttoncontrol:
const pickFolder = async () => {
  var res = await DocumentPicker.pickDirectory();
  console.log(res.uri);
}
  1. I've observed the folder picker UI appearing. I've selected a folder.
  2. In the debugger log, I've observed the full path of the selected folder being displayed.

What's required for testing (prerequisites)?

No special prerequisites, other than a React Native application with Windows support.

What are the steps to reproduce (after prerequisites)?

  1. Add the following function in your code:
const pickFolder = async () => {
  var res = await DocumentPicker.pickDirectory();
  console.log(res.uri);
}
  1. Add a Button control in the page which invokes this function when it's pressed.

Compatibility

OS Implemented
Windows ✅

Checklist

qmatteoq commented 3 years ago

Once this PR is merged, I'll work on adding the Windows implementation of the example which is included

vonovak commented 3 years ago

Hello @qmatteoq, it would be best if the target branch of this PR was not master; if you look at https://github.com/rnmods/react-native-document-picker/pull/421 (branch use-bob-with-history; dont mind the weird name 😄), it has a new ios and android example project, and a bunch of changes to js files too (all is rewritten to TS). The goal now is to have an windows example project in that branch, and the folder picking feature implemented+tested against that project. That branch will be merged to master later.

Feel free to push into that branch directly if you want, but if you want to do it the PR-way then I think there should be

(1) branch A checked out from use-bob-... targeting use-bob-... branch that adds windows example project (2) branch B checked out from A and targeting A that adds folder picking on windows. You can use the example project from branch A to develop and test everything.

We then merge B into A, A into use-bob-... and use-bob-... into master. Then we're done. 🙂

If we were to merge this into master now then use-bob- would have conflicts and it just wouldnt be pretty. Does this make sense? Thanks!

qmatteoq commented 3 years ago

Totally makes sense and sorry for submitting the PR to the wrong branch! I'll work on it next week. Do you have any preference on the approach among the two you've listed?

vonovak commented 3 years ago

Totally makes sense and sorry for submitting the PR to the wrong branch! I'll work on it next week. Do you have any preference on the approach among the two you've listed?

I guess the PR approach is a little cleaner but I'll leave it up to you. Thank you and no worries!🙂

vonovak commented 3 years ago

@qmatteoq hello, how's it going? :) Do you have an estimate on when the PR could be done agains the other branch as we discussed? :) thanks!

qmatteoq commented 3 years ago

I truly apologize for the silence, I should have given you a status update sooner. Unfortunately I had a crazy last working week before my holidays and I wasn't able to do much. I'm OOF until 26th July. I have my PC with me, so I might be able to take a look before, but I'm not able to give you a more precise timeline. Sorry!

vonovak commented 3 years ago

@qmatteoq no worries at all, just take your time off! I was just thinking you might've forgotten :) Enjoy your holidays!

qmatteoq commented 3 years ago

Hi @vonovak , I'm back and I've started working on the first task (creating a branch out from use-bob-with-history and add the Windows example). However, I'm hitting some issues, likely because I have lot of Windows experience, but I'm fairly new to React Native, so I'm messing up something for sure 😀 From a native perspective, everything is fine: I've added the windows project and I can build it and deploy just fine. However, when I try to launch the Metro package and open the application, I get weird errors like this one:

Unable to resolve module C:\src\react-native-document-picker\src\index from C:\src\react-native-document-picker\example\src\App.tsx: C:\src\react-native-document-picker\src\index could not be found within the project or in these directories:
  node_modules
  ..\node_modules

Additionally, Visual Studio Code says it's not able to resolve react-native-document-picker in App.tsx.

How is the example project configured? Is there any special configuration to setup? Another thing I've noticed is that yarn command doesn't work. I see that there's a .yarnrc file to enable some special configuration, but the outcome on my machine is that every yarn command is simply ignored.

Thanks for the help!

vonovak commented 3 years ago

@qmatteoq hi, hope you enjoyed your vacation! :) ah, sorry to hear that! :/ unfortunately, I don't have any windows RN experience, so I'm not sure I'll be able to help.

having a RN library and an example project that would use its sources is a little tricky to set up which is why I opted to use https://github.com/callstack/react-native-builder-bob to bootstrap everything. Unfortunately, windows is not supported there yet.

specifically: https://github.com/rnmods/react-native-document-picker/blob/4d46013f93bedf6778cfc3c23077e7cb9259ce43/example/metro.config.js#L6 is to make the example project "see" the JS sources

this is for ios files: https://github.com/rnmods/react-native-document-picker/blob/4d46013f93bedf6778cfc3c23077e7cb9259ce43/example/ios/Podfile#L11

this is for android files: https://github.com/rnmods/react-native-document-picker/blob/4d46013f93bedf6778cfc3c23077e7cb9259ce43/example/android/settings.gradle#L6

as for yarn commands: https://github.com/callstack/react-native-builder-bob/pull/165/files as for windows example, maybe this will help https://github.com/callstack/react-native-builder-bob/pull/172/files

I think I will just carry on and merge https://github.com/rnmods/react-native-document-picker/pull/421 so that we have that done at least on android and ios, when I have time. pickDirectory was never documented on windows, so I don't think many people will be blocked by not having it.

Hope this makes sense and hope you'll be able to figure it out! Maybe other projects that have windows support will help: https://github.com/callstack/react-native-builder-bob/issues/69

thanks!

qmatteoq commented 3 years ago

Can you give me until tomorrow? I think I have figured it out thanks to your pointers and the sample app is now working 🙂 I just have to fix a couple of methods in the implementation.

vonovak commented 3 years ago

Hi, good to hear! Yes, sure, I'll wait a few a days 🙂

qmatteoq commented 3 years ago

I've opened #443 to add the Windows implementation of the sample. Not everything is working, but now as planned I'm going to create a new branch out from this branch, which will contain the pickDirectory() implementation plus all the fixes required to align it to the latest JavaScript code.

qmatteoq commented 3 years ago

I think this one can be closed and the corresponding branch deleted.

vonovak commented 3 years ago

closed in favor of #444