react-native-documents / document-picker

Document Picker for React Native

https://react-native-documents.github.io/

MIT License

1.35k stars 437 forks source link

Added Windows implementation of the sample #443

Closed qmatteoq closed 3 years ago

qmatteoq commented 3 years ago

Summary

This PR adds a Windows implementation of the official example

Test Plan

What's required for testing (prerequisites)?

See the official requirements of React Native for Windows here

What are the steps to reproduce (after prerequisites)?

Run the following command from the root:

yarn example windows

Compatibility

OS	Implemented
iOS	❌
Android	❌
Windows	✅

Checklist

[ x ] I have tested this on a device and a simulator
[ ] I added the documentation in README.md
[ ] I updated the typed files (TS and Flow)

vonovak commented 3 years ago

@qmatteoq great, thanks! Can you please make the PR target the feat/use-bob-with-history branch, so that only the changes pertaining to the windows example project are shown? Somehow I cannot change the target branch. Thanks!

qmatteoq commented 3 years ago

Done. Sorry, I forgot to change the dropdown while opening the PR. Now it should be correct.

qmatteoq commented 3 years ago

@vonovak I have also the branch with the pickDirectory() implementation ready. However, since I worked on my fork, it seems I can't push the branch I've created choosing windows-sample as target branch since it doesn't exist in the repo. How do you suggest to proceed? Once you merge my changes into feat/use-bob-with-history, shall I submit a new PR targeting this branch?

vonovak commented 3 years ago

hello, I see what you mean. okay, let's merge this into use-bob and then you can do that, right?

Is this PR ready? It looks good to me. So you can go ahead and merge it.

Btw, you have push access to this repo, so you don't need to work from a fork :)

btw when merging, watch out for not squashing the commits since it will change their hashes and might mess up your work in the branch with pickDirectory() if I'm not mistaken. I'll leave the merge up to you :)

qmatteoq commented 3 years ago

I've merged the branch and submitted #444 against it, which brings the pickDirectory() implementation. Hopefully, all is good now and ready to be merged into master once you have validated it!

And you're totally right, it seems my vacation reset my brain and I totally forgot I have push access, but when I realized I already did all the work on the fork so I continued there 😊