hansemannn / titanium-firebase-cloud-messaging

Use the Firebase Cloud Messaging SDK in Axway Titanium 🚀 Edit
Other
43 stars 31 forks source link

feat(android): information about android 13 permission, warning if not requested yet #141

Closed m1ga closed 1 year ago

m1ga commented 2 years ago

firebase.cloudmessaging-android-3.4.0.zip

hansemannn commented 2 years ago

Was there also linting involved? Wondering about all the code changed

m1ga commented 2 years ago

Yes, that's the formatted all files in Android Studio! part :wink: That's why I've highlighted the code changes to quickly spot those.

Noticed it a bit too late. I can close it and just do the code update first if you like and then the linting in a different PR without any code changes

hansemannn commented 2 years ago

Ahh okay, sorry for that, overread it. Best would be to split it, at least into two commits. Makes the review much easier

m1ga commented 2 years ago

just need to do some more tests here on different devices.

To have that code in here too: As mentioned in the Ti SDK PR: if you want to use it already you can simply use:

var permissions = ['android.permission.POST_NOTIFICATIONS'];
Ti.Android.requestPermissions(permissions, function(e) {
  if (e.success) {
    Ti.API.info('SUCCESS');
  } else {
    Ti.API.info('ERROR: ' + e.error);
});
hansemannn commented 2 years ago

Pretty cool! Two questions:

  1. When not compiling for Android 13, how do Android 13 devices behave with and without this change?
  2. When compiling for Android (using your Ti-PR), will Android 12 devices work fine even without the permission?
m1ga commented 2 years ago

1: compiling with Ti 11 and running on Android 13 + FCM 3.3.1 will display the push notification permission by itself. You just don't get the feedback of the selection. Google says:

If your app targets 12L (API level 32) or lower, the system shows the permission dialog the first time your app starts an activity after you create a notification channel, or when your app starts an activity and then creates its first notification channel. This is usually on app startup.

2: that's something I'll need to test. But they should behave as before since the permission check is behind the API check.

and for existing apps on an Android 13 device:

In other words, these apps can continue to send notifications to users, and users don't see a runtime permission prompt.

m1ga commented 2 years ago

Tested it on an Android 8 devices:

m1ga commented 2 years ago

Updated the Android part in the readme with an Android 13 section, permission check examples and the example to use the Titanium 12 parts now.

hansemannn commented 2 years ago

Looks perfect! We will need to test it for Android < 13 as well, but looking at the changes it should not cause any issues :)

RavindraChherke commented 1 year ago

Tested (Background and foreground push with Titanium SDK 12.0.0.RC) on

m1ga commented 1 year ago

@hansemannn this was the PR for the new permissions.

It will just print the info POST_NOTIFICATIONS runtime permission is missing. Please request that permission first. if you don't have the permission. The request is done by the SDK. As you can see in the comment above this one it is also tested by other users :+1:

hansemannn commented 1 year ago

Sorry for the confusion, but don't we have a push permission request in the Ti core already? Having it here looks like duplicate logic.

m1ga commented 1 year ago

It will only inform you if you didn't request the permission yet. It's only a warning so you know you have to request it:

Log.w(LCAT, "POST_NOTIFICATIONS runtime permission is missing. Please request that permission first.");
m1ga commented 1 year ago

updated the title and infos in the initial post. ZIP also updated