Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

added popInitialNotification function #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jawadrehman
Copy link

No description provided.

@jawadrehman
Copy link
Author

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

@oney
Copy link
Owner

oney commented Dec 26, 2015

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
Copy link
Author

@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
Copy link
Owner

oney commented Jan 18, 2016

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
Copy link
Author

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

@RGBz
Copy link

RGBz commented Jan 22, 2016

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

@oney
Copy link
Owner

oney commented Jan 22, 2016

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

@RGBz
Copy link

RGBz commented Jan 22, 2016

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
Copy link
Owner

oney commented Jan 22, 2016

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
Copy link

RGBz commented Jan 22, 2016

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

@jawadrehman
Copy link
Author

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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants