evollu / react-native-fcm

react native module for firebase cloud messaging and local notification
MIT License
1.73k stars 682 forks source link

Help me wrap my head around this (android) #298

Closed chris-pikul closed 7 years ago

chris-pikul commented 7 years ago

Ok, so I have the notifications working for the most part, but I'm confused on a couple parts. Maybe it's due to the differences with iOs and Android. But currently my focus is Android since my iOs notifications work fine.

My problem is a couple. 1) I'm sending notifications from server to client with high priority and localized notification messages, as well as data content. Consider a "new message from X" notification. I don't get the heads-up banner at all, is there something I'm missing here? 2) With that same "new message from X" notification, I would like to prevent the notification from going to the system task bar if they are already viewing that thread. Also, cancelling any notifications that might be in there when they go to the thread. Is this possible?

Is there something I'm missing to get this done? It seems like the docs lead to an idea of doing 2 different notifications; 1 with payload data only, and 1 without and doing some sort of monkey work with specific property names?

evollu commented 7 years ago
  1. For android to show banner, you need to use customize_notification feature of this repo. FCM itself doesn't support notification priority
  2. You can use will_present_notification flag introduced in v6 to prevent notification from showing for iOS when app is foreground.
  3. for android, pass id in custom_notification and call cancelLocalNotification(id) to cancel it. (haven't tried yet)
chris-pikul commented 7 years ago

Sorry, been busy lately. Thanks for the reply. We use language translations and formatting for notification information (title, body). These include parameters too, like user name, message, etc. Will that work with the custom_notification object?

evollu commented 7 years ago

if you translate on your server side, then just pass translated text to the object. Use it the same way you use push notification

chris-pikul commented 7 years ago

Ahh, ok. That's currently a no-go for what I wanted. We used the localization formatting so that the notifications where device dependent (bi-lingual users) instead of user object dependent. I may look into switching to that method though.

Would double notifications work you think? As in 1 for the actual notification (title, body, badge, etc.) and another that's just data? But I imagine this will screw with the local presentation. Either way my biggest priority is getting the data to background apps so the content is already updated when the app resumes.

evollu commented 7 years ago

you can try that. The notification won't show up when these is no body text

evollu commented 7 years ago

@ChrisPikul510 is this resolved?

chris-pikul commented 7 years ago

I'm moving to a horizontally scaled cluster configuration, so I won't have any details on this for a while. You can close it if you want I suppose. Or leave it open if you want to know my results.

chris-pikul commented 7 years ago

@evollu So I decided to re-write the module for FCM (sorry!). Loosely based on what you've done. But I changed around some of the data hand-offs and such. Either way, more importantly is I've added support for Vibration Patterns, Light Settings, and am doing the custom styles now as well such as Inbox, Messaging, and probably Media and BigPicture styling as well. If interested I can package up my changes into a repo and potentially merge into this if you want.

evollu commented 7 years ago

@ChrisPikul510 Pull request is always welcomed. Just make sure the change is backward compatible as much as possible

This package has Vibration Patterns and Light Settings I think so I don't know how different it is from what you implemented. Also why changing data hand-offs?

chris-pikul commented 7 years ago

Well, I don't remember why I did to be honest. Basically when the receive message event happens it does a few things. In the end the object that get's returned to the JS event looks like this...

{
    "fcm": {
        "from": "SOMETHING",
        "collapseKey": "SOMETHING",
        "messageId": "SOMETHING",
        "sentTime": 143455267
    },
    "notification": {
        // Either the notification object from FCM, or the presentation payload from data
    },
    "data": {
        // The rest of data that isn't the "presentation" key
    }
}

I believe my idea was to make it as ambiguous as possible if the notification information was needed in JS. So whether you use the presentation key (I renamed it from "custom_notification") or the standard FCM notification object it'll appear the same at the JS level.

Also, when building the notification object to present to the tray it goes through some validation by parsing the JSON object and building it from that. I also don't set any values on the NotificationCompat.Builder object unless the value was present from the JSON in the first place.

And vibration was in this repo, but not patterns. So it can be supplied a int (supposed to be long) array for the vibration on/off times.

Lights where included in this repo, but only as a default setting and no customization. In mine you can customize the color and timing.

I also added localization of notifications using keys and argument arrays. They use the strings from res/values/strings.xml and their language counterparts. This works for title, and body, and formatting.

I'm currently working on "bundling" or the group attribute. The implementation in this repo wasn't (or at least doesn't seem) correct. So I'm also adding an extra sub-object for the group summary object. I'll be doing the same thing with the InboxStyle and MessagesStyle implementations.

WHEW! All in all it's working again, but I didn't include the local schedualing so I can't really help with a PR here. And I REALLY changed the way the notifications get published. So I can either share the helpful bits you might need, or when I package it up (if I do) then we can merge them into a version 2 or something.

evollu commented 7 years ago
I believe my idea was to make it as ambiguous as possible if the notification information was needed in JS. So whether you use the presentation key (I renamed it from "custom_notification") or the standard FCM notification object it'll appear the same at the JS level.

make sense. but I'm assuming all developers care about is the data so I just make sure that this part is same across different types of notification. If they don't match today, a pull to fix them is welcomed. But changing the whole data structure may not worth it.

Also, when building the notification object to present to the tray it goes through some validation by parsing the JSON object and building it from that. I also don't set any values on the NotificationCompat.Builder object unless the value was present from the JSON in the first place.

I'm adding default so that it matches most notification behavior we see today. I think I will keep these presets

And vibration was in this repo, but not patterns. So it can be supplied a int (supposed to be long) array for the vibration on/off times.

Lights where included in this repo, but only as a default setting and no customization. In mine you can customize the color and timing.

I also added localization of notifications using keys and argument arrays. They use the strings from res/values/strings.xml and their language counterparts. This works for title, and body, and formatting.

I'm currently working on "bundling" or the group attribute. The implementation in this repo wasn't (or at least doesn't seem) correct. So I'm also adding an extra sub-object for the group summary object. I'll be doing the same thing with the InboxStyle and MessagesStyle implementations.

These are nice add-on features and a pull request is very welcomed.

Thanks for sharing beforehand 👍

chris-pikul commented 7 years ago

Cool, I'll report back when I get it done.

evollu commented 7 years ago

close due to inactivity