Icons and click action in the service worker. #37
Conversation
| body = data.body, | ||
| icon = data.icon; | ||
| // Url needs to be acessed from outside this method. | ||
| url = data.url |
There was a problem hiding this comment.
Do you mean, do this?
body = data.body,
icon = data.icon,
url = data.url;url would be undefined in the notificationonclick if I do that. Maybe because I'm creating a new variable or something? I don't know. Anyway, in the pull, the indent is in the same level than the payload, with makes more sense.
|
Thanks a lot for your PR! |
|
I will try to change the Readme tomorrow, because i need to go to college right now. |
|
I changed the Readme and the SW can handle missing urls and/or icons in the payload. |
| }); | ||
|
|
||
| self.addEventListener('notificationclick', function (event) { |
There was a problem hiding this comment.
I am suspecting this will open same URL for several notification in same instance. As the URL is stored outside the push handeler function scope.
You can check example in MDN documentation
|
@JLucasRS Sorry for late review, was quite busy with work as well as university. |
|
@JLucasRS can you manage some time to work on this? |
|
@safwanrahman Fixed. Problem is, you can't acess data in 'notificationclick' event, BUT when you register the notification on 'push' event, you can add a key, data, which can be accessed in the 'notificationclick' event trough |
|
Thanks a lot @JLucasRS . It would be awesome if any person with significant javascript knowledge would review the changes. |
|
PR looks good to merge. Thanks for the help. |
| }); | ||
|
|
||
| self.addEventListener('notificationclick', function (event) { |
There was a problem hiding this comment.
It actually add eventlistener to all the notification that is served by the service worker. It should be someething like mentioned in the MDN Documentation. Can you please try to do it like this?
There was a problem hiding this comment.
new Notification() seems to throw TypeError and prompts you in the console to use registration.showNotification() instead.
safwanrahman
force-pushed
the
master
branch
from
f94308f
to
18a7b0e
Compare
Notifications now can show icons and open urls when clicked, if provided in the payload from the sever side.