From b8f445437e40f8941d3e662a4d1e66855dc5d021 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sun, 9 Nov 2025 13:47:03 -0500 Subject: [PATCH] fix: delay notification when all read Signed-off-by: Adam Setch --- src/renderer/components/Sidebar.tsx | 4 +-- src/renderer/context/App.test.tsx | 4 +++ src/renderer/context/App.tsx | 30 ++++++++++++++----- .../utils/notifications/notifications.test.ts | 7 +++++ .../utils/notifications/notifications.ts | 17 +++++++++-- 5 files changed, 51 insertions(+), 11 deletions(-) diff --git a/src/renderer/components/Sidebar.tsx b/src/renderer/components/Sidebar.tsx index 683652264..9db70d6b2 100644 --- a/src/renderer/components/Sidebar.tsx +++ b/src/renderer/components/Sidebar.tsx @@ -36,7 +36,7 @@ export const Sidebar: FC = () => { settings, auth, unreadCount, - hasNotifications, + hasUnreadNotifications, } = useContext(AppContext); // We naively assume that the first account is the primary account for the purposes of our sidebar quick links @@ -101,7 +101,7 @@ export const Sidebar: FC = () => { sx={sidebarButtonStyle} tooltipDirection="e" unsafeDisableTooltip={false} - variant={hasNotifications ? 'primary' : 'invisible'} + variant={hasUnreadNotifications ? 'primary' : 'invisible'} /> {isLoggedIn && ( diff --git a/src/renderer/context/App.test.tsx b/src/renderer/context/App.test.tsx index fe981dc77..e867b38bb 100644 --- a/src/renderer/context/App.test.tsx +++ b/src/renderer/context/App.test.tsx @@ -43,6 +43,10 @@ describe('renderer/context/App.tsx', () => { .spyOn(tray, 'setTrayIconColorAndTitle') .mockImplementation(jest.fn()); + jest + .spyOn(notifications, 'getNotificationCount') + .mockImplementation(jest.fn()); + jest .spyOn(notifications, 'getUnreadNotificationCount') .mockImplementation(jest.fn()); diff --git a/src/renderer/context/App.tsx b/src/renderer/context/App.tsx index 57993ae31..46895bf96 100644 --- a/src/renderer/context/App.tsx +++ b/src/renderer/context/App.tsx @@ -48,7 +48,10 @@ import { setUseAlternateIdleIcon, setUseUnreadActiveIcon, } from '../utils/comms'; -import { getUnreadNotificationCount } from '../utils/notifications/notifications'; +import { + getNotificationCount, + getUnreadNotificationCount, +} from '../utils/notifications/notifications'; import { clearState, loadState, saveState } from '../utils/storage'; import { DEFAULT_DAY_COLOR_SCHEME, @@ -75,6 +78,7 @@ interface AppContextState { notifications: AccountNotifications[]; unreadCount: number; + hasUnreadNotifications: boolean; hasNotifications: boolean; fetchNotifications: () => Promise; removeAccountNotifications: (account: Account) => Promise; @@ -111,9 +115,17 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { unsubscribeNotification, } = useNotifications(); - const unreadCount = getUnreadNotificationCount(notifications); + const notificationCount = getNotificationCount(notifications); + const unreadNotificationCount = getUnreadNotificationCount(notifications); - const hasNotifications = useMemo(() => unreadCount > 0, [unreadCount]); + const hasNotifications = useMemo( + () => notificationCount > 0, + [notificationCount], + ); + const hasUnreadNotifications = useMemo( + () => unreadNotificationCount > 0, + [unreadNotificationCount], + ); // biome-ignore lint/correctness/useExhaustiveDependencies: restoreSettings is stable and should run only once useEffect(() => { @@ -175,12 +187,12 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { useEffect(() => { setUseUnreadActiveIcon(settings.useUnreadActiveIcon); setUseAlternateIdleIcon(settings.useAlternateIdleIcon); - setTrayIconColorAndTitle(unreadCount, settings); + setTrayIconColorAndTitle(unreadNotificationCount, settings); }, [ settings.showNotificationsCountInTray, settings.useUnreadActiveIcon, settings.useAlternateIdleIcon, - unreadCount, + unreadNotificationCount, ]); useEffect(() => { @@ -359,8 +371,10 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { globalError, notifications, - unreadCount, + notificationCount, + unreadNotificationCount, hasNotifications, + hasUnreadNotifications, fetchNotifications: fetchNotificationsWithAccounts, markNotificationsAsRead: markNotificationsAsReadWithAccounts, @@ -385,8 +399,10 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { globalError, notifications, - unreadCount, + notificationCount, + unreadNotificationCount, hasNotifications, + hasUnreadNotifications, fetchNotificationsWithAccounts, markNotificationsAsReadWithAccounts, markNotificationsAsDoneWithAccounts, diff --git a/src/renderer/utils/notifications/notifications.test.ts b/src/renderer/utils/notifications/notifications.test.ts index faba73e41..8bed45437 100644 --- a/src/renderer/utils/notifications/notifications.test.ts +++ b/src/renderer/utils/notifications/notifications.test.ts @@ -21,6 +21,7 @@ import type { Repository } from '../../typesGitHub'; import * as logger from '../../utils/logger'; import { enrichNotification, + getNotificationCount, getUnreadNotificationCount, stabilizeNotificationsOrder, } from './notifications'; @@ -36,6 +37,12 @@ describe('renderer/utils/notifications/notifications.ts', () => { jest.clearAllMocks(); }); + it('getNotificationCount', () => { + const result = getNotificationCount(mockSingleAccountNotifications); + + expect(result).toBe(1); + }); + it('getUnreadNotificationCount', () => { const result = getUnreadNotificationCount(mockSingleAccountNotifications); diff --git a/src/renderer/utils/notifications/notifications.ts b/src/renderer/utils/notifications/notifications.ts index 76083e695..86d7f256c 100644 --- a/src/renderer/utils/notifications/notifications.ts +++ b/src/renderer/utils/notifications/notifications.ts @@ -15,9 +15,22 @@ import { getFlattenedNotificationsByRepo } from './group'; import { createNotificationHandler } from './handlers'; /** - * Get the count of unread notifications. + * Get the count of notifications across all accounts. * - * @param notifications - The notifications to check. + * @param notifications - The account notifications to check. + * @returns The count of all notifications. + */ +export function getNotificationCount(notifications: AccountNotifications[]) { + return notifications.reduce( + (sum, account) => sum + account.notifications.length, + 0, + ); +} + +/** + * Get the count of notifications across all accounts. + * + * @param notifications - The account notifications to check. * @returns The count of unread notifications. */ export function getUnreadNotificationCount(