From 587eb50633cee5caed4ac50ec9911c3523af4532 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sun, 19 Oct 2025 11:10:09 -0700 Subject: [PATCH 1/7] fix: notification read state for delayNotificationState Signed-off-by: Adam Setch --- .../components/notifications/NotificationRow.tsx | 11 +++-------- .../notifications/RepositoryNotifications.tsx | 10 +++++----- src/renderer/hooks/useNotifications.ts | 16 ++++++++++++++++ 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/renderer/components/notifications/NotificationRow.tsx b/src/renderer/components/notifications/NotificationRow.tsx index 3142c5fc6..afec3184b 100644 --- a/src/renderer/components/notifications/NotificationRow.tsx +++ b/src/renderer/components/notifications/NotificationRow.tsx @@ -18,13 +18,11 @@ import { NotificationHeader } from './NotificationHeader'; interface INotificationRow { notification: Notification; isAnimated?: boolean; - isRead?: boolean; } export const NotificationRow: FC = ({ notification, isAnimated = false, - isRead = false, }: INotificationRow) => { const { settings, @@ -33,12 +31,9 @@ export const NotificationRow: FC = ({ unsubscribeNotification, } = useContext(AppContext); const [animateExit, setAnimateExit] = useState(false); - const [showAsRead, setShowAsRead] = useState(false); const handleNotification = useCallback(() => { setAnimateExit(!settings.delayNotificationState); - setShowAsRead(settings.delayNotificationState); - openNotification(notification); if (settings.markAsDoneOnOpen) { @@ -55,13 +50,11 @@ export const NotificationRow: FC = ({ const actionMarkAsDone = () => { setAnimateExit(!settings.delayNotificationState); - setShowAsRead(settings.delayNotificationState); markNotificationsAsDone([notification]); }; const actionMarkAsRead = () => { setAnimateExit(!settings.delayNotificationState); - setShowAsRead(settings.delayNotificationState); markNotificationsAsRead([notification]); }; @@ -78,6 +71,8 @@ export const NotificationRow: FC = ({ const groupByDate = settings.groupBy === GroupBy.DATE; + const isNotificationRead = !notification.unread; + return (
= ({ 'text-gitify-font border-gitify-notification-border hover:bg-gitify-notification-hover', (isAnimated || animateExit) && 'translate-x-full opacity-0 transition duration-350 ease-in-out', - (isRead || showAsRead) && Opacity.READ, + isNotificationRead && Opacity.READ, )} id={notification.id} > diff --git a/src/renderer/components/notifications/RepositoryNotifications.tsx b/src/renderer/components/notifications/RepositoryNotifications.tsx index deade462d..545d4d407 100644 --- a/src/renderer/components/notifications/RepositoryNotifications.tsx +++ b/src/renderer/components/notifications/RepositoryNotifications.tsx @@ -27,7 +27,6 @@ export const RepositoryNotifications: FC = ({ const { settings, markNotificationsAsRead, markNotificationsAsDone } = useContext(AppContext); const [animateExit, setAnimateExit] = useState(false); - const [showAsRead, setShowAsRead] = useState(false); const [showRepositoryNotifications, setShowRepositoryNotifications] = useState(true); @@ -35,13 +34,11 @@ export const RepositoryNotifications: FC = ({ const actionMarkAsDone = () => { setAnimateExit(!settings.delayNotificationState); - setShowAsRead(settings.delayNotificationState); markNotificationsAsDone(repoNotifications); }; const actionMarkAsRead = () => { setAnimateExit(!settings.delayNotificationState); - setShowAsRead(settings.delayNotificationState); markNotificationsAsRead(repoNotifications); }; @@ -49,6 +46,10 @@ export const RepositoryNotifications: FC = ({ setShowRepositoryNotifications(!showRepositoryNotifications); }; + const areAllRepoNotificationsRead = repoNotifications.every( + (notification) => !notification.unread, + ); + const Chevron = getChevronDetails( true, showRepositoryNotifications, @@ -63,7 +64,7 @@ export const RepositoryNotifications: FC = ({ 'bg-gitify-repository', animateExit && 'translate-x-full opacity-0 transition duration-350 ease-in-out', - showAsRead && Opacity.READ, + areAllRepoNotificationsRead && Opacity.READ, )} direction="horizontal" onClick={actionToggleRepositoryNotifications} @@ -122,7 +123,6 @@ export const RepositoryNotifications: FC = ({ repoNotifications.map((notification) => ( diff --git a/src/renderer/hooks/useNotifications.ts b/src/renderer/hooks/useNotifications.ts index bd9b9f4ae..3e2f25979 100644 --- a/src/renderer/hooks/useNotifications.ts +++ b/src/renderer/hooks/useNotifications.ts @@ -23,6 +23,18 @@ import { } from '../utils/notifications/notifications'; import { removeNotifications } from '../utils/notifications/remove'; +// Apply optimistic local updates for read state when delayNotificationState is enabled +function applyLocalOptimisticRead( + state: GitifyState, + targetNotifications: Notification[], +) { + if (state.settings.delayNotificationState) { + for (const n of targetNotifications) { + n.unread = false; + } + } +} + interface NotificationsState { notifications: AccountNotifications[]; removeAccountNotifications: (account: Account) => Promise; @@ -125,6 +137,8 @@ export const useNotifications = (): NotificationsState => { notifications, ); + applyLocalOptimisticRead(state, readNotifications); + setNotifications(updatedNotifications); setTrayIconColor(updatedNotifications); } catch (err) { @@ -163,6 +177,8 @@ export const useNotifications = (): NotificationsState => { notifications, ); + applyLocalOptimisticRead(state, doneNotifications); + setNotifications(updatedNotifications); setTrayIconColor(updatedNotifications); } catch (err) { From c8641c9e2ca99297fad94831b50fe57e8ffcd71e Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sun, 19 Oct 2025 11:25:18 -0700 Subject: [PATCH 2/7] fix: notification read state for delayNotificationState Signed-off-by: Adam Setch --- .../notifications/NotificationRow.test.tsx | 22 + .../RepositoryNotifications.test.tsx | 14 + .../NotificationRow.test.tsx.snap | 699 ++++++++++++++++++ .../RepositoryNotifications.test.tsx.snap | 487 +++++++++++- 4 files changed, 1197 insertions(+), 25 deletions(-) diff --git a/src/renderer/components/notifications/NotificationRow.test.tsx b/src/renderer/components/notifications/NotificationRow.test.tsx index 2a47f0c34..2feb18a85 100644 --- a/src/renderer/components/notifications/NotificationRow.test.tsx +++ b/src/renderer/components/notifications/NotificationRow.test.tsx @@ -84,6 +84,28 @@ describe('renderer/components/notifications/NotificationRow.tsx', () => { expect(tree).toMatchSnapshot(); }); + it('should render itself & its children - notification is read', async () => { + jest + .spyOn(global.Date, 'now') + .mockImplementation(() => new Date('2024').valueOf()); + + const props = { + notification: { + ...mockSingleNotification, + unread: false, + }, + account: mockGitHubCloudAccount, + }; + + const tree = render( + + + , + ); + + expect(tree).toMatchSnapshot(); + }); + describe('notification interactions', () => { it('should open a notification in the browser - click', async () => { const markNotificationsAsRead = jest.fn(); diff --git a/src/renderer/components/notifications/RepositoryNotifications.test.tsx b/src/renderer/components/notifications/RepositoryNotifications.test.tsx index 90b725bb9..bd352b37e 100644 --- a/src/renderer/components/notifications/RepositoryNotifications.test.tsx +++ b/src/renderer/components/notifications/RepositoryNotifications.test.tsx @@ -39,6 +39,20 @@ describe('renderer/components/notifications/RepositoryNotifications.tsx', () => expect(tree).toMatchSnapshot(); }); + it('should render itself & its children - all notifications are read', () => { + for (const n of props.repoNotifications) { + n.unread = false; + } + + const tree = render( + + + , + ); + + expect(tree).toMatchSnapshot(); + }); + it('should open the browser when clicking on the repo name', async () => { const openExternalLinkMock = jest .spyOn(comms, 'openExternalLink') diff --git a/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap b/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap index 910804342..986fac7c5 100644 --- a/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap +++ b/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap @@ -2200,3 +2200,702 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its "unmount": [Function], } `; + +exports[`renderer/components/notifications/NotificationRow.tsx should render itself & its children - notification is read 1`] = ` +{ + "asFragment": [Function], + "baseElement": +
+
+
+ + +
+
+ + I am a robot and this is a test! + + +
+
+ +
+ + Updated + + + May 20, 2017 + +
+
+ +
+ + + 1 + +
+ + +
+ + + 1 + +
+ +
+
+
+
+ + + +
+
+
+
+ , + "container":
+
+
+ + +
+
+ + I am a robot and this is a test! + + +
+
+ +
+ + Updated + + + May 20, 2017 + +
+
+ +
+ + + 1 + +
+ + +
+ + + 1 + +
+ +
+
+
+
+ + + +
+
+
+
, + "debug": [Function], + "findAllByAltText": [Function], + "findAllByDisplayValue": [Function], + "findAllByLabelText": [Function], + "findAllByPlaceholderText": [Function], + "findAllByRole": [Function], + "findAllByTestId": [Function], + "findAllByText": [Function], + "findAllByTitle": [Function], + "findByAltText": [Function], + "findByDisplayValue": [Function], + "findByLabelText": [Function], + "findByPlaceholderText": [Function], + "findByRole": [Function], + "findByTestId": [Function], + "findByText": [Function], + "findByTitle": [Function], + "getAllByAltText": [Function], + "getAllByDisplayValue": [Function], + "getAllByLabelText": [Function], + "getAllByPlaceholderText": [Function], + "getAllByRole": [Function], + "getAllByTestId": [Function], + "getAllByText": [Function], + "getAllByTitle": [Function], + "getByAltText": [Function], + "getByDisplayValue": [Function], + "getByLabelText": [Function], + "getByPlaceholderText": [Function], + "getByRole": [Function], + "getByTestId": [Function], + "getByText": [Function], + "getByTitle": [Function], + "queryAllByAltText": [Function], + "queryAllByDisplayValue": [Function], + "queryAllByLabelText": [Function], + "queryAllByPlaceholderText": [Function], + "queryAllByRole": [Function], + "queryAllByTestId": [Function], + "queryAllByText": [Function], + "queryAllByTitle": [Function], + "queryByAltText": [Function], + "queryByDisplayValue": [Function], + "queryByLabelText": [Function], + "queryByPlaceholderText": [Function], + "queryByRole": [Function], + "queryByTestId": [Function], + "queryByText": [Function], + "queryByTitle": [Function], + "rerender": [Function], + "unmount": [Function], +} +`; diff --git a/src/renderer/components/notifications/__snapshots__/RepositoryNotifications.test.tsx.snap b/src/renderer/components/notifications/__snapshots__/RepositoryNotifications.test.tsx.snap index 329adacad..a20271b74 100644 --- a/src/renderer/components/notifications/__snapshots__/RepositoryNotifications.test.tsx.snap +++ b/src/renderer/components/notifications/__snapshots__/RepositoryNotifications.test.tsx.snap @@ -1,5 +1,442 @@ // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing +exports[`renderer/components/notifications/RepositoryNotifications.tsx should render itself & its children - all notifications are read 1`] = ` +{ + "asFragment": [Function], + "baseElement": +
+
+ +
+ + + +
+
+
+ NotificationRow +
+
+ NotificationRow +
+
+ , + "container":
+
+ +
+ + + +
+
+
+ NotificationRow +
+
+ NotificationRow +
+
, + "debug": [Function], + "findAllByAltText": [Function], + "findAllByDisplayValue": [Function], + "findAllByLabelText": [Function], + "findAllByPlaceholderText": [Function], + "findAllByRole": [Function], + "findAllByTestId": [Function], + "findAllByText": [Function], + "findAllByTitle": [Function], + "findByAltText": [Function], + "findByDisplayValue": [Function], + "findByLabelText": [Function], + "findByPlaceholderText": [Function], + "findByRole": [Function], + "findByTestId": [Function], + "findByText": [Function], + "findByTitle": [Function], + "getAllByAltText": [Function], + "getAllByDisplayValue": [Function], + "getAllByLabelText": [Function], + "getAllByPlaceholderText": [Function], + "getAllByRole": [Function], + "getAllByTestId": [Function], + "getAllByText": [Function], + "getAllByTitle": [Function], + "getByAltText": [Function], + "getByDisplayValue": [Function], + "getByLabelText": [Function], + "getByPlaceholderText": [Function], + "getByRole": [Function], + "getByTestId": [Function], + "getByText": [Function], + "getByTitle": [Function], + "queryAllByAltText": [Function], + "queryAllByDisplayValue": [Function], + "queryAllByLabelText": [Function], + "queryAllByPlaceholderText": [Function], + "queryAllByRole": [Function], + "queryAllByTestId": [Function], + "queryAllByText": [Function], + "queryAllByTitle": [Function], + "queryByAltText": [Function], + "queryByDisplayValue": [Function], + "queryByLabelText": [Function], + "queryByPlaceholderText": [Function], + "queryByRole": [Function], + "queryByTestId": [Function], + "queryByText": [Function], + "queryByTitle": [Function], + "rerender": [Function], + "unmount": [Function], +} +`; + exports[`renderer/components/notifications/RepositoryNotifications.tsx should render itself & its children 1`] = ` { "asFragment": [Function], @@ -443,7 +880,7 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should to "baseElement":