-
Notifications
You must be signed in to change notification settings - Fork 16
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
fix(notifications): stop alert notification if alert would notify of message of current active chat #3267
Conversation
✅ Yeeeehaw, deploy preview is ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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.
Could you explain the decision to change alert.state into a configurable param? Currently, we only have READ or UNREAD. Is there a plan to create other states, or is there a use case where we want to create a READ alert?
I can foresee both needing to happen honestly. Like an app update alert that auto pops up on launch, still an alert, but they are forced to see it originally so it's already read. |
Thanks, was just curious. Sounds good |
Hey Jeff! I went in and retested and found one small issue, the red badge that appears on alerts isnt showing up for me for some reason (I'm in a different chat when receiving messages) I did a new server and also tried replicating on dev branch as well. When I do open alerts all the messages that were coming in appear there just no red badge no matter which chat you are in alerts.notification.mov |
what the freaking heck |
Should be fixed now! Dumb fix |
@Jekrimo worked perfect on my end, and amazing response time with the fix 😂 |
What this PR does 📖
Don't have alert for chat that is active
Which issue(s) this PR fixes 🔨
AP-1502