From 82d969a2d589223fb3f3cdeef1d4308cfb6f039b Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Tue, 30 Dec 2025 04:21:55 +0100 Subject: [PATCH 01/16] feat: add 'Show read notifications' setting (#708) --- src/renderer/__mocks__/state-mocks.ts | 1 + .../settings/NotificationSettings.test.tsx | 16 +++++ .../settings/NotificationSettings.tsx | 21 +++++++ src/renderer/context/defaults.ts | 1 + .../__snapshots__/Settings.test.tsx.snap | 61 +++++++++++++++++-- src/renderer/types.ts | 1 + .../notifications/filters/filter.test.ts | 28 +++++++++ .../utils/notifications/filters/filter.ts | 5 ++ 8 files changed, 128 insertions(+), 6 deletions(-) diff --git a/src/renderer/__mocks__/state-mocks.ts b/src/renderer/__mocks__/state-mocks.ts index 63ad1f1c0..9d05eff33 100644 --- a/src/renderer/__mocks__/state-mocks.ts +++ b/src/renderer/__mocks__/state-mocks.ts @@ -43,6 +43,7 @@ const mockNotificationSettings: NotificationSettingsState = { showPills: true, showNumber: true, participating: false, + showReadNotifications: true, markAsDoneOnOpen: false, markAsDoneOnUnsubscribe: false, delayNotificationState: false, diff --git a/src/renderer/components/settings/NotificationSettings.test.tsx b/src/renderer/components/settings/NotificationSettings.test.tsx index 7645681ff..326f9f4d4 100644 --- a/src/renderer/components/settings/NotificationSettings.test.tsx +++ b/src/renderer/components/settings/NotificationSettings.test.tsx @@ -271,6 +271,22 @@ describe('renderer/components/settings/NotificationSettings.tsx', () => { ); }); + it('should toggle the showReadNotifications checkbox', async () => { + await act(async () => { + renderWithAppContext(, { + updateSetting: updateSettingMock, + }); + }); + + await userEvent.click(screen.getByTestId('checkbox-showReadNotifications')); + + expect(updateSettingMock).toHaveBeenCalledTimes(1); + expect(updateSettingMock).toHaveBeenCalledWith( + 'showReadNotifications', + false, + ); + }); + it('should toggle the markAsDoneOnOpen checkbox', async () => { await act(async () => { renderWithAppContext(, { diff --git a/src/renderer/components/settings/NotificationSettings.tsx b/src/renderer/components/settings/NotificationSettings.tsx index 66bb02de7..7e1c170c1 100644 --- a/src/renderer/components/settings/NotificationSettings.tsx +++ b/src/renderer/components/settings/NotificationSettings.tsx @@ -328,6 +328,27 @@ export const NotificationSettings: FC = () => { } /> + + updateSetting('showReadNotifications', evt.target.checked) + } + tooltip={ + + + When checked, {APPLICATION.NAME} will + display both read and unread notifications. + + + When unchecked, {APPLICATION.NAME} will only + display unread notifications. + + + } + /> + +
+ + + +
- - @@ -732,20 +732,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -1147,20 +1147,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -1505,20 +1505,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -1920,20 +1920,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -2278,20 +2278,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > diff --git a/src/renderer/components/settings/NotificationSettings.tsx b/src/renderer/components/settings/NotificationSettings.tsx index a042e08fd..7f2e52a7d 100644 --- a/src/renderer/components/settings/NotificationSettings.tsx +++ b/src/renderer/components/settings/NotificationSettings.tsx @@ -330,7 +330,7 @@ export const NotificationSettings: FC = () => { updateSetting('showReadNotifications', evt.target.checked) diff --git a/src/renderer/routes/__snapshots__/Settings.test.tsx.snap b/src/renderer/routes/__snapshots__/Settings.test.tsx.snap index fe35d1c88..73bf409ca 100644 --- a/src/renderer/routes/__snapshots__/Settings.test.tsx.snap +++ b/src/renderer/routes/__snapshots__/Settings.test.tsx.snap @@ -1117,7 +1117,7 @@ exports[`renderer/routes/Settings.tsx should render itself & its children 1`] = class="font-medium text-gitify-font cursor-pointer" for="showReadNotifications" > - Show read notifications + Fetch read notifications
diff --git a/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap b/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap index 920504776..2e932725f 100644 --- a/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap +++ b/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap @@ -1588,20 +1588,20 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende data-wrap="nowrap" > @@ -1878,20 +1878,20 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende data-wrap="nowrap" > @@ -2468,20 +2468,20 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende data-wrap="nowrap" > @@ -2758,20 +2758,20 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende data-wrap="nowrap" > diff --git a/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap b/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap index cf0f804e6..f73d7918d 100644 --- a/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap +++ b/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap @@ -351,20 +351,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its @@ -761,20 +761,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its @@ -1176,20 +1176,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its @@ -1534,20 +1534,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its @@ -1949,20 +1949,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its @@ -2307,20 +2307,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its @@ -2693,20 +2693,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -3022,20 +3022,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > From fe0b00ea63fb9a3b773e24dfbc10c67de42793a4 Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Thu, 1 Jan 2026 19:57:58 +0100 Subject: [PATCH 07/16] refactor: address PR review feedback from setchy --- src/renderer/__mocks__/state-mocks.ts | 2 +- src/renderer/components/Sidebar.tsx | 4 +- .../RepositoryNotifications.test.tsx | 7 + .../notifications/RepositoryNotifications.tsx | 21 +- .../RepositoryNotifications.test.tsx.snap | 318 +++++++----------- .../settings/NotificationSettings.test.tsx | 6 +- .../settings/NotificationSettings.tsx | 6 +- src/renderer/context/App.tsx | 2 +- src/renderer/context/defaults.ts | 2 +- src/renderer/hooks/useNotifications.test.ts | 210 +++++++++++- .../__snapshots__/Settings.test.tsx.snap | 12 +- src/renderer/types.ts | 2 +- src/renderer/utils/api/client.test.ts | 16 +- src/renderer/utils/api/client.ts | 5 +- 14 files changed, 380 insertions(+), 233 deletions(-) diff --git a/src/renderer/__mocks__/state-mocks.ts b/src/renderer/__mocks__/state-mocks.ts index 15f057f96..529aa94c5 100644 --- a/src/renderer/__mocks__/state-mocks.ts +++ b/src/renderer/__mocks__/state-mocks.ts @@ -43,7 +43,7 @@ const mockNotificationSettings: NotificationSettingsState = { showPills: true, showNumber: true, participating: false, - showReadNotifications: false, + fetchReadNotifications: false, markAsDoneOnOpen: false, markAsDoneOnUnsubscribe: false, delayNotificationState: false, diff --git a/src/renderer/components/Sidebar.tsx b/src/renderer/components/Sidebar.tsx index 38259db63..b3d41a2a1 100644 --- a/src/renderer/components/Sidebar.tsx +++ b/src/renderer/components/Sidebar.tsx @@ -35,7 +35,7 @@ export const Sidebar: FC = () => { status, settings, auth, - unreadNotificationCount, + notificationCount, hasUnreadNotifications, } = useAppContext(); @@ -89,7 +89,7 @@ export const Sidebar: FC = () => { openGitHubNotifications(primaryAccountHostname)} size="small" diff --git a/src/renderer/components/notifications/RepositoryNotifications.test.tsx b/src/renderer/components/notifications/RepositoryNotifications.test.tsx index b92b19c2d..c2d6fea24 100644 --- a/src/renderer/components/notifications/RepositoryNotifications.test.tsx +++ b/src/renderer/components/notifications/RepositoryNotifications.test.tsx @@ -23,6 +23,13 @@ describe('renderer/components/notifications/RepositoryNotifications.tsx', () => repoNotifications: mockGitHubCloudGitifyNotifications, }; + beforeEach(() => { + // Reset mock notification state between tests since it's mutated + for (const n of mockGitHubNotifications) { + n.unread = true; + } + }); + afterEach(() => { jest.clearAllMocks(); }); diff --git a/src/renderer/components/notifications/RepositoryNotifications.tsx b/src/renderer/components/notifications/RepositoryNotifications.tsx index 7f2f3ce0d..f1266f453 100644 --- a/src/renderer/components/notifications/RepositoryNotifications.tsx +++ b/src/renderer/components/notifications/RepositoryNotifications.tsx @@ -91,18 +91,9 @@ export const RepositoryNotifications: FC = ({ {!animateExit && ( - - = ({ label={Chevron.label} testid="repository-toggle" /> + + )} diff --git a/src/renderer/components/notifications/__snapshots__/RepositoryNotifications.test.tsx.snap b/src/renderer/components/notifications/__snapshots__/RepositoryNotifications.test.tsx.snap index ccebe1f4b..d5ba93f08 100644 --- a/src/renderer/components/notifications/__snapshots__/RepositoryNotifications.test.tsx.snap +++ b/src/renderer/components/notifications/__snapshots__/RepositoryNotifications.test.tsx.snap @@ -103,49 +103,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should re data-wrap="nowrap" > - @@ -300,49 +271,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should re data-wrap="nowrap" > - @@ -554,20 +496,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should re data-wrap="nowrap" > @@ -751,20 +693,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should re data-wrap="nowrap" > @@ -917,7 +859,7 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should to data-portal-root="true" >
@@ -1114,7 +1056,7 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should to data-portal-root="true" >
@@ -1318,7 +1260,7 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should to data-portal-root="true" >
@@ -1579,7 +1521,7 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should us data-portal-root="true" >
@@ -1783,7 +1725,7 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should us data-portal-root="true" >
diff --git a/src/renderer/components/settings/NotificationSettings.test.tsx b/src/renderer/components/settings/NotificationSettings.test.tsx index a6b06a823..8e9c92c44 100644 --- a/src/renderer/components/settings/NotificationSettings.test.tsx +++ b/src/renderer/components/settings/NotificationSettings.test.tsx @@ -271,18 +271,18 @@ describe('renderer/components/settings/NotificationSettings.tsx', () => { ); }); - it('should toggle the showReadNotifications checkbox', async () => { + it('should toggle the fetchReadNotifications checkbox', async () => { await act(async () => { renderWithAppContext(, { updateSetting: updateSettingMock, }); }); - await userEvent.click(screen.getByTestId('checkbox-showReadNotifications')); + await userEvent.click(screen.getByTestId('checkbox-fetchReadNotifications')); expect(updateSettingMock).toHaveBeenCalledTimes(1); expect(updateSettingMock).toHaveBeenCalledWith( - 'showReadNotifications', + 'fetchReadNotifications', true, ); }); diff --git a/src/renderer/components/settings/NotificationSettings.tsx b/src/renderer/components/settings/NotificationSettings.tsx index 7f2e52a7d..102e53843 100644 --- a/src/renderer/components/settings/NotificationSettings.tsx +++ b/src/renderer/components/settings/NotificationSettings.tsx @@ -329,11 +329,11 @@ export const NotificationSettings: FC = () => { /> - updateSetting('showReadNotifications', evt.target.checked) + updateSetting('fetchReadNotifications', evt.target.checked) } tooltip={ diff --git a/src/renderer/context/App.tsx b/src/renderer/context/App.tsx index 14b085815..1e475f4bf 100644 --- a/src/renderer/context/App.tsx +++ b/src/renderer/context/App.tsx @@ -303,7 +303,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { setUseUnreadActiveIcon(settings.useUnreadActiveIcon); setUseAlternateIdleIcon(settings.useAlternateIdleIcon); - const trayCount = status === 'error' ? -1 : unreadNotificationCount; + const trayCount = status === 'error' ? -1 : notificationCount; setTrayIconColorAndTitle(trayCount, settings); }, [ settings.showNotificationsCountInTray, diff --git a/src/renderer/context/defaults.ts b/src/renderer/context/defaults.ts index 6b615a99d..e26199f98 100644 --- a/src/renderer/context/defaults.ts +++ b/src/renderer/context/defaults.ts @@ -36,7 +36,7 @@ const defaultNotificationSettings: NotificationSettingsState = { showPills: true, showNumber: true, participating: false, - showReadNotifications: false, + fetchReadNotifications: false, markAsDoneOnOpen: false, markAsDoneOnUnsubscribe: false, delayNotificationState: false, diff --git a/src/renderer/hooks/useNotifications.test.ts b/src/renderer/hooks/useNotifications.test.ts index 1d7824232..38970b30d 100644 --- a/src/renderer/hooks/useNotifications.test.ts +++ b/src/renderer/hooks/useNotifications.test.ts @@ -84,11 +84,11 @@ describe('renderer/hooks/useNotifications.ts', () => { ]; nock('https://api.github.com') - .get('/notifications?participating=false') + .get('/notifications?participating=false&all=false') .reply(200, notifications); nock('https://github.gitify.io/api/v3') - .get('/notifications?participating=false') + .get('/notifications?participating=false&all=false') .reply(200, notifications); const { result } = renderHook(() => useNotifications()); @@ -114,13 +114,211 @@ describe('renderer/hooks/useNotifications.ts', () => { expect(result.current.notifications[1].notifications.length).toBe(2); }); + it('should fetch detailed notifications with success', async () => { + const mockRepository = { + name: 'notifications-test', + full_name: 'gitify-app/notifications-test', + html_url: 'https://github.com/gitify-app/notifications-test', + owner: { + login: 'gitify-app', + avatar_url: 'https://avatar.url', + type: 'Organization', + }, + }; + + const mockNotifications = [ + { + id: '1', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'This is a check suite workflow.', + type: 'CheckSuite', + url: null, + latest_comment_url: null, + }, + repository: mockRepository, + }, + { + id: '2', + unread: true, + updated_at: '2024-02-26T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'This is a Discussion.', + type: 'Discussion', + url: null, + latest_comment_url: null, + }, + repository: mockRepository, + }, + { + id: '3', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'This is an Issue.', + type: 'Issue', + url: 'https://api.github.com/repos/gitify-app/notifications-test/issues/3', + latest_comment_url: + 'https://api.github.com/repos/gitify-app/notifications-test/issues/3/comments', + }, + repository: mockRepository, + }, + { + id: '4', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'This is a Pull Request.', + type: 'PullRequest', + url: 'https://api.github.com/repos/gitify-app/notifications-test/pulls/4', + latest_comment_url: + 'https://api.github.com/repos/gitify-app/notifications-test/issues/4/comments', + }, + repository: mockRepository, + }, + { + id: '5', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'This is an invitation.', + type: 'RepositoryInvitation', + url: null, + latest_comment_url: null, + }, + repository: mockRepository, + }, + { + id: '6', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'This is a workflow run.', + type: 'WorkflowRun', + url: null, + latest_comment_url: null, + }, + repository: mockRepository, + }, + ]; + + nock('https://api.github.com') + .get('/notifications?participating=false&all=false') + .reply(200, mockNotifications); + + nock('https://api.github.com') + .post('/graphql') + .reply(200, { + data: { + search: { + nodes: [ + { + title: 'This is a Discussion.', + stateReason: null, + isAnswered: true, + url: 'https://github.com/gitify-app/notifications-test/discussions/612', + author: { + login: 'discussion-creator', + url: 'https://github.com/discussion-creator', + avatar_url: + 'https://avatars.githubusercontent.com/u/133795385?s=200&v=4', + type: 'User', + }, + comments: { + nodes: [ + { + databaseId: 2297637, + createdAt: '2022-03-04T20:39:44Z', + author: { + login: 'comment-user', + url: 'https://github.com/comment-user', + avatar_url: + 'https://avatars.githubusercontent.com/u/1?v=4', + type: 'User', + }, + replies: { + nodes: [], + }, + }, + ], + }, + labels: null, + }, + ], + }, + }, + }); + + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/issues/3') + .reply(200, { + state: 'closed', + merged: true, + user: mockNotificationUser, + labels: [], + }); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/issues/3/comments') + .reply(200, { + user: mockNotificationUser, + }); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/pulls/4') + .reply(200, { + state: 'closed', + merged: false, + user: mockNotificationUser, + labels: [], + }); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/pulls/4/reviews') + .reply(200, {}); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/issues/4/comments') + .reply(200, { + user: mockNotificationUser, + }); + + const { result } = renderHook(() => useNotifications()); + + act(() => { + result.current.fetchNotifications({ + auth: { + accounts: [mockGitHubCloudAccount], + }, + settings: { + ...mockSettings, + detailedNotifications: true, + }, + }); + }); + + expect(result.current.status).toBe('loading'); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + expect(result.current.notifications[0].account.hostname).toBe( + 'github.com', + ); + expect(result.current.notifications[0].notifications.length).toBe(6); + }); + it('should fetch notifications with same failures', async () => { const code = AxiosError.ERR_BAD_REQUEST; const status = 401; const message = 'Bad credentials'; nock('https://api.github.com/') - .get('/notifications?participating=false') + .get('/notifications?participating=false&all=false') .replyWithError({ code, response: { @@ -132,7 +330,7 @@ describe('renderer/hooks/useNotifications.ts', () => { }); nock('https://github.gitify.io/api/v3/') - .get('/notifications?participating=false') + .get('/notifications?participating=false&all=false') .replyWithError({ code, response: { @@ -163,7 +361,7 @@ describe('renderer/hooks/useNotifications.ts', () => { const code = AxiosError.ERR_BAD_REQUEST; nock('https://api.github.com/') - .get('/notifications?participating=false') + .get('/notifications?participating=false&all=false') .replyWithError({ code, response: { @@ -175,7 +373,7 @@ describe('renderer/hooks/useNotifications.ts', () => { }); nock('https://github.gitify.io/api/v3/') - .get('/notifications?participating=false') + .get('/notifications?participating=false&all=false') .replyWithError({ code, response: { diff --git a/src/renderer/routes/__snapshots__/Settings.test.tsx.snap b/src/renderer/routes/__snapshots__/Settings.test.tsx.snap index 73bf409ca..792e01bb6 100644 --- a/src/renderer/routes/__snapshots__/Settings.test.tsx.snap +++ b/src/renderer/routes/__snapshots__/Settings.test.tsx.snap @@ -1109,22 +1109,22 @@ exports[`renderer/routes/Settings.tsx should render itself & its children 1`] = >
diff --git a/src/renderer/components/notifications/RepositoryNotifications.tsx b/src/renderer/components/notifications/RepositoryNotifications.tsx index f1266f453..a0e2a363f 100644 --- a/src/renderer/components/notifications/RepositoryNotifications.tsx +++ b/src/renderer/components/notifications/RepositoryNotifications.tsx @@ -99,13 +99,6 @@ export const RepositoryNotifications: FC = ({ testid="repository-mark-as-read" /> - - = ({ label="Mark repository as done" testid="repository-mark-as-done" /> + + )} diff --git a/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap b/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap index 2e932725f..e49f20bc6 100644 --- a/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap +++ b/src/renderer/components/notifications/__snapshots__/AccountNotifications.test.tsx.snap @@ -1617,20 +1617,20 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende @@ -1907,20 +1907,20 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende @@ -2497,20 +2497,20 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende @@ -2787,20 +2787,20 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende diff --git a/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap b/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap index f73d7918d..cf0f804e6 100644 --- a/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap +++ b/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap @@ -351,20 +351,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its @@ -761,20 +761,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its @@ -1176,20 +1176,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its @@ -1534,20 +1534,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its @@ -1949,20 +1949,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its @@ -2307,20 +2307,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its @@ -2693,20 +2693,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -3022,20 +3022,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > diff --git a/src/renderer/components/notifications/__snapshots__/RepositoryNotifications.test.tsx.snap b/src/renderer/components/notifications/__snapshots__/RepositoryNotifications.test.tsx.snap index d5ba93f08..e70eee124 100644 --- a/src/renderer/components/notifications/__snapshots__/RepositoryNotifications.test.tsx.snap +++ b/src/renderer/components/notifications/__snapshots__/RepositoryNotifications.test.tsx.snap @@ -103,20 +103,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should re data-wrap="nowrap" > @@ -271,20 +271,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should re data-wrap="nowrap" > @@ -525,20 +525,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should re @@ -722,20 +722,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should re @@ -983,20 +983,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should to @@ -1180,20 +1180,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should to @@ -1384,20 +1384,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should to @@ -1645,20 +1645,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should us @@ -1849,20 +1849,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should us From 736bf2c2722f6448baa4b91fe211d15f9f96803b Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Fri, 9 Jan 2026 10:42:35 +0100 Subject: [PATCH 10/16] refactor: handle mark as done limitation when fetching read notifications --- .../notifications/NotificationRow.tsx | 21 +++++++++++++++---- .../notifications/RepositoryNotifications.tsx | 13 ++++++++---- .../settings/NotificationSettings.tsx | 14 ++++++------- src/renderer/hooks/useNotifications.ts | 7 ++++++- .../__snapshots__/Settings.test.tsx.snap | 2 +- 5 files changed, 39 insertions(+), 18 deletions(-) diff --git a/src/renderer/components/notifications/NotificationRow.tsx b/src/renderer/components/notifications/NotificationRow.tsx index 1f20aeb73..7ee2637bc 100644 --- a/src/renderer/components/notifications/NotificationRow.tsx +++ b/src/renderer/components/notifications/NotificationRow.tsx @@ -33,10 +33,16 @@ export const NotificationRow: FC = ({ const [animateExit, setAnimateExit] = useState(false); const handleNotification = useCallback(() => { - setAnimateExit(!settings.delayNotificationState); + // Don't animate exit when fetchReadNotifications is enabled + // as notifications will stay in the list with reduced opacity + const shouldAnimateExit = + !settings.delayNotificationState && !settings.fetchReadNotifications; + setAnimateExit(shouldAnimateExit); openNotification(notification); - if (settings.markAsDoneOnOpen) { + // Fall back to mark as read when fetchReadNotifications is enabled + // since marking as done won't work correctly (API limitation) + if (settings.markAsDoneOnOpen && !settings.fetchReadNotifications) { markNotificationsAsDone([notification]); } else { markNotificationsAsRead([notification]); @@ -54,7 +60,11 @@ export const NotificationRow: FC = ({ }; const actionMarkAsRead = () => { - setAnimateExit(!settings.delayNotificationState); + // Don't animate exit when fetchReadNotifications is enabled + // as the notification will stay in the list with reduced opacity + if (!settings.fetchReadNotifications) { + setAnimateExit(!settings.delayNotificationState); + } markNotificationsAsRead([notification]); }; @@ -147,7 +157,10 @@ export const NotificationRow: FC = ({ = ({ }; const actionMarkAsRead = () => { - setAnimateExit(!settings.delayNotificationState); + // Don't animate exit when fetchReadNotifications is enabled + // as the notifications will stay in the list with reduced opacity + if (!settings.fetchReadNotifications) { + setAnimateExit(!settings.delayNotificationState); + } markNotificationsAsRead(repoNotifications); }; @@ -101,9 +105,10 @@ export const RepositoryNotifications: FC = ({ { updateSetting('fetchReadNotifications', evt.target.checked) } tooltip={ - - When checked, {APPLICATION.NAME} will fetch - and display both read and unread notifications. - - - When unchecked, {APPLICATION.NAME} will only - fetch and display unread notifications. + Fetch all notifications including read and done. + + ⚠️ GitHub's API does not distinguish between read and done + states, so 'Mark as done' actions will be unavailable when this + setting is enabled. ⚠️ Enabling this setting will increase API usage and may cause diff --git a/src/renderer/hooks/useNotifications.ts b/src/renderer/hooks/useNotifications.ts index a13d287d5..e2c885e88 100644 --- a/src/renderer/hooks/useNotifications.ts +++ b/src/renderer/hooks/useNotifications.ts @@ -222,7 +222,12 @@ export const useNotifications = (): NotificationsState => { notification.account.token, ); - if (state.settings.markAsDoneOnUnsubscribe) { + // Fall back to mark as read when fetchReadNotifications is enabled + // since marking as done won't work correctly (API limitation) + if ( + state.settings.markAsDoneOnUnsubscribe && + !state.settings.fetchReadNotifications + ) { await markNotificationsAsDone(state, [notification]); } else { await markNotificationsAsRead(state, [notification]); diff --git a/src/renderer/routes/__snapshots__/Settings.test.tsx.snap b/src/renderer/routes/__snapshots__/Settings.test.tsx.snap index 792e01bb6..3605497b4 100644 --- a/src/renderer/routes/__snapshots__/Settings.test.tsx.snap +++ b/src/renderer/routes/__snapshots__/Settings.test.tsx.snap @@ -1117,7 +1117,7 @@ exports[`renderer/routes/Settings.tsx should render itself & its children 1`] = class="font-medium text-gitify-font cursor-pointer" for="fetchReadNotifications" > - Fetch read notifications + Fetch read & done notifications