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(android): use default or incoming notification channelId for foreground notifications #1649

Closed
wants to merge 3 commits into from
Closed

feat(android): use default or incoming notification channelId for foreground notifications #1649

wants to merge 3 commits into from

Conversation

tafelnl
Copy link
Contributor

@tafelnl tafelnl commented Jun 8, 2023

This PR drops the creation of a custom foreground channel on Android. Instead if will derive the channelId to be used for the foreground in the following order:

  1. Firstly it will check if the incoming notification has a channelId set
  2. Then it will check for a possible given value in the AndroidManifest.xml1
  3. Lastly it will use the fallback channelId that the Firebase SDK provides for us. This channel will be created by the Firebase SDK upon receiving the first push message2

1 From the Firebase docs:

From Android 8.0 (API level 26) and higher, notification channels are supported and recommended. FCM provides a default notification channel with basic settings. If you prefer to create and use your own default channel, set default_notification_channel_id to the ID of your notification channel object as shown; FCM will use this value whenever incoming messages do not explicitly set a notification channel.

(see README also)

2 This is similar to how react-native-push-notification is handling this: https://github.com/zo0r/react-native-push-notification/blob/master/android/src/main/java/com/dieam/reactnativepushnotification/modules/RNPushNotificationConfig.java#LL69C19-L69C50

I think this makes way more sense than the current solution. It will also automatically inherit the channel settings set for the incoming notification. So that's more expected behaviour for the developer


By merging this PR #1423 becomes obsolete

Fixes #1368
Fixes #1388

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant