-
Notifications
You must be signed in to change notification settings - Fork 72
Add runtime permission request to post notification #145
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
base: master
Are you sure you want to change the base?
Add runtime permission request to post notification #145
Conversation
| requestPermissionForAlias("location", call, "locationPermissionsCallback"); | ||
| } else { | ||
| call.reject("Permission denied.", "NOT_AUTHORIZED"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these nested if blocks are easier to understand. Why did you change it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a hard time understanding the flow... I guess it's a matter of taste...
I also saw that keepAlive was called in a place it didn't need to be called due to the nesting of the ifs...
| requestPermissionForAlias("notifications", call, "notificationsPermissionsCallback"); | ||
| return; | ||
| } | ||
| service.addWatcher( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be replaced with a direct call to notificationsPermissionsCallback?
|
This PR certainly needs to implement equivalent functionality for iOS before being merged. |
|
I don't think there's an iOS equivalent to this, this is an Android requirement as far as I understand. |
|
Excellent point. |
|
I've moved to maintain the following plugin: Sorry for splitting the echosystem and community, but I needed full control over the code due to the importance of this plugin in my app. Feel free to copy anything you'd like, or close issues I opened. |
|
Thanks, that is absolutely fine with me. I have little time to maintain this plugin beyond fixing bugs. |
In this PR I did the following:
backgroundMessageis definedThis also solves an issue for when running the app for the first time and there was a code flow that would create the notification and create a watcher even though the permission is not granted, so there was an error in the logcat.