zetavg / react-native-system-notification

Android system notifications for React Native. Supports push notifications with GCM integrated.
https://www.npmjs.com/package/react-native-system-notification
244 stars 102 forks source link

Adding bigText support (expanded notification) && improving notification press event #14

Closed julianocomg closed 8 years ago

julianocomg commented 8 years ago

:beers:

andreyvital commented 8 years ago

:+1:

zetavg commented 8 years ago

Thanks very much! And sorry for the delay since I've been busy around Chinese New Year.

Only one question: why do we need to add sendBroadcast on https://github.com/Neson/react-native-system-notification/pull/14/files#diff-237cc59d2764ba779d3ace196a412e44R46? I think that broadcast will not be caught by the app because the app hasn't been fully started right after the execution ofcontext.startActivity(launchIntent);. Currently I've used launchIntent.putExtra (here) to pass that data while launching the app, pass it into JS through the Java native module here and check it in JS here. So I think it's unnecessary to add these lines. Are there any considerations that I've ignored?

julianocomg commented 8 years ago

Hi, and happy new year :monkey:

So... The module works fine when the app is launching, BUT, the callback wasn't firing when coming from background to foreground. If the app is open in background, and the notification is pressed, the app come to foreground, but without callback :/

zetavg commented 8 years ago

Oh I see. The last doubt is that will the callback be fired multiple times in some cases? But since this is a small issue and might not probably happen (if the app is not running, it won't be able to catch the broadcast in time, or if the app is running in background, the callback won't be fired), I'm merging this first. :smile:

julianocomg commented 8 years ago

In my tests this broadcast is sent only when the app is already running in background (https://github.com/Neson/react-native-system-notification/blob/master/android/src/main/java/io/neson/react-native/notification/NotificationEventReceiver.java#L46)

So I think everything will work as expected now :+1: