-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
feat(messaging): Support Android 13+ notification permissions #300
feat(messaging): Support Android 13+ notification permissions #300
Conversation
The permission |
a6aecf9
to
f71ccb6
Compare
Thanks very much for the quick response. I've updated to hopefully fix the build error for when it comes time to merge. As noted in some other comments, the problem is that targeting API level 32 and using an Android 13 device means that the permission is immediately prompted on first open, meaning we can't prime the user for permissions as is best practice. Do you have any insight into why using a forked version with this permission might cause any issues with Capacitor 4 running on older and newer Androids? Is there some feature in Android 13 that is incompatible with Capacitor 4? |
Unfortunately not. I would just give it a try and see if it works. I just can't merge this before Capacitor 5 is released because everyone would need to target API Level 33. |
@CalumMurray Can you please resolve the merge conflicts? |
f71ccb6
to
6a4e949
Compare
Done. It's been a while since I looked at this, so let me know if it still looks alright. |
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.
The last thing that is missing: we need to check the Android version and should only request the permission if it is >= Android 13.
"@capacitor-firebase/messaging": minor | ||
--- | ||
|
||
feat(messaging): Support Android 13+ notification permissions |
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.
feat(messaging): Support Android 13+ notification permissions | |
feat(android): support Android 13+ notification permission |
@CalumMurray I just implemented it in another PR so I can finally test the Capacitor 5 version. Thanks for the PR anyway! |
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run changeset
).Close #321