From 428d98c6d62e1703fe0c1c2f35e8486cdbe6e9b8 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 26 Aug 2026 15:32:49 +1000 Subject: [PATCH 1/5] feat(desktop): add back history menu Co-authored-by: Jitter Signed-off-by: Matt Toohey --- desktop/src/app/AppShell.tsx | 11 +- desktop/src/app/AppTopChrome.tsx | 134 ++++++++++++++++-- .../app/navigation/navigationGuard.test.mjs | 32 ++++- desktop/src/app/navigation/navigationGuard.ts | 17 +++ .../app/navigation/navigationHistory.test.mjs | 80 +++++++++++ .../src/app/navigation/navigationHistory.ts | 85 +++++++++++ .../app/navigation/useBackForwardControls.ts | 86 ++++++++--- desktop/tests/e2e/navigation.spec.ts | 45 ++++++ 8 files changed, 451 insertions(+), 39 deletions(-) create mode 100644 desktop/src/app/navigation/navigationHistory.test.mjs create mode 100644 desktop/src/app/navigation/navigationHistory.ts diff --git a/desktop/src/app/AppShell.tsx b/desktop/src/app/AppShell.tsx index eb5ab5a95d8..e5a647fde5b 100644 --- a/desktop/src/app/AppShell.tsx +++ b/desktop/src/app/AppShell.tsx @@ -13,6 +13,7 @@ import { TerminalContextOverrideProvider, } from "@/app/TerminalContextOverrideContext"; import { useAppNavigation } from "@/app/navigation/useAppNavigation"; +import { describeHistoryLocation } from "@/app/navigation/navigationHistory"; import { useBackForwardControls } from "@/app/navigation/useBackForwardControls"; import { useCommunityNavigationTransitions } from "@/app/useCommunityNavigationTransitions"; import { useLiveHomeFeedActions } from "@/app/useLiveHomeFeedActions"; @@ -141,6 +142,8 @@ export function AppShell() { const mainInsetRef = React.useRef(null); const location = useLocation(); const queryClient = useQueryClient(); + const channelsQuery = useChannelsQuery(); + const channels = channelsQuery.data ?? []; useManagedAgentRuntimeReconciliation(communitiesHook.communities); // sync storage snapshot const { goAgents, @@ -154,8 +157,8 @@ export function AppShell() { closeSettings, openSearchHit, } = useAppNavigation(); - const { canGoBack, canGoForward, goBack, goForward } = - useBackForwardControls(); + const { backHistory, canGoBack, canGoForward, goBack, goBackTo, goForward } = + useBackForwardControls(describeHistoryLocation(location, channels)); const { selectedChannelId, selectedView } = React.useMemo( () => deriveShellRoute(location.pathname), [location.pathname], @@ -233,8 +236,6 @@ export function AppShell() { const { feedProfilesQuery, homeFeedQuery, notificationSettings } = useHomeFeedNotifications(identityQuery.data?.pubkey); const feedItemState = useFeedItemState(identityQuery.data?.pubkey); - const channelsQuery = useChannelsQuery(); - const channels = channelsQuery.data ?? []; useReminderNotifications( identityQuery.data?.pubkey, notificationSettings.settings, @@ -774,10 +775,12 @@ export function AppShell() { {!settingsOpen && !isHuddleRoom ? ( ) : null} diff --git a/desktop/src/app/AppTopChrome.tsx b/desktop/src/app/AppTopChrome.tsx index 35b5e4b2093..f58cf1e6fb1 100644 --- a/desktop/src/app/AppTopChrome.tsx +++ b/desktop/src/app/AppTopChrome.tsx @@ -1,18 +1,27 @@ import * as React from "react"; import { ChevronLeft, ChevronRight } from "lucide-react"; +import type { BackHistoryEntry } from "@/app/navigation/navigationHistory"; import { isMacPlatform } from "@/shared/lib/platform"; import { useIsFullscreen } from "@/shared/lib/useIsFullscreen"; import { Button } from "@/shared/ui/button"; +import { + ContextMenu, + ContextMenuContent, + ContextMenuItem, + ContextMenuTrigger, +} from "@/shared/ui/context-menu"; import { DrawerPanelIcon } from "@/shared/ui/DrawerPanelIcon"; import { cn } from "@/shared/lib/cn"; import { topChromeBackdrop } from "@/shared/layout/chromeLayout"; import { useOptionalSidebar } from "@/shared/ui/sidebar"; type AppTopChromeProps = { + backHistory: BackHistoryEntry[]; canGoBack: boolean; canGoForward: boolean; onGoBack: () => void; + onGoBackTo: (index: number) => void; onGoForward: () => void; hasCommunityRail?: boolean; }; @@ -25,6 +34,7 @@ const TOP_CHROME_ICON_BUTTON_CLASS = "h-[28px] w-[28px] rounded-[4px] text-sidebar-foreground/65 hover:bg-sidebar-accent hover:text-sidebar-accent-foreground"; const HISTORY_ICON_BUTTON_CLASS = "h-[28px] w-[24px] rounded-[4px] text-sidebar-foreground/65 hover:bg-sidebar-accent hover:text-sidebar-accent-foreground [&_svg]:size-[16px]"; +const BACK_HISTORY_LONG_PRESS_MS = 500; function preventTopChromeWheel(event: WheelEvent) { event.preventDefault(); @@ -52,10 +62,117 @@ function TopChromeSidebarTrigger() { ); } +function BackHistoryButton({ + backHistory, + canGoBack, + onGoBack, + onGoBackTo, +}: Pick< + AppTopChromeProps, + "backHistory" | "canGoBack" | "onGoBack" | "onGoBackTo" +>) { + const buttonRef = React.useRef(null); + const longPressTimerRef = React.useRef(null); + const longPressTriggeredRef = React.useRef(false); + + const cancelLongPress = React.useCallback(() => { + if (longPressTimerRef.current !== null) { + window.clearTimeout(longPressTimerRef.current); + longPressTimerRef.current = null; + } + }, []); + + React.useEffect(() => cancelLongPress, [cancelLongPress]); + + const handlePointerDown = React.useCallback( + (event: React.PointerEvent) => { + if (event.button !== 0 || backHistory.length === 0) { + return; + } + + cancelLongPress(); + longPressTriggeredRef.current = false; + const { clientX, clientY } = event; + longPressTimerRef.current = window.setTimeout(() => { + longPressTimerRef.current = null; + longPressTriggeredRef.current = true; + buttonRef.current?.dispatchEvent( + new MouseEvent("contextmenu", { + bubbles: true, + button: 2, + cancelable: true, + clientX, + clientY, + view: window, + }), + ); + }, BACK_HISTORY_LONG_PRESS_MS); + }, + [backHistory.length, cancelLongPress], + ); + + return ( + { + if (!open) { + longPressTriggeredRef.current = false; + } + }} + > + + + + {backHistory.length > 0 ? ( + + {backHistory.map((entry) => ( + onGoBackTo(entry.index)} + > + {entry.label} + + ))} + + ) : null} + + ); +} + export function AppTopChrome({ + backHistory, canGoBack, canGoForward, onGoBack, + onGoBackTo, onGoForward, hasCommunityRail = false, }: AppTopChromeProps) { @@ -132,17 +249,12 @@ export function AppTopChrome({ >
- + - {backHistory.length > 0 ? ( + {entries.length > 0 ? ( - {backHistory.map((entry) => ( + {entries.map((entry) => ( onGoBackTo(entry.index)} + onSelect={() => onGoTo(entry.index)} > {entry.label} @@ -171,9 +183,11 @@ export function AppTopChrome({ backHistory, canGoBack, canGoForward, + forwardHistory, onGoBack, onGoBackTo, onGoForward, + onGoForwardTo, hasCommunityRail = false, }: AppTopChromeProps) { const topChromeRef = React.useRef(null); @@ -249,23 +263,20 @@ export function AppTopChrome({ >
- + -
{ ); }); +test("forward history returns the nearest ten entries in navigation order", () => { + const entriesByIndex = new Map( + Array.from({ length: 14 }, (_, index) => [index, entry(index)]), + ); + + assert.deepEqual( + getForwardHistoryEntries(entriesByIndex, 0, 13).map(({ index }) => index), + [1, 2, 3, 4, 5, 6, 7, 8, 9, 10], + ); +}); + test("history labels identify channel and thread destinations", () => { const channels = [ { diff --git a/desktop/src/app/navigation/navigationHistory.ts b/desktop/src/app/navigation/navigationHistory.ts index dc4b09452b5..6ba95efb73a 100644 --- a/desktop/src/app/navigation/navigationHistory.ts +++ b/desktop/src/app/navigation/navigationHistory.ts @@ -1,22 +1,22 @@ import type { Channel } from "@/shared/api/types"; -export type BackHistoryEntry = { +export type NavigationHistoryEntry = { index: number; key: string; label: string; }; -const MAX_BACK_HISTORY_ENTRIES = 10; +const MAX_HISTORY_MENU_ENTRIES = 10; export function getBackHistoryEntries( - entriesByIndex: ReadonlyMap, + entriesByIndex: ReadonlyMap, currentIndex: number, -): BackHistoryEntry[] { - const entries: BackHistoryEntry[] = []; +): NavigationHistoryEntry[] { + const entries: NavigationHistoryEntry[] = []; for ( let index = currentIndex - 1; - index >= 0 && entries.length < MAX_BACK_HISTORY_ENTRIES; + index >= 0 && entries.length < MAX_HISTORY_MENU_ENTRIES; index -= 1 ) { const entry = entriesByIndex.get(index); @@ -28,6 +28,27 @@ export function getBackHistoryEntries( return entries; } +export function getForwardHistoryEntries( + entriesByIndex: ReadonlyMap, + currentIndex: number, + maxIndex: number, +): NavigationHistoryEntry[] { + const entries: NavigationHistoryEntry[] = []; + + for ( + let index = currentIndex + 1; + index <= maxIndex && entries.length < MAX_HISTORY_MENU_ENTRIES; + index += 1 + ) { + const entry = entriesByIndex.get(index); + if (entry) { + entries.push(entry); + } + } + + return entries; +} + type HistoryLocation = { pathname: string; search: unknown; diff --git a/desktop/src/app/navigation/useBackForwardControls.ts b/desktop/src/app/navigation/useBackForwardControls.ts index 2da1d726f77..f9c87575d1c 100644 --- a/desktop/src/app/navigation/useBackForwardControls.ts +++ b/desktop/src/app/navigation/useBackForwardControls.ts @@ -9,8 +9,9 @@ import { listen } from "@tauri-apps/api/event"; import { matchBackForwardChord } from "@/app/navigation/backForwardChords"; import { - type BackHistoryEntry, + type NavigationHistoryEntry, getBackHistoryEntries, + getForwardHistoryEntries, } from "@/app/navigation/navigationHistory"; import { traverseHistory, @@ -37,7 +38,7 @@ export function useBackForwardControls(currentLabel: string) { const locationKey = locationState.__TSR_key ?? locationState.key ?? String(locationIndex); const [historyState, setHistoryState] = React.useState(() => ({ - entriesByIndex: new Map([ + entriesByIndex: new Map([ [ locationIndex, { index: locationIndex, key: locationKey, label: currentLabel }, @@ -82,6 +83,15 @@ export function useBackForwardControls(currentLabel: string) { () => getBackHistoryEntries(historyState.entriesByIndex, locationIndex), [historyState.entriesByIndex, locationIndex], ); + const forwardHistory = React.useMemo( + () => + getForwardHistoryEntries( + historyState.entriesByIndex, + locationIndex, + historyState.maxIndex, + ), + [historyState.entriesByIndex, historyState.maxIndex, locationIndex], + ); const goBack = React.useCallback(() => { if (!canGoBack) { @@ -111,6 +121,18 @@ export function useBackForwardControls(currentLabel: string) { [historyState.entriesByIndex, locationIndex, router.history], ); + const goForwardTo = React.useCallback( + (index: number) => { + const delta = index - locationIndex; + if (delta <= 0 || !historyState.entriesByIndex.has(index)) { + return; + } + + traverseHistoryBy(router.history, delta); + }, + [historyState.entriesByIndex, locationIndex, router.history], + ); + const handleKeyDown = React.useEffectEvent((event: KeyboardEvent) => { // Note: the chords deliberately fire even when focus is inside an // editable element. The composer autofocuses on every channel switch @@ -172,8 +194,10 @@ export function useBackForwardControls(currentLabel: string) { backHistory, canGoBack, canGoForward, + forwardHistory, goBack, goBackTo, goForward, + goForwardTo, }; } diff --git a/desktop/tests/e2e/navigation.spec.ts b/desktop/tests/e2e/navigation.spec.ts index dd97feee625..f93bac4da7d 100644 --- a/desktop/tests/e2e/navigation.spec.ts +++ b/desktop/tests/e2e/navigation.spec.ts @@ -1,5 +1,6 @@ import { expect, test } from "@playwright/test"; +import { waitForAnimations } from "../helpers/animations"; import { installMockBridge } from "../helpers/bridge"; import { openSettings } from "../helpers/settings"; @@ -40,6 +41,69 @@ async function navigateToWorkflows(page: import("@playwright/test").Page) { await expect(page.getByTestId("workflows-view")).toBeVisible(); } +async function openHistoryMenuWithLongPress( + page: import("@playwright/test").Page, + button: import("@playwright/test").Locator, + menu: import("@playwright/test").Locator, +) { + const bounds = await button.boundingBox(); + expect(bounds).not.toBeNull(); + await page.mouse.move( + (bounds?.x ?? 0) + (bounds?.width ?? 0) / 2, + (bounds?.y ?? 0) + (bounds?.height ?? 0) / 2, + ); + await page.mouse.down(); + await expect(menu).toBeVisible(); + await page.mouse.up(); + await expect(menu).toBeVisible(); +} + +async function captureHistoryControlAppearance( + page: import("@playwright/test").Page, + button: import("@playwright/test").Locator, + menu: import("@playwright/test").Locator, +) { + await waitForAnimations(page); + const [buttonBounds, menuBounds] = await Promise.all([ + button.boundingBox(), + menu.boundingBox(), + ]); + if (!buttonBounds || !menuBounds) { + throw new Error("History control is not visible"); + } + + const padding = 4; + const x = Math.max( + 0, + Math.floor(Math.min(buttonBounds.x, menuBounds.x)) - padding, + ); + const y = Math.max( + 0, + Math.floor(Math.min(buttonBounds.y, menuBounds.y)) - padding, + ); + const right = Math.ceil( + Math.max( + buttonBounds.x + buttonBounds.width, + menuBounds.x + menuBounds.width, + ), + ); + const bottom = Math.ceil( + Math.max( + buttonBounds.y + buttonBounds.height, + menuBounds.y + menuBounds.height, + ), + ); + + return page.screenshot({ + clip: { + height: bottom - y + padding, + width: right - x + padding, + x, + y, + }, + }); +} + async function createWorkflow( page: import("@playwright/test").Page, name: string, @@ -93,7 +157,7 @@ test("global back and forward move across channel routes", async ({ page }) => { await expect(page.getByTestId("chat-title")).toHaveText("random"); }); -test("back history menu opens on right click and long press", async ({ +test("back and forward history menus match across right click and long press", async ({ page, }) => { await page.goto("/"); @@ -116,26 +180,60 @@ test("back history menu opens on right click and long press", async ({ "#general", "Inbox", ]); + const rightClickAppearance = await captureHistoryControlAppearance( + page, + backButton, + historyMenu, + ); + await page.keyboard.press("Escape"); + await expect(historyMenu).not.toBeVisible(); + + await openHistoryMenuWithLongPress(page, backButton, historyMenu); + await expect(historyMenu.getByTestId("global-back-history-item")).toHaveText([ + "#random", + "#general", + "Inbox", + ]); + expect( + await captureHistoryControlAppearance(page, backButton, historyMenu), + ).toEqual(rightClickAppearance); await historyMenu .getByRole("menuitem", { name: "Go back to #general" }) .click(); await expect(page.getByTestId("chat-title")).toHaveText("general"); - await page.getByTestId("channel-deep-history").click(); - await expect(page.getByTestId("chat-title")).toHaveText("deep-history"); + const forwardButton = page.getByTestId("global-forward"); + const forwardHistoryMenu = page.getByTestId("global-forward-history-menu"); - const bounds = await backButton.boundingBox(); - expect(bounds).not.toBeNull(); - await page.mouse.move( - (bounds?.x ?? 0) + (bounds?.width ?? 0) / 2, - (bounds?.y ?? 0) + (bounds?.height ?? 0) / 2, + await expect(forwardButton).toHaveAttribute("data-history-count", "2"); + await forwardButton.click({ button: "right" }); + await expect(forwardHistoryMenu).toBeVisible(); + await expect( + forwardHistoryMenu.getByTestId("global-forward-history-item"), + ).toHaveText(["#random", "#engineering"]); + const forwardRightClickAppearance = await captureHistoryControlAppearance( + page, + forwardButton, + forwardHistoryMenu, ); - await page.mouse.down(); - await expect(historyMenu).toBeVisible(); - await page.mouse.up(); - await expect(historyMenu).toBeVisible(); - await historyMenu.getByRole("menuitem", { name: "Go back to Inbox" }).click(); - await expect(page.getByTestId("home-inbox")).toBeVisible(); + await page.keyboard.press("Escape"); + await expect(forwardHistoryMenu).not.toBeVisible(); + + await openHistoryMenuWithLongPress(page, forwardButton, forwardHistoryMenu); + await expect( + forwardHistoryMenu.getByTestId("global-forward-history-item"), + ).toHaveText(["#random", "#engineering"]); + expect( + await captureHistoryControlAppearance( + page, + forwardButton, + forwardHistoryMenu, + ), + ).toEqual(forwardRightClickAppearance); + await forwardHistoryMenu + .getByRole("menuitem", { name: "Go forward to #engineering" }) + .click(); + await expect(page.getByTestId("chat-title")).toHaveText("engineering"); }); test("back/forward keyboard chords work while the composer has focus", async ({ From 4417928f92c4063d1d6c6d1e5b3cb54f0ac54c5f Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 26 Aug 2026 16:19:28 +1000 Subject: [PATCH 3/5] fix(desktop): satisfy AppShell size ratchet Co-authored-by: Jitter Signed-off-by: Matt Toohey --- desktop/src/app/AppShell.tsx | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/desktop/src/app/AppShell.tsx b/desktop/src/app/AppShell.tsx index 8ffafb5ff17..de50d19451c 100644 --- a/desktop/src/app/AppShell.tsx +++ b/desktop/src/app/AppShell.tsx @@ -157,16 +157,9 @@ export function AppShell() { closeSettings, openSearchHit, } = useAppNavigation(); - const { - backHistory, - canGoBack, - canGoForward, - forwardHistory, - goBack, - goBackTo, - goForward, - goForwardTo, - } = useBackForwardControls(describeHistoryLocation(location, channels)); + const navigationControls = useBackForwardControls( + describeHistoryLocation(location, channels), + ); const { selectedChannelId, selectedView } = React.useMemo( () => deriveShellRoute(location.pathname), [location.pathname], @@ -783,15 +776,15 @@ export function AppShell() { {!settingsOpen && !isHuddleRoom ? ( ) : null} {settingsOpen ? ( From 3485f05cd59aa24a261249b072f2cb882fe1dc38 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 27 Aug 2026 14:43:17 +1000 Subject: [PATCH 4/5] fix(desktop): keep forward history across replace navigations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the review comments on the back/forward history menus. A `replace: true` navigation mints a fresh __TSR_key at the *same* history index, which the old heuristic read as "the forward stack was truncated". Mid-history replaces — the huddle redirects, settings section switches, `goHome({ replace: true })` — therefore wiped the tracked forward entries and disabled the forward control even though `history.go(1)` still worked. Only a push moves the index forward, so truncation now keys off an index that advanced past a stale entry. The fold is extracted as a pure `recordHistoryVisit` reducer so the push/replace/traverse cases are unit-testable without a renderer. Also: - Route labels move to a table that falls back to the raw pathname. The old chain ended in `return "Inbox"`, so any future route would have shown up in the menus as the inbox with nothing to flag the omission. - The right-click vs long-press E2E now compares menu geometry and item text instead of raw screenshot bytes; a mismatch names the field that drifted rather than handing back an opaque Buffer diff. The reported left-click-while-open double action does not reproduce: Radix's modal dismiss layer sets `pointer-events: none` on the body, so the click lands on the layer and never reaches the button — one click dismisses and nothing else, matching the native control. Locked in with an E2E case that also proves the next click still navigates. Verified: pnpm typecheck, pnpm check, pnpm test (5517 pass), check:file-sizes, and the navigation smoke spec (21 pass, 1 pre-existing fixme). Signed-off-by: Matt Toohey --- .../app/navigation/navigationHistory.test.mjs | 87 ++++++++++++++ .../src/app/navigation/navigationHistory.ts | 98 ++++++++++++++-- .../app/navigation/useBackForwardControls.ts | 51 +++------ desktop/tests/e2e/navigation.spec.ts | 107 +++++++++++------- 4 files changed, 256 insertions(+), 87 deletions(-) diff --git a/desktop/src/app/navigation/navigationHistory.test.mjs b/desktop/src/app/navigation/navigationHistory.test.mjs index 8886a95d4be..b6a04eab2c4 100644 --- a/desktop/src/app/navigation/navigationHistory.test.mjs +++ b/desktop/src/app/navigation/navigationHistory.test.mjs @@ -2,15 +2,21 @@ import assert from "node:assert/strict"; import test from "node:test"; import { + createNavigationHistoryState, describeHistoryLocation, getBackHistoryEntries, getForwardHistoryEntries, + recordHistoryVisit, } from "./navigationHistory.ts"; function entry(index) { return { index, key: `key-${index}`, label: `Entry ${index}` }; } +function visit(state, index, key, label) { + return recordHistoryVisit(state, { index, key, label }); +} + test("back history returns the nearest ten entries in reverse order", () => { const entriesByIndex = new Map( Array.from({ length: 13 }, (_, index) => [index, entry(index)]), @@ -73,6 +79,80 @@ test("history labels identify channel and thread destinations", () => { ); }); +test("replacing an entry mid-history keeps the forward entries", () => { + // Inbox → #general → #random, then back to #general. + let state = createNavigationHistoryState({ + index: 0, + key: "key-inbox", + label: "Inbox", + }); + state = visit(state, 1, "key-general", "#general"); + state = visit(state, 2, "key-random", "#random"); + state = visit(state, 1, "key-general", "#general"); + + // A `replace: true` navigation mints a fresh key at the same index while the + // browser keeps entry 2 reachable with forward. + state = visit(state, 1, "key-general-replaced", "#general thread"); + + assert.equal(state.maxIndex, 2); + assert.deepEqual( + getForwardHistoryEntries(state.entriesByIndex, 1, state.maxIndex).map( + ({ label }) => label, + ), + ["#random"], + ); + assert.equal(state.entriesByIndex.get(1).label, "#general thread"); +}); + +test("pushing from mid-history drops the stale forward entries", () => { + let state = createNavigationHistoryState({ + index: 0, + key: "key-inbox", + label: "Inbox", + }); + state = visit(state, 1, "key-general", "#general"); + state = visit(state, 2, "key-random", "#random"); + state = visit(state, 1, "key-general", "#general"); + state = visit(state, 0, "key-inbox", "Inbox"); + + // Navigating anew from the inbox invalidates both entries ahead of it. + state = visit(state, 1, "key-design", "#design"); + + assert.equal(state.maxIndex, 1); + assert.deepEqual([...state.entriesByIndex.keys()], [0, 1]); + assert.deepEqual( + getForwardHistoryEntries(state.entriesByIndex, 1, state.maxIndex), + [], + ); +}); + +test("traversing back and forward leaves the tracked entries untouched", () => { + let state = createNavigationHistoryState({ + index: 0, + key: "key-inbox", + label: "Inbox", + }); + state = visit(state, 1, "key-general", "#general"); + state = visit(state, 2, "key-random", "#random"); + state = visit(state, 0, "key-inbox", "Inbox"); + + assert.equal(state.maxIndex, 2); + assert.deepEqual( + getForwardHistoryEntries(state.entriesByIndex, 0, state.maxIndex).map( + ({ label }) => label, + ), + ["#general", "#random"], + ); + + state = visit(state, 2, "key-random", "#random"); + + assert.equal(state.maxIndex, 2); + assert.deepEqual( + getBackHistoryEntries(state.entriesByIndex, 2).map(({ label }) => label), + ["#general", "Inbox"], + ); +}); + test("history labels cover static and detail routes", () => { assert.equal( describeHistoryLocation({ pathname: "/", search: {} }, []), @@ -90,3 +170,10 @@ test("history labels cover static and detail routes", () => { "Project details", ); }); + +test("an unlabelled route falls back to its pathname, not the inbox", () => { + assert.equal( + describeHistoryLocation({ pathname: "/not-labelled-yet", search: {} }, []), + "/not-labelled-yet", + ); +}); diff --git a/desktop/src/app/navigation/navigationHistory.ts b/desktop/src/app/navigation/navigationHistory.ts index 6ba95efb73a..8dc7844cdf4 100644 --- a/desktop/src/app/navigation/navigationHistory.ts +++ b/desktop/src/app/navigation/navigationHistory.ts @@ -1,4 +1,5 @@ import type { Channel } from "@/shared/api/types"; +import { trimMapToSize } from "@/shared/lib/trimMapToSize"; export type NavigationHistoryEntry = { index: number; @@ -6,7 +7,66 @@ export type NavigationHistoryEntry = { label: string; }; +export type NavigationHistoryState = { + entriesByIndex: Map; + /** Index of the entry the previous visit landed on. */ + lastIndex: number; + /** Highest index still reachable with forward navigation. */ + maxIndex: number; +}; + const MAX_HISTORY_MENU_ENTRIES = 10; +const MAX_TRACKED_HISTORY_ENTRIES = 200; + +export function createNavigationHistoryState( + entry: NavigationHistoryEntry, +): NavigationHistoryState { + return { + entriesByIndex: new Map([[entry.index, entry]]), + lastIndex: entry.index, + maxIndex: entry.index, + }; +} + +/** + * Folds the location the router just landed on into the tracked history. + * + * TanStack's history mints a fresh `__TSR_key` for pushes *and* replaces, so + * the key alone cannot tell them apart. Only a push moves the index forward, + * and only a push from mid-history drops the browser's forward entries. A + * replace performed mid-history — `goHome({ replace: true })`, the huddle + * redirects, settings section switches — keeps them, so it must not clear + * what we track or forward navigation would go dark while `history.go(1)` + * still works. + */ +export function recordHistoryVisit( + state: NavigationHistoryState, + entry: NavigationHistoryEntry, +): NavigationHistoryState { + const entriesByIndex = new Map(state.entriesByIndex); + const pushedOverForwardEntries = + entry.index > state.lastIndex && + entriesByIndex.get(entry.index)?.key !== entry.key; + + if (pushedOverForwardEntries) { + for (const storedIndex of entriesByIndex.keys()) { + if (storedIndex >= entry.index) { + entriesByIndex.delete(storedIndex); + } + } + } + + entriesByIndex.set(entry.index, entry); + trimMapToSize(entriesByIndex, MAX_TRACKED_HISTORY_ENTRIES); + + return { + entriesByIndex, + lastIndex: entry.index, + maxIndex: pushedOverForwardEntries + ? entry.index + : Math.max(state.maxIndex, entry.index), + }; +} export function getBackHistoryEntries( entriesByIndex: ReadonlyMap, @@ -63,6 +123,25 @@ function searchHasValue(search: unknown, key: string): boolean { return typeof value === "string" && value.length > 0; } +// Mirrors the route table in `routes.ts`. A route missing from here falls back +// to its raw pathname rather than a plausible-looking label, so the omission is +// visible instead of masquerading as some other destination. +const ROUTE_LABELS: Record = { + "/": "Inbox", + "/agents": "Agents", + "/messages/new": "New message", + "/projects": "Projects", + "/pulse": "Pulse", + "/reminders": "Reminders", + "/settings": "Settings", + "/workflows": "Workflows", +}; + +const ROUTE_PREFIX_LABELS: readonly (readonly [string, string])[] = [ + ["/projects/", "Project details"], + ["/workflows/", "Workflow details"], +]; + export function describeHistoryLocation( location: HistoryLocation, channels: readonly Channel[], @@ -92,15 +171,12 @@ export function describeHistoryLocation( return channelLabel; } - if (pathname === "/messages/new") return "New message"; - if (pathname === "/agents") return "Agents"; - if (pathname === "/workflows") return "Workflows"; - if (pathname.startsWith("/workflows/")) return "Workflow details"; - if (pathname === "/projects") return "Projects"; - if (pathname.startsWith("/projects/")) return "Project details"; - if (pathname === "/pulse") return "Pulse"; - if (pathname === "/reminders") return "Reminders"; - if (pathname === "/settings") return "Settings"; - - return "Inbox"; + const routeLabel = ROUTE_LABELS[pathname]; + if (routeLabel) return routeLabel; + + for (const [prefix, label] of ROUTE_PREFIX_LABELS) { + if (pathname.startsWith(prefix)) return label; + } + + return pathname || "Inbox"; } diff --git a/desktop/src/app/navigation/useBackForwardControls.ts b/desktop/src/app/navigation/useBackForwardControls.ts index f9c87575d1c..91e9708082e 100644 --- a/desktop/src/app/navigation/useBackForwardControls.ts +++ b/desktop/src/app/navigation/useBackForwardControls.ts @@ -9,16 +9,16 @@ import { listen } from "@tauri-apps/api/event"; import { matchBackForwardChord } from "@/app/navigation/backForwardChords"; import { - type NavigationHistoryEntry, + createNavigationHistoryState, getBackHistoryEntries, getForwardHistoryEntries, + recordHistoryVisit, } from "@/app/navigation/navigationHistory"; import { traverseHistory, traverseHistoryBy, } from "@/app/navigation/navigationGuard"; import { isMacPlatform } from "@/shared/lib/platform"; -import { trimMapToSize } from "@/shared/lib/trimMapToSize"; type RouterHistoryState = { __TSR_index?: number; @@ -26,8 +26,6 @@ type RouterHistoryState = { key?: string; }; -const MAX_TRACKED_HISTORY_ENTRIES = 200; - export function useBackForwardControls(currentLabel: string) { const router = useRouter(); const canGoBack = useCanGoBack(); @@ -37,45 +35,22 @@ export function useBackForwardControls(currentLabel: string) { const locationIndex = locationState.__TSR_index ?? 0; const locationKey = locationState.__TSR_key ?? locationState.key ?? String(locationIndex); - const [historyState, setHistoryState] = React.useState(() => ({ - entriesByIndex: new Map([ - [ - locationIndex, - { index: locationIndex, key: locationKey, label: currentLabel }, - ], - ]), - maxIndex: locationIndex, - })); + const [historyState, setHistoryState] = React.useState(() => + createNavigationHistoryState({ + index: locationIndex, + key: locationKey, + label: currentLabel, + }), + ); React.useEffect(() => { - setHistoryState((current) => { - const entriesByIndex = new Map(current.entriesByIndex); - const currentEntry = entriesByIndex.get(locationIndex); - const replacedForwardEntry = - currentEntry !== undefined && currentEntry.key !== locationKey; - - if (replacedForwardEntry) { - for (const storedIndex of entriesByIndex.keys()) { - if (storedIndex >= locationIndex) { - entriesByIndex.delete(storedIndex); - } - } - } - - entriesByIndex.set(locationIndex, { + setHistoryState((current) => + recordHistoryVisit(current, { index: locationIndex, key: locationKey, label: currentLabel, - }); - trimMapToSize(entriesByIndex, MAX_TRACKED_HISTORY_ENTRIES); - - return { - entriesByIndex, - maxIndex: replacedForwardEntry - ? locationIndex - : Math.max(current.maxIndex, locationIndex), - }; - }); + }), + ); }, [currentLabel, locationIndex, locationKey]); const canGoForward = locationIndex < historyState.maxIndex; diff --git a/desktop/tests/e2e/navigation.spec.ts b/desktop/tests/e2e/navigation.spec.ts index f93bac4da7d..aa9244491d4 100644 --- a/desktop/tests/e2e/navigation.spec.ts +++ b/desktop/tests/e2e/navigation.spec.ts @@ -58,50 +58,37 @@ async function openHistoryMenuWithLongPress( await expect(menu).toBeVisible(); } -async function captureHistoryControlAppearance( +/** + * Describes an open history menu by where it sits relative to its button, how + * big it is, and what it lists. Comparing this instead of raw pixels keeps the + * "both gestures open the same menu" assertion legible: a mismatch names the + * field that drifted rather than handing back an opaque Buffer diff. + */ +async function readHistoryMenuLayout( page: import("@playwright/test").Page, button: import("@playwright/test").Locator, menu: import("@playwright/test").Locator, + itemTestId: string, ) { await waitForAnimations(page); - const [buttonBounds, menuBounds] = await Promise.all([ + const [buttonBounds, menuBounds, items] = await Promise.all([ button.boundingBox(), menu.boundingBox(), + menu.getByTestId(itemTestId).allTextContents(), ]); if (!buttonBounds || !menuBounds) { throw new Error("History control is not visible"); } - const padding = 4; - const x = Math.max( - 0, - Math.floor(Math.min(buttonBounds.x, menuBounds.x)) - padding, - ); - const y = Math.max( - 0, - Math.floor(Math.min(buttonBounds.y, menuBounds.y)) - padding, - ); - const right = Math.ceil( - Math.max( - buttonBounds.x + buttonBounds.width, - menuBounds.x + menuBounds.width, - ), - ); - const bottom = Math.ceil( - Math.max( - buttonBounds.y + buttonBounds.height, - menuBounds.y + menuBounds.height, - ), - ); - - return page.screenshot({ - clip: { - height: bottom - y + padding, - width: right - x + padding, - x, - y, - }, - }); + return { + height: Math.round(menuBounds.height), + items, + // Radix anchors a context menu at the pointer, and both gestures point at + // the button's centre, so the offset must come out identical. + offsetX: Math.round(menuBounds.x - buttonBounds.x), + offsetY: Math.round(menuBounds.y - buttonBounds.y), + width: Math.round(menuBounds.width), + }; } async function createWorkflow( @@ -180,10 +167,11 @@ test("back and forward history menus match across right click and long press", a "#general", "Inbox", ]); - const rightClickAppearance = await captureHistoryControlAppearance( + const rightClickLayout = await readHistoryMenuLayout( page, backButton, historyMenu, + "global-back-history-item", ); await page.keyboard.press("Escape"); await expect(historyMenu).not.toBeVisible(); @@ -195,8 +183,13 @@ test("back and forward history menus match across right click and long press", a "Inbox", ]); expect( - await captureHistoryControlAppearance(page, backButton, historyMenu), - ).toEqual(rightClickAppearance); + await readHistoryMenuLayout( + page, + backButton, + historyMenu, + "global-back-history-item", + ), + ).toEqual(rightClickLayout); await historyMenu .getByRole("menuitem", { name: "Go back to #general" }) .click(); @@ -211,10 +204,11 @@ test("back and forward history menus match across right click and long press", a await expect( forwardHistoryMenu.getByTestId("global-forward-history-item"), ).toHaveText(["#random", "#engineering"]); - const forwardRightClickAppearance = await captureHistoryControlAppearance( + const forwardRightClickLayout = await readHistoryMenuLayout( page, forwardButton, forwardHistoryMenu, + "global-forward-history-item", ); await page.keyboard.press("Escape"); await expect(forwardHistoryMenu).not.toBeVisible(); @@ -224,18 +218,55 @@ test("back and forward history menus match across right click and long press", a forwardHistoryMenu.getByTestId("global-forward-history-item"), ).toHaveText(["#random", "#engineering"]); expect( - await captureHistoryControlAppearance( + await readHistoryMenuLayout( page, forwardButton, forwardHistoryMenu, + "global-forward-history-item", ), - ).toEqual(forwardRightClickAppearance); + ).toEqual(forwardRightClickLayout); await forwardHistoryMenu .getByRole("menuitem", { name: "Go forward to #engineering" }) .click(); await expect(page.getByTestId("chat-title")).toHaveText("engineering"); }); +test("left clicking a history button only dismisses its open menu", async ({ + page, +}) => { + await page.goto("/"); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await page.getByTestId("channel-random").click(); + await expect(page.getByTestId("chat-title")).toHaveText("random"); + + const backButton = page.getByTestId("global-back"); + const historyMenu = page.getByTestId("global-back-history-menu"); + await backButton.click({ button: "right" }); + await expect(historyMenu).toBeVisible(); + await waitForAnimations(page); + + // The open menu is modal, so its dismiss layer — not the button — takes the + // click. Drive the mouse directly: Playwright's actionability checks refuse + // to click a button that is covered. + const bounds = await backButton.boundingBox(); + expect(bounds).not.toBeNull(); + await page.mouse.click( + (bounds?.x ?? 0) + (bounds?.width ?? 0) / 2, + (bounds?.y ?? 0) + (bounds?.height ?? 0) / 2, + ); + + // One click closes the menu and does nothing else, like the native control. + await expect(historyMenu).not.toBeVisible(); + await expect(page.getByTestId("chat-title")).toHaveText("random"); + + // The next click is an ordinary back — proof the dismissal neither consumed + // it nor left the button inert. + await backButton.click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); +}); + test("back/forward keyboard chords work while the composer has focus", async ({ page, }) => { From 9f53f25f9498af31caaffcce95b2ff163da5622b Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 28 Aug 2026 10:43:42 +1000 Subject: [PATCH 5/5] refactor(desktop): extract community destination restore hook AppShell.tsx crossed the 1000-line desktop ratchet again after the forward-history wiring. Move the community destination restore effect into its own hook alongside useCommunityNavigationTransitions instead of squeezing the call site further. Signed-off-by: Matt Toohey --- desktop/src/app/AppShell.tsx | 55 ++---------- .../src/app/useCommunityDestinationRestore.ts | 83 +++++++++++++++++++ 2 files changed, 89 insertions(+), 49 deletions(-) create mode 100644 desktop/src/app/useCommunityDestinationRestore.ts diff --git a/desktop/src/app/AppShell.tsx b/desktop/src/app/AppShell.tsx index de50d19451c..5576fb05c69 100644 --- a/desktop/src/app/AppShell.tsx +++ b/desktop/src/app/AppShell.tsx @@ -15,6 +15,7 @@ import { import { useAppNavigation } from "@/app/navigation/useAppNavigation"; import { describeHistoryLocation } from "@/app/navigation/navigationHistory"; import { useBackForwardControls } from "@/app/navigation/useBackForwardControls"; +import { useCommunityDestinationRestore } from "@/app/useCommunityDestinationRestore"; import { useCommunityNavigationTransitions } from "@/app/useCommunityNavigationTransitions"; import { useLiveHomeFeedActions } from "@/app/useLiveHomeFeedActions"; import { useChannelBrowserDialog } from "@/app/useChannelBrowserDialog"; @@ -82,11 +83,6 @@ import { CommunityRail } from "@/features/sidebar/ui/CommunityRail"; import { useChannelMutes } from "@/features/sidebar/lib/useChannelMutes"; import { useChannelStars } from "@/features/sidebar/lib/useChannelStars"; import { useCommunities } from "@/features/communities/useCommunities"; -import { - consumePendingCommunityRestore, - loadCommunityDestination, - saveCommunityDestination, -} from "@/features/communities/communityNavigationStorage"; import { useAddCommunityDialogState } from "@/features/communities/addCommunityPrefill"; import { useApplyTemplate } from "@/features/channel-templates/useApplyTemplate"; import { relayClient } from "@/shared/api/relayClient"; @@ -276,54 +272,15 @@ export function AppShell() { ), [huddleBackingChannelIds, memberChannels, revealedHuddleChannelIds], ); - const hasRestoredCommunityDestinationRef = React.useRef(false); - React.useEffect(() => { - const activeCommunityId = communitiesHook.activeCommunity?.id; - if ( - hasRestoredCommunityDestinationRef.current || - !channelsQuery.isSuccess || - channelsQuery.dataUpdatedAt === 0 || - !activeCommunityId - ) { - return; - } - hasRestoredCommunityDestinationRef.current = true; - - // Restoration belongs to an explicit community transition. Cold boot and - // reconnect remounts must preserve the route the user explicitly opened. - if (!consumePendingCommunityRestore(activeCommunityId)) { - return; - } - - const destination = loadCommunityDestination(activeCommunityId); - if (!destination || destination.kind === "home") { - return; - } - - const channelIsAvailable = sidebarChannels.some( - (channel) => channel.id === destination.channelId, - ); - if (!channelIsAvailable) { - saveCommunityDestination(activeCommunityId, { kind: "home" }); - void goHome({ replace: true }); - return; - } - - // The normal switch path writes the remembered channel into the hash before - // the target community mounts, so no intermediate Inbox frame is painted. - // Older transition callers may still arrive at neutral Home; repair those. - if (selectedView === "home") { - void goChannel(destination.channelId, { replace: true }); - } - }, [ - channelsQuery.dataUpdatedAt, - channelsQuery.isSuccess, - communitiesHook.activeCommunity?.id, + useCommunityDestinationRestore({ + activeCommunityId: communitiesHook.activeCommunity?.id, + channelsDataUpdatedAt: channelsQuery.dataUpdatedAt, + channelsLoaded: channelsQuery.isSuccess, goChannel, goHome, selectedView, sidebarChannels, - ]); + }); const [terminalContextOverride, setTerminalContextOverride] = React.useState(null); const { activeChannel, terminalContext } = useTerminalContext({ diff --git a/desktop/src/app/useCommunityDestinationRestore.ts b/desktop/src/app/useCommunityDestinationRestore.ts new file mode 100644 index 00000000000..502e2ffbbc3 --- /dev/null +++ b/desktop/src/app/useCommunityDestinationRestore.ts @@ -0,0 +1,83 @@ +import * as React from "react"; + +import type { deriveShellRoute } from "@/app/AppShell.helpers"; +import type { useAppNavigation } from "@/app/navigation/useAppNavigation"; +import { + consumePendingCommunityRestore, + loadCommunityDestination, + saveCommunityDestination, +} from "@/features/communities/communityNavigationStorage"; +import type { Channel } from "@/shared/api/types"; + +type ShellRoute = ReturnType; +type AppNavigation = ReturnType; + +/** + * Restores the channel a community was last viewed on, once for the first + * successful channel load after an explicit community transition. + */ +export function useCommunityDestinationRestore({ + activeCommunityId, + channelsDataUpdatedAt, + channelsLoaded, + goChannel, + goHome, + selectedView, + sidebarChannels, +}: { + activeCommunityId: string | undefined; + channelsDataUpdatedAt: number; + channelsLoaded: boolean; + goChannel: AppNavigation["goChannel"]; + goHome: AppNavigation["goHome"]; + selectedView: ShellRoute["selectedView"]; + sidebarChannels: Channel[]; +}) { + const hasRestoredCommunityDestinationRef = React.useRef(false); + React.useEffect(() => { + if ( + hasRestoredCommunityDestinationRef.current || + !channelsLoaded || + channelsDataUpdatedAt === 0 || + !activeCommunityId + ) { + return; + } + hasRestoredCommunityDestinationRef.current = true; + + // Restoration belongs to an explicit community transition. Cold boot and + // reconnect remounts must preserve the route the user explicitly opened. + if (!consumePendingCommunityRestore(activeCommunityId)) { + return; + } + + const destination = loadCommunityDestination(activeCommunityId); + if (!destination || destination.kind === "home") { + return; + } + + const channelIsAvailable = sidebarChannels.some( + (channel) => channel.id === destination.channelId, + ); + if (!channelIsAvailable) { + saveCommunityDestination(activeCommunityId, { kind: "home" }); + void goHome({ replace: true }); + return; + } + + // The normal switch path writes the remembered channel into the hash before + // the target community mounts, so no intermediate Inbox frame is painted. + // Older transition callers may still arrive at neutral Home; repair those. + if (selectedView === "home") { + void goChannel(destination.channelId, { replace: true }); + } + }, [ + activeCommunityId, + channelsDataUpdatedAt, + channelsLoaded, + goChannel, + goHome, + selectedView, + sidebarChannels, + ]); +}