From 7aa82006e6d92da256c3ffaa81cb74c06463ff04 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 19 Apr 2024 16:42:42 -0400 Subject: [PATCH 1/2] feat: show read notifications --- src/__mocks__/mock-state.ts | 1 + src/components/NotificationRow.tsx | 80 +++++++++++++++++++++++------- src/components/Repository.test.tsx | 8 +-- src/components/Repository.tsx | 52 +++++++++++++++---- src/context/App.test.tsx | 6 +-- src/context/App.tsx | 17 +++++-- src/hooks/useNotifications.test.ts | 24 +++++++-- src/hooks/useNotifications.ts | 68 ++++++++++++------------- src/routes/Settings.tsx | 8 +++ src/types.ts | 1 + src/utils/constants.ts | 2 + 11 files changed, 187 insertions(+), 80 deletions(-) diff --git a/src/__mocks__/mock-state.ts b/src/__mocks__/mock-state.ts index 01cb113a3..22b06f519 100644 --- a/src/__mocks__/mock-state.ts +++ b/src/__mocks__/mock-state.ts @@ -18,4 +18,5 @@ export const mockSettings: SettingsState = { detailedNotifications: false, markAsDoneOnOpen: false, showAccountHostname: false, + showReadNotifications: false, }; diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index 517b5d7f9..cfcb1c998 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -16,6 +16,7 @@ import { import { AppContext } from '../context/App'; import type { Notification } from '../typesGithub'; import { openExternalLink } from '../utils/comms'; +import Constants from '../utils/constants'; import { formatForDisplay, openInBrowser } from '../utils/helpers'; import { getNotificationTypeIcon, @@ -32,21 +33,32 @@ export const NotificationRow: FC = ({ notification, hostname }) => { const { settings, accounts, - removeNotificationFromState, markNotificationRead, markNotificationDone, + removeNotificationFromState, unsubscribeNotification, notifications, } = useContext(AppContext); + const setNotificationAsOpaque = () => { + if (notification.unread) { + const notificationRow = document.getElementById(notification.id); + notificationRow.className += Constants.READ_CLASS_NAME; + } + notification.unread = false; + }; + const openNotification = useCallback(() => { openInBrowser(notification, accounts); if (settings.markAsDoneOnOpen) { markNotificationDone(notification.id, hostname); - } else { - // no need to mark as read, github does it by default when opening it + } + + if (!settings.showReadNotifications) { removeNotificationFromState(notification.id, hostname); + } else { + setNotificationAsOpaque(); } }, [notifications, notification, accounts, settings]); // notifications required here to prevent weird state issues @@ -55,6 +67,33 @@ export const NotificationRow: FC = ({ notification, hostname }) => { event.stopPropagation(); unsubscribeNotification(notification.id, hostname); + markNotificationRead(notification.id, hostname); + + if (!settings.showReadNotifications) { + removeNotificationFromState(notification.id, hostname); + } else { + setNotificationAsOpaque(); + } + }; + + const markAsRead = () => { + markNotificationRead(notification.id, hostname); + + if (!settings.showReadNotifications) { + removeNotificationFromState(notification.id, hostname); + } else { + setNotificationAsOpaque(); + } + }; + + const markAsDone = () => { + markNotificationDone(notification.id, hostname); + + if (!settings.showReadNotifications) { + removeNotificationFromState(notification.id, hostname); + } else { + setNotificationAsOpaque(); + } }; const openUserProfile = ( @@ -82,18 +121,21 @@ export const NotificationRow: FC = ({ notification, hostname }) => { ]); return ( -
+
-
openNotification()} - onKeyDown={() => openNotification()} + onClick={openNotification} + onKeyDown={openNotification} >
= ({ notification, hostname }) => { type="button" className="focus:outline-none h-full hover:text-green-500" title="Mark as Done" - onClick={() => markNotificationDone(notification.id, hostname)} + onClick={markAsDone} > @@ -155,14 +197,18 @@ export const NotificationRow: FC = ({ notification, hostname }) => { - + {notification.unread ? ( + + ) : ( +
+ )}
); diff --git a/src/components/Repository.test.tsx b/src/components/Repository.test.tsx index a2e5d877f..75973f5ab 100644 --- a/src/components/Repository.test.tsx +++ b/src/components/Repository.test.tsx @@ -10,7 +10,7 @@ jest.mock('./NotificationRow', () => ({ })); describe('components/Repository.tsx', () => { - const markRepoNotifications = jest.fn(); + const markRepoNotificationsRead = jest.fn(); const markRepoNotificationsDone = jest.fn(); const props = { @@ -20,7 +20,7 @@ describe('components/Repository.tsx', () => { }; beforeEach(() => { - markRepoNotifications.mockReset(); + markRepoNotificationsRead.mockReset(); jest.spyOn(shell, 'openExternal'); }); @@ -51,14 +51,14 @@ describe('components/Repository.tsx', () => { it('should mark a repo as read', () => { render( - + , ); fireEvent.click(screen.getByTitle('Mark Repository as Read')); - expect(markRepoNotifications).toHaveBeenCalledWith( + expect(markRepoNotificationsRead).toHaveBeenCalledWith( 'manosim/notifications-test', 'github.com', ); diff --git a/src/components/Repository.tsx b/src/components/Repository.tsx index 28bef678f..2f2a6fe9f 100644 --- a/src/components/Repository.tsx +++ b/src/components/Repository.tsx @@ -6,6 +6,7 @@ import { CSSTransition, TransitionGroup } from 'react-transition-group'; import { AppContext } from '../context/App'; import type { Notification } from '../typesGithub'; import { openExternalLink } from '../utils/comms'; +import Constants from '../utils/constants'; import { NotificationRow } from './NotificationRow'; interface IProps { @@ -19,8 +20,23 @@ export const RepositoryNotifications: FC = ({ repoNotifications, hostname, }) => { - const { markRepoNotifications, markRepoNotificationsDone } = - useContext(AppContext); + const { + markRepoNotificationsRead, + markRepoNotificationsDone, + settings, + notifications, + removeNotificationsFromState, + } = useContext(AppContext); + + const setNotificationsAsOpaque = (repoNotifications: Notification[]) => { + for (const notification of repoNotifications) { + if (notification.unread) { + const notificationRow = document.getElementById(notification.id); + notificationRow.className += Constants.READ_CLASS_NAME; + } + notification.unread = false; + } + }; const openBrowser = useCallback(() => { const url = repoNotifications[0].repository.html_url; @@ -29,12 +45,22 @@ export const RepositoryNotifications: FC = ({ const markRepoAsRead = useCallback(() => { const repoSlug = repoNotifications[0].repository.full_name; - markRepoNotifications(repoSlug, hostname); + markRepoNotificationsRead(repoSlug, hostname); + if (!settings.showReadNotifications) { + removeNotificationsFromState(repoSlug, notifications, hostname); + } else { + setNotificationsAsOpaque(repoNotifications); + } }, [repoNotifications, hostname]); const markRepoAsDone = useCallback(() => { const repoSlug = repoNotifications[0].repository.full_name; markRepoNotificationsDone(repoSlug, hostname); + if (!settings.showReadNotifications) { + removeNotificationsFromState(repoSlug, notifications, hostname); + } else { + setNotificationsAsOpaque(repoNotifications); + } }, [repoNotifications, hostname]); const avatarUrl = repoNotifications[0].repository.owner.avatar_url; @@ -74,14 +100,18 @@ export const RepositoryNotifications: FC = ({
- + {repoNotifications.some((notification) => notification.unread) ? ( + + ) : ( +
+ )}
diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx index 3280d5773..8e8742179 100644 --- a/src/context/App.test.tsx +++ b/src/context/App.test.tsx @@ -193,15 +193,15 @@ describe('context/App.tsx', () => { ); }); - it('should call markRepoNotifications', async () => { + it('should call markRepoNotificationsRead', async () => { const TestComponent = () => { - const { markRepoNotifications } = useContext(AppContext); + const { markRepoNotificationsRead } = useContext(AppContext); return (