-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug #307 Fix: Notifications stopped working #356
Conversation
Where's the like button? |
@@ -27,23 +30,13 @@ | |||
return; | |||
} | |||
|
|||
if (document.webkitVisibilityState && document.webkitVisibilityState == 'visible') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is here to make sure we don't show notifications when the page is in focus. Is there a reason you removed it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To emphasize the effect of notifying!
the truth is I removed it while testing to always see the notifications, and decided it's not that bad to keep it that way.
wdyt?
var notification = window.webkitNotifications.createNotification('', title, content); | ||
lastNotification = notification; | ||
//using the 'tag' to avoid showing duplicate notifications | ||
var notification = new Notification(title, {'tag': title+content, 'body': content}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why aren't you using the title anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am using the title.
the title is the first parameter passed to the constructor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦
Bug #307 Fix: Notifications stopped working
Updated to use the newer Notification object (as described in MDN)