oney / react-native-gcm-android

GCM for React Native Android
MIT License
172 stars 76 forks source link

added popInitialNotification function #19

Open jawadrehman opened 8 years ago

jawadrehman commented 8 years ago

This function allows you to access the "payload" sent with your notification.

oney commented 8 years ago

Thanks!

One question. Why don't you put popInitialNotification in public Map<String, Object> getConstants() rather than here I think putting it in getConstants will be more like official react native implementation

jawadrehman commented 8 years ago

@oney I didnt realize that could be done. I think that will be a better approach since then we dont need to use an event listener. Also I was thinking that we need to clear that notification after it has been "popped".

oney commented 8 years ago

Currently, this module uses https://github.com/Neson/react-native-system-notification to create notification in 0.1.7. You can get payload of clicked notification in DeviceEventEmitter.addListener('sysNotificationClick' listener. Please see whether it meets your requirements. I still more like use popInitialNotification way to get payload, but I prefer to handle creating notification in react-native-system-notification rather than react-native-gcm-android.

jawadrehman commented 8 years ago

DeviceEventEmitter.addListener('sysNotificationClick' doesnt work when the app is not already open.

RGBz commented 8 years ago

Yeah DeviceEventEmitter.addListener('sysNotificationClick'... only works when the app is open. popInitialNotification would be ideal.

oney commented 8 years ago

@RGBz absolutely agree with you. I will remind the author of react-native-system-notification this issue, and wait him to fix it.

RGBz commented 8 years ago

Thinking about it a bit more, a callback-based approach like this implementation of popInitialNotification or DeviceEventEmitter.addListener('sysNotificationClick'... is not what you'd want to handle opening the app via notification because it means the app will: 1) Open up and start rendering a screen (like an apps home screen) 2) Process the DeviceEventEmitter.addListener('sysNotificationClick'... callback 3) Render the screen you'd want to show for the notification

The PushNotificationIOS.popInitialNotification API returns its value inline which instead of using a callback. This would allow for you to open the app right up to the screen you'd want on the first render pass.

@oney What are your thoughts? If you agree I can look into making a popInitialNotification that returns its value inline as well.

oney commented 8 years ago

My opinion is same as you, directly get payload from popInitialNotification like PushNotificationIOS.popInitialNotification, not from or on a callback. I have mentioned here. Because the notification is created from react-native-system-notification module, the better way of this implementation is making PR in that module instead of here.

RGBz commented 8 years ago

I agree, I'll hop over there. Thanks!

jawadrehman commented 8 years ago

I agree a more inline approach would have been suitable. Wasn't sure how to make that work.