From 1c30d6208b7b0adbeae9d9e1165780c451751fdc Mon Sep 17 00:00:00 2001 From: Optic00 Date: Fri, 24 Jul 2026 08:28:51 +0200 Subject: [PATCH] fix(settings): reset to General on bare /settings so browser Back can't strand a stale tab (#405) Follow-up to #411: nav-rail clicks now push ?tab= entries onto the hash history, so browser Back can land on bare /settings - the route-sync effect previously ignored the absent param and left the old tab visible. Shares one tabFromRoute resolver between first-mount initialTab and the sync effect, and adds a T1 regression test. --- app/renderer/src/routes/Settings.tsx | 33 +++++++++++++++-------- e2e/specs/settings-cmdk-search.t1.spec.ts | 18 +++++++++++++ 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/app/renderer/src/routes/Settings.tsx b/app/renderer/src/routes/Settings.tsx index eb33b272..fbf2438f 100644 --- a/app/renderer/src/routes/Settings.tsx +++ b/app/renderer/src/routes/Settings.tsx @@ -61,6 +61,17 @@ function resolveTab(id: DeepLinkId): SettingsTabId { return id === 'transcription' ? 'ai' : id; } +// Resolve the `?tab=` param of a route to a nav tab, or null when absent or +// not a known deep-link id. Single definition shared by the first-mount +// initialTab and the route-reactive sync effect so the two can't drift. +function tabFromRoute(route: string): SettingsTabId | null { + const requested = getRouteParam(route, 'tab'); + if (requested && (DEEP_LINK_IDS as readonly string[]).includes(requested)) { + return resolveTab(requested as DeepLinkId); + } + return null; +} + // --------------------------------------------------------------------------- // Settings page — a full takeover of the main app chrome (Granola/Wispr // style): the folder/meeting sidebar is replaced entirely by SettingsNav @@ -76,23 +87,23 @@ export function Settings() { // Used by the sidebar's "Sign in to organisation" CTA to land users // directly on the org sign-in form rather than the General tab. const route = useRoute(); - const initialTab = React.useMemo(() => { - const requested = getRouteParam(route, 'tab'); - if (requested && (DEEP_LINK_IDS as readonly string[]).includes(requested)) { - return resolveTab(requested as DeepLinkId); - } - return 'general'; - }, []); // Intentional — only consume the URL param on first mount. + const initialTab = React.useMemo( + () => tabFromRoute(route) ?? 'general', + [], // Intentional — only consume the URL param on first mount. + ); const [tab, setTab] = React.useState(initialTab); // Keep the visible tab in sync when the `?tab=` param changes AFTER mount — // e.g. the ⌘K settings search navigates to /settings?tab= while Settings // is already open. initialTab (above) only consumes the param on first mount, // so without this the hash would update but the tab wouldn't switch. + // + // A bare `/settings` (no param) resets to General — the same meaning the + // absent param has on first mount. This matters for browser Back: nav-rail + // clicks push `?tab=` entries onto the hash history (route is the single + // source of truth since #411), so Back can land on the bare route and must + // not leave a stale tab visible. React.useEffect(() => { - const requested = getRouteParam(route, 'tab'); - if (requested && (DEEP_LINK_IDS as readonly string[]).includes(requested)) { - setTab(resolveTab(requested as DeepLinkId)); - } + setTab(tabFromRoute(route) ?? 'general'); }, [route]); const version = useAppVersion(); // Templates' own editor is a full-page takeover with its own header/back diff --git a/e2e/specs/settings-cmdk-search.t1.spec.ts b/e2e/specs/settings-cmdk-search.t1.spec.ts index 63b79462..6c58d55a 100644 --- a/e2e/specs/settings-cmdk-search.t1.spec.ts +++ b/e2e/specs/settings-cmdk-search.t1.spec.ts @@ -106,6 +106,24 @@ test('search still works after a manual nav-rail tab switch (#405 regression)', await expect(page.locator(settingsPage)).not.toContainText('Launch on login'); }); +test('browser Back to bare /settings resets the visible tab to General (#405)', async ({ + launchApp, +}) => { + const { page } = await launchApp(launchOpts); + await openSettings(page); // bare /settings → General + + // Nav to AI via the rail — pushes /settings?tab=ai onto the hash history. + await page.locator('[data-settings-nav="ai"]').click(); + await expect(page.locator(settingsPage)).toContainText('AI provider'); + + // Browser Back returns to the bare route. The route→tab effect must treat + // the absent param like first mount (General), not leave the AI tab stale. + await page.goBack(); + await expect.poll(() => page.evaluate(() => window.location.hash)).toBe('#/settings'); + await expect(page.locator(settingsPage)).toContainText('Launch on login'); + await expect(page.locator(settingsPage)).not.toContainText('AI provider'); +}); + // Drift guard: a few index titles must still match the labels their tabs // actually render, so a renamed control can't leave a stale search entry that // jumps to a tab where nothing matches. Uses only cross-platform settings.