Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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 <key>" 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<typeof vi.fn>;

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(<KeyboardShortcutsDialog />);

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(<KeyboardShortcutsDialog />);

pressKeys("g", "a");

expect(assign).not.toHaveBeenCalled();
});

it("still hard-navigates on 'g r' outside /app/*", () => {
useLocation.mockReturnValue({ pathname: "/docs" });
render(<KeyboardShortcutsDialog />);

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(<KeyboardShortcutsDialog />);

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(<KeyboardShortcutsDialog />);

fireEvent.click(screen.getByRole("button", { name: "Open keyboard shortcuts" }));

expect(screen.getByText("Go to API reference")).toBeTruthy();
expect(screen.queryByText("Go to runs")).toBeNull();
});
});
99 changes: 60 additions & 39 deletions apps/loopover-ui/src/components/site/keyboard-shortcuts.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useLocation } from "@tanstack/react-router";
import { Keyboard } from "lucide-react";
import { useEffect, useState } from "react";
import { toast } from "sonner";
Expand All @@ -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 <key>" 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<string | null>(null);

Expand Down Expand Up @@ -103,7 +121,10 @@ export function KeyboardShortcutsDialog() {
return;
}

// Two-key "g <x>" sequences
// Two-key "g <x>" sequences — AppShell owns these while an /app/* route is mounted (its own
// "g <key>" 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);
Expand All @@ -130,7 +151,7 @@ export function KeyboardShortcutsDialog() {
window.removeEventListener("keydown", onKey);
if (timer) clearTimeout(timer);
};
}, [pending]);
}, [pending, isAppRoute]);

return (
<>
Expand All @@ -156,13 +177,13 @@ export function KeyboardShortcutsDialog() {
</DialogTitle>
</DialogHeader>
<div className="mt-2 grid gap-5">
{GROUPS.length === 0 ? (
{groups.length === 0 ? (
<EmptyState
title="No shortcuts available"
description="Shortcut metadata did not load. Close this sheet and try opening it again."
/>
) : (
GROUPS.map((g) => (
groups.map((g) => (
<section key={g.group}>
<div className="mb-2 font-mono text-token-2xs uppercase tracking-wider text-muted-foreground">
{g.group}
Expand Down
Loading