Skip to content
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

Conversation

CalumMurray
Copy link

@CalumMurray CalumMurray commented Jan 10, 2023

Pull request checklist

Please check if your PR fulfills the following requirements:

  • The changes have been tested successfully.
  • A changeset has been created (npm run changeset).

Close #321

@robingenz
Copy link
Member

The permission POST_NOTIFICATIONS is only supported from Android 13 (API Level 33).
Capacitor 4 currently officially supports API level 32 (see this comment). API level 33 will be supported with Capacitor 5. For this reason, this PR has to wait until the release of Capacitor 5. In the meantime you can try to use the following plugin: https://github.com/hermitdemschoenenleben/capacitor-plugin-android-post-notifications-permission

@robingenz robingenz added this to the v2.0.0 milestone Jan 10, 2023
@CalumMurray CalumMurray force-pushed the android-13-notifications-permission branch from a6aecf9 to f71ccb6 Compare January 11, 2023 11:01
@CalumMurray
Copy link
Author

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?

@robingenz
Copy link
Member

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.

@robingenz
Copy link
Member

@CalumMurray Can you please resolve the merge conflicts?

@CalumMurray CalumMurray force-pushed the android-13-notifications-permission branch from f71ccb6 to 6a4e949 Compare April 26, 2023 09:33
@CalumMurray
Copy link
Author

Done. It's been a while since I looked at this, so let me know if it still looks alright.

Copy link
Member

@robingenz robingenz left a 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.

Example: https://github.com/ionic-team/capacitor-plugins/blob/main/push-notifications/android/src/main/java/com/capacitorjs/plugins/pushnotifications/PushNotificationsPlugin.java#L78:L100

"@capacitor-firebase/messaging": minor
---

feat(messaging): Support Android 13+ notification permissions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
feat(messaging): Support Android 13+ notification permissions
feat(android): support Android 13+ notification permission

@robingenz
Copy link
Member

@CalumMurray I just implemented it in another PR so I can finally test the Capacitor 5 version. Thanks for the PR anyway!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants