titanium-sdk
titanium-sdk copied to clipboard
feat(android): remoteNotificationsEnabled for Android 13
- add parity for
remoteNotificationsEnabled
This will return true
is you enable push notifications in the app settings:
even if it is false
you will still get a token from FCM! It just won't display a push notification.
To test it:
- ~use https://github.com/tidev/titanium_mobile/pull/13473 to have the latest support libraries~ (merged)
- all target and compile SDKs have to be set to "Tiramisu" (value has to be a string, so add " " around it)
- change compileSdkVersion/targetSdkVersion to compileSdkPreview/targetSdkPreview
- build apps and install them by hand with
adb install -r -t app.apk
since it is a test version
TODO:
- other Android 13 push stuff
Tested it in my firebase test app
console.log("remoteNotificationsEnabled", Ti.Network.remoteNotificationsEnabled);
The new push implementation is strange :smile: Even when I have it off I was able to send a push notification :thinking:
You can request the permission with:
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);
});
and after that Ti.Network.remoteNotificationsEnabled
is true. I'll add that to fcm.registerForPushNotifications()
so it will ask for permissions. Thought about using Ti.Network.registerForPushNotifications()
but we don't get a token, just the permission so we still need the fcm part.
Tested it on an Andorid 8 device. Ti.Network.remoteNotificationsEnabled
returns true (as expected) :+1:
This PR is great. But it does much more than mentioned in the title. We should therefore either split it up in Android 13 support and push notification changes or change the title to make it more expressive. Also more testing from the community would be great!
you're right! the remoteNotificationsEnabled
is only one part since we have the new Android 13 permission now. I've change the title.
Looks fine from my end! Should we also bump some library versions? For example, material components and some other patch-version bumps? And yeah, I would also prefer having the permission check in registerForPushNotifications
, even if we don't get a token on Android. It simplifies the cross platform permission handling a lot already.
Sorry, missed the notification! Will have a look at it :+1:
material-components is the latest version already https://mvnrepository.com/artifact/com.google.android.material/material?repo=google (1.6.1). The rest is still alpha
@m1ga I added the parity, updated the docs and fixed the version check (I suppose - as Manifest.permission.POST_NOTIFICATIONS
should be checked for Android 13+, not Android 12+). Maybe I missed something and it was on purpose, so it would be great if you could check the commit in detail. After that, we should be good to go!
Here is my test case:
const window = Titanium.UI.createWindow();
window.addEventListener('open', () => refreshPermissionState());
const label = Ti.UI.createLabel({ top: 100 });
const btn = Ti.UI.createButton({ title: 'Request permissions', top: 200 });
btn.addEventListener('click', () => requestPermissions());
window.add([label, btn]);
window.open();
function refreshPermissionState() {
const enabled = Ti.Network.remoteNotificationsEnabled;
label.text = `Push Notifications enabled: ${enabled}`;
}
function requestPermissions() {
Ti.Network.registerForPushNotifications({
success: () => {
alert('Push notifications ready!');
refreshPermissionState();
},
error: () => {
alert('Push permissions denied!');
refreshPermissionState();
}
});
}
Found two libs that have a newer version. Will test that together. Rest looks update to date (I've checked those in a different PR before)
First quick tests (firebase push test app, my climbing app) looked good: permission check was shown, app works fine (I have a version of my app in store already with the first part of this PR :wink: ) But I'll do some more tests tomorrow.
BTW: since most people will use the FCM module in combination with this PR it will add POST_NOTIFICATIONS
permission automatically. But it is still good to mention it in the docs :+1:
Ready to merge from my end. If there are no other objections, this is good to go!
Merge it :rocket: Hopefully more people will test the next RC to get some more feedback from other apps.