-
Notifications
You must be signed in to change notification settings - Fork 164
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
Show notification permission request prompt on Android T+ #731
Show notification permission request prompt on Android T+ #731
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Thanks Martin! |
I've patched my npm install (v 1.18.1) with these updates to allow notifications on Android 13+. Should I be getting a prompt on the device to enable notifications when the app installs? Currently, I'm getting the effect of notifications defaulting to disabled and not providing an option to the user to enable them. |
@alexzlaten The prompt should appear when calling |
@marto55555 Thanks for that. So the call must be from user action? I find if I call Notification.requestPermission() from the console it is immediately denied without a prompt. |
@alexzlaten Actually, I was wrong - a user action is not needed, sorry. Calling it directly from the console worked for me. Have you tried uninstalling the app so that it resets Android's requested permissions history? Or maybe something went wrong with the patching. |
@marto55555 I have uninstalled/reinstalled several times and cleared site settings in chrome. I even enabled notifications for the app, uninstalled it, and reinstalled. Notification were disabled and Notification.requestPermission() returned "denied". The patch seems pretty simple, but perhaps I did something incorrectly. My version has FeatureManager.js rather than FeatureManager.ts. Maybe it's time for me to checkout the project and make a build with your changes rather than patching? That will be a new adventure for me. |
@marto55555 Success! I was targeting SDK 31 and it needed 33 for this feature. I uploaded a test to the play store and the notification permissions were requested by the OS kicked off by Onesignal's library. Thank you for the help! |
This makes notifications work again on Android 13 (and newer) by declaring an activity for showing the notification permission request prompt.
Potentially fixes #730