NativeScript / plugins

@nativescript plugins to help with your developments.
https://docs.nativescript.org/plugins/index.html
Apache License 2.0
190 stars 107 forks source link

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

Closed agonper closed 1 year ago

agonper commented 1 year ago

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

agonper commented 1 year ago

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

NathanWalker commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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

NathanWalker commented 1 year ago

Looks like it's already there πŸ‘ https://github.com/nativescript-community/perms/blob/master/src/perms/index.android.ts#L144

agonper commented 1 year ago

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 commented 1 year ago

Sweet! nice work @agonper πŸ€—

NathanWalker commented 1 year ago

6.1.0 published with the addition.

agonper commented 1 year ago

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!

NathanWalker commented 1 year ago

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

NathanWalker commented 1 year ago

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. https://github.com/NativeScript/plugins/commit/25b2055537b181cd53cc74ed1c42485d8afce6d9

agonper commented 1 year ago

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

joaomsg11 commented 1 year ago

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?