fix: delay notification when all read (#2359)

Signed-off-by: Adam Setch <[email protected]>

Author: setchy

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
@@ -98,7 +98,7 @@ export const Sidebar: FC = () => {
           size="small"
           tooltipDirection="e"
           unsafeDisableTooltip={false}
-          variant={hasNotifications ? 'primary' : 'invisible'}
+          variant={hasUnreadNotifications ? 'primary' : 'invisible'}
         />
 
         {isLoggedIn && (

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());

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<void>;
   removeAccountNotifications: (account: Account) => Promise<void>;
@@ -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,

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);
 

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(