android: use PermissionListener to avoid Android manual steps #252
Conversation
|
Ping? |
|
Gentle reminder @wmcmahan :-) |
|
Ping... |
|
@saghul can you resolve conflicts please? |
|
Will do! |
The previous commit breaks the public API so it should be semver major.
b370103 to
928a80e
Compare
|
@MoOx rebased, please take a look! |
|
Ping again... |
|
👋 About to celebrate the PRs first birthday here. |
|
I was wondering if we should handle permissions in the codebase as we have a solid react-native-permissions solution this days. Thoughts? |
|
@MoOx I don't know what you mean. This change uses that exact permissions solution, the builtin one. No glue code is necessary to make it work, it piggybacks on the same mechanism used by the PermissionsAndroid module, for instance. What do you suggest? |
|
@saghul I mean we could remove all the "permission" check/request in the codebase of this project & rely on more solid ones like react-native-permissions that can now ask only for a single permissions (without bloating your Info.plist) for iOS. |
|
Why spend the time doing that when this plugin works just fine as is? This PR is a net improvement for Android, because it requires users to do one less step. I've been using our fork for quite a while now on an app installed by millions of users with no issues. I don't want to sound rude, but it's really discouraging to have this un-merged for so long. |
|
Hi @saghul. I feel your frustration. Sorry but I am not the original author of this lib & just tried to help a little. I asked for write access so I can merge PR & triage issues but I don't feel confortable enough with the existing code to feel confident when merging stuff :) |
|
@MoOx Sounds good to me! Thanks for understanding! |
This happens when in the background, for instance.
|
@saghul you should have received an invite! |
|
I will let you merge your own PR :) (please use Squash & Merge to keep the git history concise, which helps for changelog). |
| { | ||
| "name": "react-native-calendar-events", | ||
| "version": "1.7.3", | ||
| "version": "2.0.0", |
There was a problem hiding this comment.
Maybe revert this so we can bump the version only when we cut the release?
|
Cleaning up the repo a little bit, and being more familiar with it, this PR makes TOTAL SENSE. Thanks @saghul ! |
See #243 and #248