diff --git a/apps/loopover-ui/src/components/site/keyboard-shortcuts.test.tsx b/apps/loopover-ui/src/components/site/keyboard-shortcuts.test.tsx new file mode 100644 index 0000000000..8d516cc4a5 --- /dev/null +++ b/apps/loopover-ui/src/components/site/keyboard-shortcuts.test.tsx @@ -0,0 +1,83 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// #6811: app-shell.tsx registers its own "g " handler for /app/* SPA navigation. This site-wide +// handler used to fire alongside it unconditionally, racing a hard `window.location.assign` against the +// SPA navigation on the same "g r" / "g a" keystrokes. Control the reported route via a mocked useLocation. +const { useLocation } = vi.hoisted(() => ({ useLocation: vi.fn() })); +vi.mock("@tanstack/react-router", () => ({ + useLocation: () => useLocation(), +})); + +import { KeyboardShortcutsDialog } from "./keyboard-shortcuts"; + +function pressKeys(...keys: string[]) { + for (const key of keys) { + fireEvent.keyDown(window, { key }); + } +} + +describe("KeyboardShortcutsDialog g-prefix navigation (#6811)", () => { + let assign: ReturnType; + + beforeEach(() => { + assign = vi.fn(); + Object.defineProperty(window, "location", { + value: { ...window.location, assign }, + writable: true, + configurable: true, + }); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it("does not hard-navigate on 'g r' while an /app/* route is mounted", () => { + useLocation.mockReturnValue({ pathname: "/app/runs" }); + render(); + + pressKeys("g", "r"); + + // AppShell's own handler owns this keystroke on /app/* routes; the site-wide handler must stay inert. + expect(assign).not.toHaveBeenCalled(); + }); + + it("does not hard-navigate on 'g a' while an /app/* route is mounted", () => { + useLocation.mockReturnValue({ pathname: "/app/analytics" }); + render(); + + pressKeys("g", "a"); + + expect(assign).not.toHaveBeenCalled(); + }); + + it("still hard-navigates on 'g r' outside /app/*", () => { + useLocation.mockReturnValue({ pathname: "/docs" }); + render(); + + pressKeys("g", "r"); + + expect(assign).toHaveBeenCalledWith("/api"); + }); + + it("advertises the in-app destinations instead of the site-wide ones while on /app/*", () => { + useLocation.mockReturnValue({ pathname: "/app/runs" }); + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open keyboard shortcuts" })); + + expect(screen.getByText("Go to runs")).toBeTruthy(); + expect(screen.queryByText("Go to API reference")).toBeNull(); + }); + + it("advertises the site-wide destinations outside /app/*", () => { + useLocation.mockReturnValue({ pathname: "/docs" }); + render(); + + fireEvent.click(screen.getByRole("button", { name: "Open keyboard shortcuts" })); + + expect(screen.getByText("Go to API reference")).toBeTruthy(); + expect(screen.queryByText("Go to runs")).toBeNull(); + }); +}); diff --git a/apps/loopover-ui/src/components/site/keyboard-shortcuts.tsx b/apps/loopover-ui/src/components/site/keyboard-shortcuts.tsx index f9d29a91b5..a034110986 100644 --- a/apps/loopover-ui/src/components/site/keyboard-shortcuts.tsx +++ b/apps/loopover-ui/src/components/site/keyboard-shortcuts.tsx @@ -1,3 +1,4 @@ +import { useLocation } from "@tanstack/react-router"; import { Keyboard } from "lucide-react"; import { useEffect, useState } from "react"; import { toast } from "sonner"; @@ -16,44 +17,61 @@ interface ShortcutGroup { items: Shortcut[]; } -const GROUPS: ShortcutGroup[] = [ - { - group: "Navigation", - items: [ - { keys: ["⌘", "K"], label: "Open command palette" }, - { keys: ["g", "h"], label: "Go home" }, - { keys: ["g", "d"], label: "Go to docs" }, - { keys: ["g", "a"], label: "Go to app" }, - { keys: ["g", "r"], label: "Go to API reference" }, - ], - }, - { - group: "API reference", - items: [ - { keys: ["←"], label: "Previous endpoint" }, - { keys: ["→"], label: "Next endpoint" }, - { keys: ["["], label: "Previous tag section" }, - { keys: ["]"], label: "Next tag section" }, - ], - }, - { - group: "Reading", - items: [ - { keys: ["j"], label: "Scroll down" }, - { keys: ["k"], label: "Scroll up" }, - { keys: ["t"], label: "Back to top" }, - ], - }, - { - group: "General", - items: [ - { keys: ["?"], label: "Open this cheat sheet" }, - { keys: ["Esc"], label: "Close dialogs / menus" }, - ], - }, +// AppShell (app-shell.tsx) owns "g " navigation while an /app/* route is mounted, using its own +// destination map (o/w/r/p/a). The site-wide map below only applies outside /app/*, so the advertised +// list here must match whichever map is actually live (#6811). +const SITE_NAV_SHORTCUTS: Shortcut[] = [ + { keys: ["g", "h"], label: "Go home" }, + { keys: ["g", "d"], label: "Go to docs" }, + { keys: ["g", "a"], label: "Go to app" }, + { keys: ["g", "r"], label: "Go to API reference" }, ]; +const APP_NAV_SHORTCUTS: Shortcut[] = [ + { keys: ["g", "o"], label: "Go to overview" }, + { keys: ["g", "w"], label: "Go to workbench" }, + { keys: ["g", "r"], label: "Go to runs" }, + { keys: ["g", "p"], label: "Go to repositories" }, + { keys: ["g", "a"], label: "Go to analytics" }, +]; + +function buildGroups(isAppRoute: boolean): ShortcutGroup[] { + const navShortcuts = isAppRoute ? APP_NAV_SHORTCUTS : SITE_NAV_SHORTCUTS; + return [ + { + group: "Navigation", + items: [{ keys: ["⌘", "K"], label: "Open command palette" }, ...navShortcuts], + }, + { + group: "API reference", + items: [ + { keys: ["←"], label: "Previous endpoint" }, + { keys: ["→"], label: "Next endpoint" }, + { keys: ["["], label: "Previous tag section" }, + { keys: ["]"], label: "Next tag section" }, + ], + }, + { + group: "Reading", + items: [ + { keys: ["j"], label: "Scroll down" }, + { keys: ["k"], label: "Scroll up" }, + { keys: ["t"], label: "Back to top" }, + ], + }, + { + group: "General", + items: [ + { keys: ["?"], label: "Open this cheat sheet" }, + { keys: ["Esc"], label: "Close dialogs / menus" }, + ], + }, + ]; +} export function KeyboardShortcutsDialog() { + const location = useLocation(); + const isAppRoute = location.pathname.startsWith("/app"); + const groups = buildGroups(isAppRoute); const [open, setOpen] = useState(false); const [pending, setPending] = useState(null); @@ -103,7 +121,10 @@ export function KeyboardShortcutsDialog() { return; } - // Two-key "g " sequences + // Two-key "g " sequences — AppShell owns these while an /app/* route is mounted (its own + // "g " handler uses a different destination map), so defer entirely rather than racing + // it with a second, colliding hard navigation (#6811). + if (isAppRoute) return; if (e.key === "g") { setPending("g"); if (timer) clearTimeout(timer); @@ -130,7 +151,7 @@ export function KeyboardShortcutsDialog() { window.removeEventListener("keydown", onKey); if (timer) clearTimeout(timer); }; - }, [pending]); + }, [pending, isAppRoute]); return ( <> @@ -156,13 +177,13 @@ export function KeyboardShortcutsDialog() {
- {GROUPS.length === 0 ? ( + {groups.length === 0 ? ( ) : ( - GROUPS.map((g) => ( + groups.map((g) => (
{g.group}