Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: rework module using react native bob #421

Merged
merged 10 commits into from
Aug 9, 2021
Merged

Conversation

vonovak
Copy link
Collaborator

@vonovak vonovak commented Jun 13, 2021

Summary

this PR reworks the module to be built on top of react native builder bob which provides some cool benefits out of the box:

  • there is an example project linked to the the module sources, the example project can be used to develop the module itself: this is something immensely helpful for making contributions easier to happen
  • releases will be done automatically, including release notes
  • TS typings are no longer maintained separately - the module JS part was rewritten to TS

closes #343

Test Plan

Example project runs on android and ios device

should run on windows as well: @qmatteoq could you please double check this works fine on windows and add example project that could be used for development? Perhaps datetime picker repo could be of help: https://github.com/react-native-datetimepicker/datetimepicker/tree/master/example

What are the steps to reproduce (after prerequisites)?

see contributing.md

Compatibility

OS Implemented
iOS
Android
Windows ?

Checklist

  • 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 vonovak requested a review from qmatteoq June 13, 2021 11:50
Copy link
Collaborator

@qmatteoq qmatteoq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm everything keeps working fine on Windows. In the next days I will work on a Windows sample to add to the existing ones and I'll submit a PR.

@vonovak vonovak force-pushed the feat/use-bob-with-history branch from 9e9ef9e to 4d46013 Compare June 17, 2021 16:59
@vonovak
Copy link
Collaborator Author

vonovak commented Jun 17, 2021

@qmatteoq perfect, thank you!

There's one more thing: this PR #380 added a folder picker windows-only functionality to the library but was never documented. In a recent PR, the same functionality was added on Android #414, under the pickDirectory name.

Would you be so kind and rework the old, undocumented implementation so that pickDirectory works on windows as well? I believe it should not be too complicated :)

Also, what do you think of the naming? Would you prefer this to be named pickDirectory or selectDirectory or does it not matter in your opinion? Thanks again :)

btw I'll be vacationing now for 10 days so I won't respond here :)

@qmatteoq
Copy link
Collaborator

The way I implemented this feature in Windows is to retrieve a list of all the files in the selected folder, but I realize that this is different from what you have in the other platforms. I will rework the feature to work like on Android and return the path of the selected folder and submit a PR. Enjoy your holidays! 😊

@qmatteoq
Copy link
Collaborator

Also, what do you think of the naming? Would you prefer this to be named pickDirectory or selectDirectory or does it not matter in your opinion? Thanks again :)

I like pickDirectory more, since it's consistent with the existing APIs for files ☺

@vonovak
Copy link
Collaborator Author

vonovak commented Aug 2, 2021

so, the PR is now complete. I will find some time later this week to merge this and set up automated publishing.

@vonovak vonovak merged commit db09e24 into master Aug 9, 2021
vonovak added a commit that referenced this pull request Aug 9, 2021
BREAKING - this commit will publish work done in #421 

* chore: set up automated releases
vonovak added a commit that referenced this pull request Aug 9, 2021
BREAKING CHANGE: includes the work from (#421)
vonovak added a commit that referenced this pull request Aug 9, 2021
BREAKING CHANGE: includes the work from (#421)
@vonovak vonovak deleted the feat/use-bob-with-history branch August 9, 2021 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Typescript definitions
2 participants