-
Notifications
You must be signed in to change notification settings - Fork 258
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
feat: show read notifications #889
Conversation
@@ -23,11 +23,11 @@ describe('hooks/useNotifications.ts', () => { | |||
]; | |||
|
|||
nock('https://api.github.com') | |||
.get('/notifications?participating=false') | |||
.get('/notifications?all=false&participating=false') |
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.
Needing to update the same string throughout is a code smell, FWIW :)
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.
These features are getting easy :). Nice stuff!
Haha yeah, they're starting to flow nicely. I remembered a use-case I need to test with this enhancement (ensure clicking mark as read correctly changes row state before the next scheduled fetch). Will confirm shortly |
If you're happy and you know it, merge conflict |
Moving back to |
Reopening a new PR (#1052) as I've reworked this |
closes #708
Add read / unread toggle in sidebar. Read notifications have a higher opacity and hide the mark as read button on hover-over.
Some GitHub API Limitations I've found:
unread
/notifications
response to indicate if a notification is done or not done (ie: in the inbox)These have been documented in #890