Skip to content

feat(local-notifications): allow requesting send notifications permission on Android 13+ devices #472

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

Conversation

agonper
Copy link
Contributor

@agonper agonper commented Mar 24, 2023

This PR updates the local-notifications plugin for hasPermission and requestPermission to work as expected on Android 13+ devices (asking for the corresponding permission). The behaviour is equivalent to that of iOS.

Fixes #336

@cla-bot cla-bot bot added the cla: yes label Mar 24, 2023
@agonper
Copy link
Contributor Author

agonper commented Apr 4, 2023

Hello @NathanWalker, any chance this getting merged anytime soon? Thanks!

@NathanWalker
Copy link
Contributor

@agonper thank you so much for this! Let’s switch to making transient on https://github.com/nativescript-community/perms as a better permissions handler. If able to switch up would be amazing but lemme know, otherwise we could merge and I could switch up in couple days to publish a minor bump.

@agonper
Copy link
Contributor Author

agonper commented Apr 5, 2023

Hello @NathanWalker, you are welcome! Thanks for the reply. I checked the solution you're suggesting for permissions handling, but I saw in the docs that asking for notification delivery permissions in Android is not yet supported by the community alternative. Am I wrong?

@NathanWalker
Copy link
Contributor

Let's confirm with @farfromrefug and we can help add it there if needed.

@NathanWalker
Copy link
Contributor

@agonper
Copy link
Contributor Author

agonper commented Apr 6, 2023

Oh, I did not check the code, just the README 😄

Done! Switched to the suggested solution.

If there's anything else, please let me know.

@NathanWalker
Copy link
Contributor

Sweet! nice work @agonper 🤗

@NathanWalker NathanWalker merged commit 1aa1d0b into NativeScript:main Apr 9, 2023
@NathanWalker
Copy link
Contributor

6.1.0 published with the addition.

@agonper
Copy link
Contributor Author

agonper commented Apr 10, 2023

Great! Thanks for the compliment @NathanWalker!

I tried to install the new version in one of my projects, however, it seems that something happened during the release. The android aar file is missing in the npm package now:

image

Could you have a look please? Thanks!

@agonper agonper deleted the feat/local-notifications/android13-perm branch April 10, 2023 09:51
@NathanWalker
Copy link
Contributor

Hmm @agonper will look in moment and publish patch shortly.

@NathanWalker
Copy link
Contributor

Should be resolved in 6.1.1 @agonper - lemme know if further issue. I added an extra check in the build.all step to ensure that doesn't happen again. Looked to be race condition on copy with the platforms folder on publish.
25b2055

@agonper
Copy link
Contributor Author

agonper commented Apr 10, 2023

I can confirm it's solved! Thank you very much @NathanWalker for the quick check!!

@joaomsg11
Copy link

joaomsg11 commented Jul 18, 2023

Tried to use this with the most recent version of local-notifications but it seems like it is not possible again (i also see that the changes to the readme made in this pr were reverted by the pr in https://github.com/NativeScript/plugins/pull/458/files)
Is it not supported anymore?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[@nativescript/local-notifications] [Android] Requires runtime permission dialog to allow notifications.
3 participants