Skip to content
This repository has been archived by the owner on Sep 4, 2020. It is now read-only.

Background notification should trigger event if possible #397

Closed

Conversation

renanccastro
Copy link

Notification event should always be sent, if possible. This removes the problem where notification gets to the desk, but event handler is never called.

Notification event should always be sent, if possible. This removes the problem where notification gets to the desk, but event handler is never called.
@fredgalvao
Copy link
Collaborator

There's currently only one case where it doesn't immediately dispatch the notification event, which is when it has a title or a message, and in this case it creates a notification whose click will eventually dispatch that notification event as soon as the user interacts with the notification.

Although I see what you're trying to accomplish here, and I agree that it's a good idea (which is to somewhat have two different events, one for notification.receive and another for notification.click, maybe even a third one notification.discard), but I disagree this is the way to go.

With your patch, we would have multiple notification events for the same object (considering it has either title or message):

  • one when it arrives(the one you patched)
  • and another one when the user clicks on it (the one already in place)

and that's kinda inconsistent.
We would at the very least have to document that quirky case.

@macdonst
Copy link
Member

macdonst commented Dec 6, 2015

@renanccastro sorry, while I appreciate the PR I'm not going to merge it. @fredgalvao provides a very detailed explanation as to why this is the case. The way this is going to be handled in the 1.5.0 release is if the notification has no title/message it will call the on('notification') event as it does now. However, if you add a content-available: 1 property to the notification it will force the on('notification') handler to be called just like iOS. That is Issue #293.

@macdonst macdonst closed this Dec 6, 2015
@fredgalvao
Copy link
Collaborator

@macdonst Wouldn't all of these issues regarding background/cold start/content-available/force-show be solved if we just split the notification event into multiple ones like I mentioned? Each one of them would be easily configurable and wouldn't interfere with each other that much. Also, we'd have the path clear for the android part on cold start notifications having no callback.

@lock
Copy link

lock bot commented Jun 3, 2018

This thread has been automatically locked.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants