From fae3efb4d848b2e85b63b0ecc06cd50e62a2e920 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 7 Mar 2026 10:32:20 -0500 Subject: [PATCH 1/4] feat(error): action buttons Signed-off-by: Adam Setch --- src/renderer/components/Oops.test.tsx | 33 +- src/renderer/components/Oops.tsx | 18 + .../components/layout/EmojiSplash.tsx | 15 +- .../__snapshots__/EmojiSplash.test.tsx.snap | 4 +- .../AccountNotifications.test.tsx.snap | 490 ------------------ src/renderer/types.ts | 9 + src/renderer/utils/errors.ts | 18 + 7 files changed, 86 insertions(+), 501 deletions(-) diff --git a/src/renderer/components/Oops.test.tsx b/src/renderer/components/Oops.test.tsx index bd1970715..d96074ae5 100644 --- a/src/renderer/components/Oops.test.tsx +++ b/src/renderer/components/Oops.test.tsx @@ -1,4 +1,7 @@ -import { act } from '@testing-library/react'; +import { act, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { PersonIcon } from '@primer/octicons-react'; import { ensureStableEmojis, @@ -7,6 +10,12 @@ import { import { Oops } from './Oops'; +const navigateMock = vi.fn(); +vi.mock('react-router-dom', async () => ({ + ...(await vi.importActual('react-router-dom')), + useNavigate: () => navigateMock, +})); + describe('renderer/components/Oops.tsx', () => { beforeEach(() => { ensureStableEmojis(); @@ -37,4 +46,26 @@ describe('renderer/components/Oops.tsx', () => { expect(tree.container).toMatchSnapshot(); }); + + it('should render action buttons and navigate on click', async () => { + const mockError = { + title: 'Error title', + descriptions: ['Error description'], + emojis: ['🔥'], + actions: [ + { + label: 'Go somewhere', + route: '/somewhere', + variant: 'danger' as const, + icon: PersonIcon, + }, + ], + }; + + renderWithAppContext(); + + await userEvent.click(screen.getByText('Go somewhere')); + + expect(navigateMock).toHaveBeenCalledWith('/somewhere'); + }); }); diff --git a/src/renderer/components/Oops.tsx b/src/renderer/components/Oops.tsx index 9724eece4..d0b1bae1a 100644 --- a/src/renderer/components/Oops.tsx +++ b/src/renderer/components/Oops.tsx @@ -1,4 +1,7 @@ import { type FC, useMemo } from 'react'; +import { useNavigate } from 'react-router-dom'; + +import { Button } from '@primer/react'; import { EmojiSplash } from './layout/EmojiSplash'; @@ -16,14 +19,29 @@ export const Oops: FC = ({ fullHeight = true, }: OopsProps) => { const err = error ?? Errors.UNKNOWN; + const navigate = useNavigate(); const emoji = useMemo( () => err.emojis[Math.floor(Math.random() * err.emojis.length)], [err], ); + const actions = err.actions?.length + ? err.actions.map((action) => ( + + )) + : null; + return ( = ({ - fullHeight = true, - subHeadings = [], - ...props -}: EmojiSplashProps) => { +export const EmojiSplash: FC = (props: EmojiSplashProps) => { return ( - + = ({
{props.heading}
 - {subHeadings.map((description, i) => { + {props.subHeadings?.map((description, i) => { const key = `error_description_${i}`; return (
@@ -39,6 +36,8 @@ export const EmojiSplash: FC = ({
); })} + + {props.actions &&
{props.actions}
} ); diff --git a/src/renderer/components/layout/__snapshots__/EmojiSplash.test.tsx.snap b/src/renderer/components/layout/__snapshots__/EmojiSplash.test.tsx.snap index 5dade9fe6..dbae2b13d 100644 --- a/src/renderer/components/layout/__snapshots__/EmojiSplash.test.tsx.snap +++ b/src/renderer/components/layout/__snapshots__/EmojiSplash.test.tsx.snap @@ -12,7 +12,7 @@ exports[`renderer/components/layout/EmojiSplash.tsx > should render itself & its data-portal-root="true" >
should render itself & its data-portal-root="true" >
should render itself - account error for multiple accounts 1`] = ` -
-
-
- -
-
-
-
- 🔥 -
-
- Error title -
-
-
- Error description -
-
-
-
-
-
-`; - -exports[`renderer/components/notifications/AccountNotifications.tsx > should render itself - account error for single account 1`] = ` -
-
-
- -
-
-
-
- 🔥 -
-
- Error title -
-
-
- Error description -
-
-
-
-
-
-`; - exports[`renderer/components/notifications/AccountNotifications.tsx > should render itself - group notifications by date 1`] = `
; } export type ErrorType = diff --git a/src/renderer/utils/errors.ts b/src/renderer/utils/errors.ts index 2f719f37e..308f892ff 100644 --- a/src/renderer/utils/errors.ts +++ b/src/renderer/utils/errors.ts @@ -1,3 +1,5 @@ +import { PersonIcon } from '@primer/octicons-react'; + import { Constants } from '../constants'; import type { AccountNotifications, ErrorType, GitifyError } from '../types'; @@ -7,11 +9,27 @@ export const Errors: Record = { title: 'Bad Credentials', descriptions: ['Your credentials are either invalid or expired.'], emojis: Constants.EMOJIS.ERRORS.BAD_CREDENTIALS, + actions: [ + { + label: 'Manage Accounts', + route: '/accounts', + variant: 'danger', + icon: PersonIcon, + }, + ], }, MISSING_SCOPES: { title: 'Missing Scopes', descriptions: ['Your credentials are missing a required API scope.'], emojis: Constants.EMOJIS.ERRORS.MISSING_SCOPES, + actions: [ + { + label: 'Manage Accounts', + route: '/accounts', + variant: 'danger', + icon: PersonIcon, + }, + ], }, NETWORK: { title: 'Network Error', From a0b839754ef08be3b1924a7280d6c558aaef9020 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 7 Mar 2026 13:40:20 -0500 Subject: [PATCH 2/4] fix: test mock for client id Signed-off-by: Adam Setch --- src/renderer/__helpers__/test-utils.tsx | 48 +- src/renderer/__helpers__/vitest.setup.ts | 10 + .../components/GlobalShortcuts.test.tsx | 222 ++------ src/renderer/components/Oops.test.tsx | 7 +- src/renderer/components/Sidebar.test.tsx | 210 +++----- .../components/layout/AppLayout.test.tsx | 8 +- .../AccountNotifications.test.tsx.snap | 490 ++++++++++++++++++ .../components/primitives/Header.test.tsx | 9 +- .../settings/SettingsFooter.test.tsx | 34 +- src/renderer/hooks/useNotifications.test.ts | 26 +- src/renderer/routes/Accounts.test.tsx | 9 +- src/renderer/routes/Filters.test.tsx | 9 +- src/renderer/routes/Login.test.tsx | 9 +- .../routes/LoginWithDeviceFlow.test.tsx | 9 +- .../routes/LoginWithOAuthApp.test.tsx | 9 +- .../LoginWithPersonalAccessToken.test.tsx | 9 +- src/renderer/routes/Settings.test.tsx | 25 +- 17 files changed, 698 insertions(+), 445 deletions(-) diff --git a/src/renderer/__helpers__/test-utils.tsx b/src/renderer/__helpers__/test-utils.tsx index 6765591a5..e6cd951e7 100644 --- a/src/renderer/__helpers__/test-utils.tsx +++ b/src/renderer/__helpers__/test-utils.tsx @@ -1,5 +1,6 @@ import { render } from '@testing-library/react'; import { type ReactElement, type ReactNode, useMemo } from 'react'; +import { MemoryRouter } from 'react-router-dom'; import { BaseStyles, ThemeProvider } from '@primer/react'; @@ -7,22 +8,39 @@ import { mockAuth, mockSettings } from '../__mocks__/state-mocks'; import { AppContext, type AppContextState } from '../context/App'; +export { navigateMock } from './vitest.setup'; export type DeepPartial = { [K in keyof T]?: DeepPartial }; +const EMPTY_APP_CONTEXT: TestAppContext = {}; + +interface RenderOptions extends Partial { + initialEntries?: string[]; +} + +/** + * Test context + */ +type TestAppContext = Partial; + /** * Props for the AppContextProvider wrapper */ interface AppContextProviderProps { readonly children: ReactNode; - readonly value?: Partial; + readonly value?: TestAppContext; + readonly initialEntries?: string[]; } /** * Wrapper component that provides ThemeProvider, BaseStyles, and AppContext * with sensible defaults for testing. */ -function AppContextProvider({ children, value = {} }: AppContextProviderProps) { - const defaultValue: AppContextState = useMemo(() => { +function AppContextProvider({ + children, + value = EMPTY_APP_CONTEXT, + initialEntries, +}: AppContextProviderProps) { + const defaultValue: TestAppContext = useMemo(() => { return { auth: mockAuth, settings: mockSettings, @@ -58,17 +76,19 @@ function AppContextProvider({ children, value = {} }: AppContextProviderProps) { updateFilter: vi.fn(), ...value, - } as AppContextState; + } as TestAppContext; }, [value]); return ( - - - - {children} - - - + + + + + {children} + + + + ); } @@ -80,11 +100,13 @@ function AppContextProvider({ children, value = {} }: AppContextProviderProps) { */ export function renderWithAppContext( ui: ReactElement, - context: Partial = {}, + { initialEntries, ...context }: RenderOptions = {}, ) { return render(ui, { wrapper: ({ children }) => ( - {children} + + {children} + ), }); } diff --git a/src/renderer/__helpers__/vitest.setup.ts b/src/renderer/__helpers__/vitest.setup.ts index 484651a00..30b4e80a5 100644 --- a/src/renderer/__helpers__/vitest.setup.ts +++ b/src/renderer/__helpers__/vitest.setup.ts @@ -2,6 +2,15 @@ import '@testing-library/jest-dom/vitest'; import { useFiltersStore } from '../stores'; +/** + * Shared navigate mock — import from test-utils in any test that needs to assert on navigation + */ +export const navigateMock = vi.fn(); +vi.mock('react-router-dom', async () => ({ + ...(await vi.importActual('react-router-dom')), + useNavigate: () => navigateMock, +})); + // Sets timezone to UTC for consistent date/time in tests and snapshots process.env.TZ = 'UTC'; @@ -10,6 +19,7 @@ process.env.TZ = 'UTC'; */ beforeEach(() => { useFiltersStore.getState().reset(); + navigateMock.mockReset(); }); /** diff --git a/src/renderer/components/GlobalShortcuts.test.tsx b/src/renderer/components/GlobalShortcuts.test.tsx index b7ac6669b..e09f866b7 100644 --- a/src/renderer/components/GlobalShortcuts.test.tsx +++ b/src/renderer/components/GlobalShortcuts.test.tsx @@ -1,17 +1,13 @@ import userEvent from '@testing-library/user-event'; -import { MemoryRouter } from 'react-router-dom'; -import { renderWithAppContext } from '../__helpers__/test-utils'; +import { renderWithAppContext, + navigateMock +} from '../__helpers__/test-utils'; import * as comms from '../utils/comms'; import * as links from '../utils/links'; import { GlobalShortcuts } from './GlobalShortcuts'; -const navigateMock = vi.fn(); -vi.mock('react-router-dom', async () => ({ - ...(await vi.importActual('react-router-dom')), - useNavigate: () => navigateMock, -})); describe('components/GlobalShortcuts.tsx', () => { const fetchNotificationsMock = vi.fn(); @@ -25,11 +21,7 @@ describe('components/GlobalShortcuts.tsx', () => { describe('key bindings', () => { describe('ignores keys that are not valid', () => { it('ignores B key', async () => { - renderWithAppContext( - - - , - ); + renderWithAppContext(); await userEvent.keyboard('b'); @@ -39,11 +31,7 @@ describe('components/GlobalShortcuts.tsx', () => { describe('home', () => { it('navigates home when pressing H key', async () => { - renderWithAppContext( - - - , - ); + renderWithAppContext(); await userEvent.keyboard('h'); @@ -57,14 +45,9 @@ describe('components/GlobalShortcuts.tsx', () => { .mockImplementation(vi.fn()); it('opens primary account GitHub notifications webpage when pressing N while logged in', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { isLoggedIn: true, - }, - ); + }); await userEvent.keyboard('n'); @@ -72,14 +55,9 @@ describe('components/GlobalShortcuts.tsx', () => { }); it('does not open primary account GitHub notifications webpage when logged out', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { isLoggedIn: false, - }, - ); + }); await userEvent.keyboard('n'); @@ -89,15 +67,10 @@ describe('components/GlobalShortcuts.tsx', () => { describe('focus mode', () => { it('toggles focus when pressing W while logged in', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { updateSetting: updateSettingMock, isLoggedIn: true, - }, - ); + }); await userEvent.keyboard('w'); @@ -105,16 +78,11 @@ describe('components/GlobalShortcuts.tsx', () => { }); it('does not toggle focus mode when loading', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { updateSetting: updateSettingMock, status: 'loading', isLoggedIn: true, - }, - ); + }); await userEvent.keyboard('w'); @@ -122,15 +90,10 @@ describe('components/GlobalShortcuts.tsx', () => { }); it('does not toggle focus mode when logged out', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { updateSetting: updateSettingMock, isLoggedIn: false, - }, - ); + }); await userEvent.keyboard('w'); @@ -140,14 +103,9 @@ describe('components/GlobalShortcuts.tsx', () => { describe('filters', () => { it('toggles filters when pressing F while logged in', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { isLoggedIn: true, - }, - ); + }); await userEvent.keyboard('f'); @@ -155,14 +113,9 @@ describe('components/GlobalShortcuts.tsx', () => { }); it('does not toggle filters when logged out', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { isLoggedIn: false, - }, - ); + }); await userEvent.keyboard('f'); @@ -176,14 +129,9 @@ describe('components/GlobalShortcuts.tsx', () => { .mockImplementation(vi.fn()); it('opens primary account GitHub issues webpage when pressing I while logged in', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { isLoggedIn: true, - }, - ); + }); await userEvent.keyboard('i'); @@ -191,14 +139,9 @@ describe('components/GlobalShortcuts.tsx', () => { }); it('does not open primary account GitHub issues webpage when logged out', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { isLoggedIn: false, - }, - ); + }); await userEvent.keyboard('n'); @@ -212,14 +155,9 @@ describe('components/GlobalShortcuts.tsx', () => { .mockImplementation(vi.fn()); it('opens primary account GitHub pull requests webpage when pressing N while logged in', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { isLoggedIn: true, - }, - ); + }); await userEvent.keyboard('p'); @@ -227,14 +165,9 @@ describe('components/GlobalShortcuts.tsx', () => { }); it('does not open primary account GitHub pull requests webpage when logged out', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { isLoggedIn: false, - }, - ); + }); await userEvent.keyboard('n'); @@ -244,14 +177,9 @@ describe('components/GlobalShortcuts.tsx', () => { describe('refresh', () => { it('refreshes notifications when pressing R key', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { fetchNotifications: fetchNotificationsMock, - }, - ); + }); await userEvent.keyboard('r'); @@ -260,14 +188,9 @@ describe('components/GlobalShortcuts.tsx', () => { }); it('does not refresh when status is loading', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { status: 'loading', - }, - ); + }); await userEvent.keyboard('r'); @@ -277,14 +200,9 @@ describe('components/GlobalShortcuts.tsx', () => { describe('settings', () => { it('toggles settings when pressing S while logged in', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { isLoggedIn: true, - }, - ); + }); await userEvent.keyboard('s'); @@ -292,14 +210,9 @@ describe('components/GlobalShortcuts.tsx', () => { }); it('does not toggle settings when logged out', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { isLoggedIn: false, - }, - ); + }); await userEvent.keyboard('s'); @@ -309,14 +222,7 @@ describe('components/GlobalShortcuts.tsx', () => { describe('accounts', () => { it('navigates to accounts when pressing A on settings route', async () => { - renderWithAppContext( - - - , - { - isLoggedIn: true, - }, - ); + renderWithAppContext(, { initialEntries: ['/settings'], isLoggedIn: true, }); await userEvent.keyboard('a'); @@ -324,14 +230,9 @@ describe('components/GlobalShortcuts.tsx', () => { }); it('does not trigger accounts when not on settings route', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { isLoggedIn: true, - }, - ); + }); await userEvent.keyboard('a'); @@ -341,14 +242,7 @@ describe('components/GlobalShortcuts.tsx', () => { describe('quit app', () => { it('quits the app when pressing Q on settings route', async () => { - renderWithAppContext( - - - , - { - isLoggedIn: true, - }, - ); + renderWithAppContext(, { initialEntries: ['/settings'], isLoggedIn: true, }); await userEvent.keyboard('q'); @@ -356,14 +250,9 @@ describe('components/GlobalShortcuts.tsx', () => { }); it('does not quit the app when not on settings route', async () => { - renderWithAppContext( - - - , - { + renderWithAppContext(, { isLoggedIn: true, - }, - ); + }); await userEvent.keyboard('q'); @@ -374,14 +263,14 @@ describe('components/GlobalShortcuts.tsx', () => { describe('modifiers', () => { it('ignores shortcuts when typing in an input', async () => { renderWithAppContext( - - - - , + <> + + + , { isLoggedIn: true, }, - ); + ); const input = document.getElementById( 'test-input', @@ -394,14 +283,14 @@ describe('components/GlobalShortcuts.tsx', () => { it('ignores shortcuts when typing in a textarea', async () => { renderWithAppContext( - - -