invertase / react-native-notifee

Moved to https://github.com/invertase/notifee
https://invertase.io/blog/open-sourcing-notifee
Other
465 stars 31 forks source link

FCM Android - backwards compatible #274

Closed adam-sajko closed 3 years ago

adam-sajko commented 3 years ago

Do you plan to add support for standard notifications (contains title and body) in future versions? Anything like "Notification Service Extension" for iOS. It's just about to get such a notification with notifee.getInitialNotification() and catch it if the user clicks on it.

Context: We need to support notifications sent the old way (with title and body, but without actions). At least for a while. We currently support both react-native-push-notification and notifee. And it works. But it requires some extra tangled code along with notifee.

Notifications sent the old way:

{
  notification: {
    title: 'TITLE',
    body: 'BODY'
  },
  data: {
    ...
  }
}

Data-only notifications:

{
  data: {
    notifee: {
      title: 'TITLE',
      body: 'BODY',
      ...
    }
  }
}
mikehardy commented 3 years ago

I think that was just released? https://notifee.app/react-native/docs/release-notes#120

Hopefully that's what you mean, and if so it is brand new so feedback is welcome, to know if it's working well or not

adam-sajko commented 3 years ago

@mikehardy Yup, this is for iOS, it would be nice to have something like this for Android as well. Currently, the integration with FCM focuses on sending a notification without title and body (data-only), so it's only delivered and local notification is displayed based on the payload. Standard notifications are displayed immediately. That's why it would be nice to handle such notifications as well.

adam-sajko commented 3 years ago

So all that is needed is to deliver such a notification to JS

mikehardy commented 3 years ago

Ah, interesting. This is focused on iOS because that's where the delivery unreliability is. Have you experienced unreliable delivery on data-only messages on android? Just curious - I have not, so haven't thought of it as a priority.

I could see an argument for it simply for harmony between platforms now that notification extension exists for ios though, I'm not for or against, just an honest question about android reliability in your usage

adam-sajko commented 3 years ago

I think, this is not a delivery problem to the app. It's more about launching the app through notification and read the notification in JS. For data-only messages it's okay. I'm receiving events in JS. But for standard notifications, the events are not delivered to JS. My case is that the app needs to support standard notifications for a while so the user doesn't lose support for those notifications that he received earlier.

I hope I cleared it up a bit :)

It's not that important for new apps, but for apps that migrate from the old system it can make things easier ;)

adam-sajko commented 3 years ago

so to support the old notifications you still need to use the old library:


import notifee from '@notifee/react-native'
import messaging, { FirebaseMessagingTypes } from '@react-native-firebase/messaging'
import RNPN from 'react-native-push-notification'

...

// {
//   notification: {
//     title: 'TITLE',
//     body: 'BODY'
//   },
//   data: {
//     ...
//   }
// }

if (Platform.OS === 'android') {
  RNPN.configure({
    popInitialNotification: false,
    requestPermissions: false,
    onNotification: handleNotification,
    onRegistrationError,
    onRegister,
  })

  ...

  RNPN.popInitialNotification(handleNotification)
}

//  {
//   data: {
//     notifee: {
//       title: 'TITLE',
//       body: 'BODY',
//       ...
//     }
//   }
// }

messaging().onMessage(handleMessageReceived)
messaging().setBackgroundMessageHandler(handleMessageReceived)
notifee.onBackgroundEvent(handleBackgroundEvent)

...

notifee.getInitialNotification()
notifee.onForegroundEvent(handleForegroundEvent)

...
helenaford commented 3 years ago

Hey, thanks for the question. This is something that I've been looking into to because I understand by having data-only for android and not iOS could produce platform-dependent code.

I'm not sure if I understand the issue around having to still use RNPN as Firebase message provide getInitialNotification. That should remove the need for that package, right?

helenaford commented 3 years ago

Closing this in favour of https://github.com/notifee/react-native-notifee/issues/183