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
Don't use toastr when notifications are blocked. #2317
Don't use toastr when notifications are blocked. #2317
Conversation
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.
Works as intended, great!
@sebastienvercammen this was discussed in the original PR: |
@pogo-excalibur Yes, it definitely was, but we've gotten reports about changing the behavior because it's more intuitive. |
@sebastienvercammen fair enough 🤷 |
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.
What's the reason for leaving the error-handling function?
@pogo-excalibur Clarification on the decision to change the behavior. |
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.
@sebastienvercammen fair enough
Description
When a client denies Notifications permission, we shouldn't show any notifications at all. We should assume the user intentionally blocked them so they wouldn't see any notifications at all.
toastr fallback is still enabled for browsers that don't support the Notifications API.
Motivation and Context
By popular demand.
Types of changes
Checklist: