feat(notifications): add loading state for initial fetch

src/renderer/components/Loading.test.tsx

@@ -0,0 +1,36 @@
+import { act } from '@testing-library/react';
+
+import { renderWithProviders } from '../__helpers__/test-utils';
+
+import { Loading } from './Loading';
+
+describe('renderer/components/Loading.tsx', () => {
+  it('should render itself & its children', async () => {
+    let tree: ReturnType<typeof renderWithProviders> | null = null;
+
+    await act(async () => {
+      tree = renderWithProviders(<Loading />);
+    });
+
+    expect(tree!.getByText('Loading')).toBeInTheDocument();
+    expect(tree!.container.querySelector('img')).toHaveAttribute('alt', '⏳');
+    expect(tree!.container).toMatchSnapshot();
+  });
+
+  it('should keep the same emoji when re-rendering', async () => {
+    let tree: ReturnType<typeof renderWithProviders> | null = null;
+
+    await act(async () => {
+      tree = renderWithProviders(<Loading />);
+    });
+
+    const emojiSrc = tree!.container.querySelector('img')?.getAttribute('src');
+    expect(emojiSrc).toBeTruthy();
+
+    await act(async () => {
+      tree!.rerender(<Loading />);
+    });
+
+    expect(tree!.container.querySelector('img')).toHaveAttribute('src', emojiSrc!);
+  });
+});

src/renderer/components/Loading.tsx

@@ -0,0 +1,13 @@
+import { useMemo } from 'react';
+
+import { Constants } from '../constants';
+
+import { EmojiSplash } from './layout/EmojiSplash';
+
+import { randomElement } from '../utils/core/random';
+
+export function Loading() {
+  const emoji = useMemo(() => randomElement(Constants.EMOJIS.LOADING), []);
+
+  return <EmojiSplash emoji={emoji} fullHeight heading="Loading" />;
+}

src/renderer/constants.ts

@@ -64,6 +64,7 @@ export const Constants = {
   // Emojis for different states and events
   EMOJIS: {
     ALL_READ: ['🎉', '🎊', '🥳', '👏', '🙌', '😎', '🏖️', '🚀', '✨', '🏆'],
+    LOADING: ['⏳', '⌛'],
     ERRORS: {
       ACTION_FORBIDDEN: ['🚫'],
       BAD_CREDENTIALS: ['🔓'],

src/renderer/hooks/notifications/useNotificationsQuery.ts

@@ -1,6 +1,6 @@
 import { useCallback, useMemo } from 'react';
 
-import { keepPreviousData, type QueryKey, useQuery, useQueryClient } from '@tanstack/react-query';
+import { type QueryKey, useQuery, useQueryClient } from '@tanstack/react-query';
 
 import { Constants } from '../../constants';
 
@@ -46,6 +46,20 @@ export interface NotificationsQueryResult {
   removeAccountNotifications: (account: Account) => void;
 }
 
+/**
+ * Keep previous notifications visible across query-key changes only when they
+ * contain actual content or account errors. An account set with empty inboxes is
+ * not content: it must not make a new account set look settled. Returning
+ * undefined lets the query report initial loading instead.
+ */
+function keepNonEmptyPreviousData(
+  previousData: AccountNotifications[] | undefined,
+): AccountNotifications[] | undefined {
+  return previousData?.some((account) => account.notifications.length > 0 || account.error !== null)
+    ? previousData
+    : undefined;
+}
+
 /**
  * The read side of notification state: the TanStack Query observer (optionally
  * polling), the filter projection, derived counts, and the derived status and
@@ -129,7 +143,7 @@ export const useNotificationsQuery = ({ poll }: { poll: boolean }): Notification
     // This allows filter changes to instantly update without refetching
     select: selectFilteredNotifications,
 
-    placeholderData: keepPreviousData,
+    placeholderData: keepNonEmptyPreviousData,
 
     // Fresh for at least one poll cycle at the fastest possible interval, so
     // data isn't treated as stale before a refetch could realistically have

src/renderer/hooks/useNotifications.test.tsx

@@ -192,6 +192,109 @@ describe('renderer/hooks/useNotifications.ts', () => {
     });
   });
 
+  describe('account set changes', () => {
+    it('reports loading when a new account set starts from an empty settled result', async () => {
+      useAccountsStore.setState({ accounts: [] });
+      getAllNotificationsMock.mockResolvedValueOnce([]);
+
+      const { result } = renderNotificationsHook();
+      await waitFor(() => expect(result.current.status).toBe('success'));
+
+      let resolveFetch: ((data: AccountNotifications[]) => void) | undefined;
+      getAllNotificationsMock.mockImplementationOnce(
+        () =>
+          new Promise((resolve) => {
+            resolveFetch = resolve;
+          }),
+      );
+
+      act(() => {
+        useAccountsStore.setState({ accounts: [mockGitHubCloudAccount] });
+      });
+
+      await waitFor(() => expect(result.current.status).toBe('loading'));
+      expect(getAllNotificationsMock).toHaveBeenCalledTimes(2);
+
+      await act(async () => {
+        resolveFetch?.(mockSingleAccountNotifications);
+      });
+
+      await waitFor(() => expect(result.current.status).toBe('success'));
+      expect(result.current.notifications).toEqual(mockSingleAccountNotifications);
+    });
+
+    it('reports loading when the account set changes from an empty settled inbox', async () => {
+      getAllNotificationsMock.mockResolvedValueOnce([
+        {
+          account: mockGitHubCloudAccount,
+          notifications: [],
+          error: null,
+        },
+      ]);
+
+      const { result } = renderNotificationsHook();
+      await waitFor(() => expect(result.current.status).toBe('success'));
+      expect(result.current.notifications).toHaveLength(1);
+      expect(result.current.notifications[0].notifications).toEqual([]);
+
+      let resolveFetch: ((data: AccountNotifications[]) => void) | undefined;
+      getAllNotificationsMock.mockImplementationOnce(
+        () =>
+          new Promise((resolve) => {
+            resolveFetch = resolve;
+          }),
+      );
+
+      act(() => {
+        useAccountsStore.setState({
+          accounts: [mockGitHubCloudAccount, mockGitHubEnterpriseServerAccount],
+        });
+      });
+
+      await waitFor(() => expect(result.current.status).toBe('loading'));
+
+      await act(async () => {
+        resolveFetch?.(mockMultipleAccountNotifications);
+      });
+
+      await waitFor(() => expect(result.current.status).toBe('success'));
+      expect(result.current.notifications).toEqual(mockMultipleAccountNotifications);
+    });
+
+    it('keeps non-empty notifications visible while a new account set loads', async () => {
+      getAllNotificationsMock.mockResolvedValueOnce(mockSingleAccountNotifications);
+
+      const { result } = renderNotificationsHook();
+      await waitFor(() => expect(result.current.status).toBe('success'));
+      expect(result.current.notifications).toEqual(mockSingleAccountNotifications);
+
+      let resolveFetch: ((data: AccountNotifications[]) => void) | undefined;
+      getAllNotificationsMock.mockImplementationOnce(
+        () =>
+          new Promise((resolve) => {
+            resolveFetch = resolve;
+          }),
+      );
+
+      act(() => {
+        useAccountsStore.setState({
+          accounts: [mockGitHubCloudAccount, mockGitHubEnterpriseServerAccount],
+        });
+      });
+
+      await waitFor(() => expect(result.current.isFetching).toBe(true));
+      expect(result.current.status).toBe('success');
+      expect(result.current.notifications).toEqual(mockSingleAccountNotifications);
+
+      await act(async () => {
+        resolveFetch?.(mockMultipleAccountNotifications);
+      });
+
+      await waitFor(() => expect(result.current.isFetching).toBe(false));
+      expect(result.current.notifications).toEqual(mockMultipleAccountNotifications);
+    });
+  });
+
   describe('status stability during background refetches', () => {
     it('keeps status as error while a retry during an ongoing outage is in flight', async () => {
       let rejectRetry: (() => void) | undefined;
@@ -264,6 +367,36 @@ describe('renderer/hooks/useNotifications.ts', () => {
       await waitFor(() => expect(result.current.isFetching).toBe(false));
       expect(result.current.status).toBe('success');
     });
+
+    it('keeps status as success while a settled empty inbox refetches', async () => {
+      let resolveRefetch: ((data: AccountNotifications[]) => void) | undefined;
+      getAllNotificationsMock.mockResolvedValueOnce([]).mockImplementationOnce(
+        () =>
+          new Promise((resolve) => {
+            resolveRefetch = resolve;
+          }),
+      );
+
+      const { result } = renderNotificationsHook();
+
+      await waitFor(() => expect(result.current.status).toBe('success'));
+      expect(result.current.notifications).toEqual([]);
+
+      act(() => {
+        void result.current.refetchNotifications();
+      });
+
+      await waitFor(() => expect(result.current.isFetching).toBe(true));
+      expect(result.current.status).toBe('success');
+      expect(result.current.notifications).toEqual([]);
+
+      await act(async () => {
+        resolveRefetch?.([]);
+      });
+
+      await waitFor(() => expect(result.current.isFetching).toBe(false));
+      expect(result.current.status).toBe('success');
+    });
   });
 
   describe('polling', () => {

src/renderer/routes/Notifications.test.tsx

@@ -15,6 +15,10 @@ vi.mock('../components/AllRead', () => ({
   AllRead: () => <p>AllRead</p>,
 }));
 
+vi.mock('../components/Loading', () => ({
+  Loading: () => <p>Loading</p>,
+}));
+
 vi.mock('../components/Oops', () => ({
   Oops: () => <p>Oops</p>,
 }));
@@ -31,9 +35,61 @@ describe('renderer/routes/Notifications.tsx', () => {
 
   it('should render itself & its children (all read notifications)', () => {
     const tree = renderWithProviders(<NotificationsRoute />);
+    expect(screen.getByText('AllRead')).toBeInTheDocument();
+    expect(screen.queryByText('Loading')).not.toBeInTheDocument();
     expect(tree.container).toMatchSnapshot();
   });
 
+  it('should render loading state instead of all-read while notifications are loading', () => {
+    const tree = renderWithProviders(<NotificationsRoute />, {
+      status: 'loading',
+    });
+
+    expect(screen.getByText('Loading')).toBeInTheDocument();
+    expect(screen.queryByText('AllRead')).not.toBeInTheDocument();
+    expect(tree.container).toMatchSnapshot();
+  });
+
+  it('should keep notifications visible during a background refresh', () => {
+    renderWithProviders(<NotificationsRoute />, {
+      notifications: mockMultipleAccountNotifications,
+      hasNotifications: true,
+      isFetching: true,
+    });
+
+    expect(screen.getByText('NotificationList')).toBeInTheDocument();
+    expect(screen.queryByText('Loading')).not.toBeInTheDocument();
+  });
+
+  it('should keep all-read visible during a background refresh with an empty inbox', () => {
+    renderWithProviders(<NotificationsRoute />, {
+      isFetching: true,
+    });
+
+    expect(screen.getByText('AllRead')).toBeInTheDocument();
+    expect(screen.queryByText('Loading')).not.toBeInTheDocument();
+  });
+
+  it('should prefer the offline state over loading', () => {
+    renderWithProviders(<NotificationsRoute />, {
+      isOnline: false,
+      status: 'loading',
+    });
+
+    expect(screen.getByText('Oops')).toBeInTheDocument();
+    expect(screen.queryByText('Loading')).not.toBeInTheDocument();
+  });
+
+  it('should prefer the error state over loading', () => {
+    renderWithProviders(<NotificationsRoute />, {
+      globalError: Errors.UNKNOWN,
+      status: 'error',
+    });
+
+    expect(screen.getByText('Oops')).toBeInTheDocument();
+    expect(screen.queryByText('Loading')).not.toBeInTheDocument();
+  });
+
   it('should render itself & its children (show account header)', () => {
     const tree = renderWithProviders(<NotificationsRoute />, {
       notifications: [mockMultipleAccountNotifications[0]],

src/renderer/routes/Notifications.tsx

@@ -6,6 +6,7 @@ import { useAccountsStore, useFiltersStore, useSettingsStore } from '../stores';
 
 import { AllRead } from '../components/AllRead';
 import { Page } from '../components/layout/Page';
+import { Loading } from '../components/Loading';
 import { NotificationList } from '../components/notifications/NotificationList';
 import { Oops } from '../components/Oops';
 
@@ -42,6 +43,10 @@ export function NotificationsRoute() {
     return <Oops error={globalError ?? Errors.UNKNOWN} />;
   }
 
+  if (status === 'loading') {
+    return <Loading />;
+  }
+
   if (!hasNotifications && noAccountErrors) {
     return <AllRead />;
   }