-
Notifications
You must be signed in to change notification settings - Fork 356
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
Create a live icon for showing notification counts #4503
Conversation
updates every n-seconds with new notification count
✅ Deploy Preview for gallant-galileo-14878c canceled.
|
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.
Looks good overall! Left a few small comments. Also, I did notice that the live bell is only working for admins/moderators and it's not working for regular commenters. This seems to be due to the @auth
placed on the notificationCount
.
client/src/core/client/stream/tabs/Notifications/LiveBellIcon.tsx
Outdated
Show resolved
Hide resolved
I looked into it and have fixed this issue. The check is now handled gracefully in the resolver and no longer needs an |
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.
looks good! thanks for the update
What does this PR do?
Adds a live icon that shows notification counts.
These changes will impact:
What changes to the GraphQL/Database Schema does this PR introduce?
None, piggybacks on existing
notificationCount(...)
query.Does this PR introduce any new environment variables or feature flags?
Adds the
notificationsPollRate
env var.If any indexes were added, were they added to
INDEXES.md
?N/A
How do I test this PR?
Were any tests migrated to React Testing Library?
None
How do we deploy this PR?
Merge into epic branch and release that.