diff --git a/apps/ui/src/components/CoachTip.test.tsx b/apps/ui/src/components/CoachTip.test.tsx index b836c44c..1fc92c8a 100644 --- a/apps/ui/src/components/CoachTip.test.tsx +++ b/apps/ui/src/components/CoachTip.test.tsx @@ -104,15 +104,21 @@ describe("CoachTip (RIG-2530)", () => { test("label-only: a command with no keymap row renders the label and no chip", async () => { setPlatform("other"); - // Guard the premise: view.backlog has no keymap row on this base. - expect(shortcutFor(cmd("view.backlog"), "other")).toBeUndefined(); + // Guard the premise: board.openCardCrossLink has no keymap row (it is + // board-nav dispatched, never a global chord — keymap.test.ts pins this). + expect( + shortcutFor(cmd("board.openCardCrossLink"), "other"), + ).toBeUndefined(); const { getByRole, baseElement } = render(() => ( - Backlog + Open cross-link - + )); @@ -121,7 +127,7 @@ describe("CoachTip (RIG-2530)", () => { const tooltip = tooltipOf(baseElement); expect(tooltip).not.toBeNull(); - expect(tooltip?.textContent).toContain("Backlog"); + expect(tooltip?.textContent).toContain("Open cross-link"); expect(tooltip?.querySelector(".cx-palette-shortcut")).toBeNull(); expect(tooltip?.querySelector("kbd")).toBeNull(); }); diff --git a/apps/ui/src/components/Palette.test.tsx b/apps/ui/src/components/Palette.test.tsx index db04868a..45f8aedc 100644 --- a/apps/ui/src/components/Palette.test.tsx +++ b/apps/ui/src/components/Palette.test.tsx @@ -245,6 +245,8 @@ describe("Palette (RIG-2483)", () => { ); const bridge = links.find((b) => b.textContent?.includes("Bridge")); const settings = links.find((b) => b.textContent?.includes("Settings")); + const backlog = links.find((b) => b.textContent?.includes("Backlog")); + const done = links.find((b) => b.textContent?.includes("Done")); // view.bridge → Mod+B, view.settings → Mod+, — aria uses the WAI-ARIA // Control token. The display chord no longer rides a native title (the // RIG-2530 sweep coaches it via CoachTip); a native title would @@ -252,5 +254,13 @@ describe("Palette (RIG-2483)", () => { expect(bridge?.getAttribute("aria-keyshortcuts")).toBe("Control+B"); expect(bridge?.getAttribute("title")).toBeNull(); expect(settings?.getAttribute("aria-keyshortcuts")).toBe("Control+,"); + expect(settings?.getAttribute("title")).toBeNull(); + // view.backlog / view.done are sequence-only (G L / G D): shortcutForAria + // skips the sequence so NO aria-keyshortcuts is emitted, and the RIG-2530 + // sweep moved coaching to a CoachTip, so there is no native title either. + expect(backlog?.getAttribute("aria-keyshortcuts")).toBeNull(); + expect(backlog?.getAttribute("title")).toBeNull(); + expect(done?.getAttribute("aria-keyshortcuts")).toBeNull(); + expect(done?.getAttribute("title")).toBeNull(); }); }); diff --git a/apps/ui/src/components/ShortcutsOverlay.test.tsx b/apps/ui/src/components/ShortcutsOverlay.test.tsx index 8e2c7adc..52e523de 100644 --- a/apps/ui/src/components/ShortcutsOverlay.test.tsx +++ b/apps/ui/src/components/ShortcutsOverlay.test.tsx @@ -63,8 +63,11 @@ describe("ShortcutsOverlay (RIG-2482)", () => { fireEvent.input(input, { target: { value: "bridge" } }); await flush(); const rows = container.querySelectorAll(".cx-shortcuts-row"); - expect(rows.length).toBe(1); - expect(rows[0]?.textContent).toContain("Bridge"); + // "bridge" now matches both Mod+B and the G B leader sequence (RIG-2484). + expect(rows.length).toBe(2); + const text = [...rows].map((r) => r.textContent ?? ""); + expect(text.every((t) => t.includes("Bridge"))).toBe(true); + expect(text.some((t) => t.includes("G then B"))).toBe(true); }); test("a no-match query shows the dim empty row and no rows", async () => { diff --git a/apps/ui/src/keyboard-e2e.test.tsx b/apps/ui/src/keyboard-e2e.test.tsx index ee82dbca..13ebff54 100644 --- a/apps/ui/src/keyboard-e2e.test.tsx +++ b/apps/ui/src/keyboard-e2e.test.tsx @@ -116,6 +116,35 @@ describe("App-root keyboard spine (RIG-2456)", () => { expect(store.view()).toBe("settings"); }); + + test("G then S lands on Settings (leader sequence, real App wiring)", async () => { + setPlatform("other"); + const { store } = mountApp("/"); + expect(store.view()).toBe("bridge"); + + // The `G S` sequence (keymap.ts) resolves through the same tier-3 path as + // `Mod+,` once the T3 runtime arms the leader. Registers nothing — the + // spine already registered view.settings. + press({ key: "g" }); + press({ key: "s" }); + await flush(); + + expect(store.view()).toBe("settings"); + }); + + test("G then L lands on Backlog (sequence-only command, real App wiring)", async () => { + setPlatform("other"); + const { store } = mountApp("/"); + expect(store.view()).toBe("bridge"); + + // view.backlog's ONLY keyboard binding is the `G L` sequence; this proves + // the leader runtime resolves it end to end with no App-specific setup. + press({ key: "g" }); + press({ key: "l" }); + await flush(); + + expect(store.view()).toBe("backlog"); + }); }); describe("shortcuts overlay (RIG-2482)", () => { diff --git a/apps/ui/src/keyboard/dispatch.test.ts b/apps/ui/src/keyboard/dispatch.test.ts index addbcf7f..ad0ca711 100644 --- a/apps/ui/src/keyboard/dispatch.test.ts +++ b/apps/ui/src/keyboard/dispatch.test.ts @@ -1,6 +1,11 @@ -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, jest, test } from "bun:test"; import type { Command, CommandId, CommandScope } from "./commands"; -import { detectPlatform, eventToChord, installKeymap } from "./dispatch"; +import { + detectPlatform, + eventToChord, + installKeymap, + LEADER_TIMEOUT_MS, +} from "./dispatch"; import type { Platform } from "./keymap"; import { createCommandRegistry } from "./registry"; import type { RovingGroupHandle } from "./roving"; @@ -445,3 +450,219 @@ describe("installKeymap", () => { expect(ran).toBe(0); }); }); + +// The leader/mnemonic runtime (RIG-2484 T3): "press G, then " sequences +// armed inside the ONE keydown handler, with a timeout, the editable guard +// ahead of arming, and dead-sequence fall-through to single-chord resolution. +describe("installKeymap — leader sequences", () => { + let uninstall: (() => void) | null = null; + + afterEach(() => { + uninstall?.(); + uninstall = null; + jest.useRealTimers(); + setPlatform("other"); + }); + + test("g then b runs view.bridge (G B); the arming g is defaultPrevented", () => { + const registry = createCommandRegistry(); + let ran = 0; + registry.register(makeCommand("view.bridge", () => ran++)); + uninstall = installKeymap(registry, () => null); + + const armed = keydown({ key: "g" }); + expect(armed.defaultPrevented).toBe(true); + expect(ran).toBe(0); + + keydown({ key: "b" }); + expect(ran).toBe(1); + // The completion disarmed the leader: a second bare b is a no-op, not a + // stuck-pending double-complete. + keydown({ key: "b" }); + expect(ran).toBe(1); + }); + + test("timeout: after LEADER_TIMEOUT_MS the leader disarms, so b does not complete", () => { + jest.useFakeTimers(); + const registry = createCommandRegistry(); + let ran = 0; + registry.register(makeCommand("view.bridge", () => ran++)); + uninstall = installKeymap(registry, () => null); + + keydown({ key: "g" }); + jest.advanceTimersByTime(LEADER_TIMEOUT_MS + 1); + keydown({ key: "b" }); + + expect(ran).toBe(0); + }); + + test("editable-guard: g then b in an input arms nothing, runs nothing, prevents nothing", () => { + const registry = createCommandRegistry(); + let ran = 0; + registry.register(makeCommand("view.bridge", () => ran++)); + uninstall = installKeymap(registry, () => null); + + const input = document.createElement("input"); + document.body.appendChild(input); + const armed = keydown({ key: "g" }, input); + const completed = keydown({ key: "b" }, input); + + expect(armed.defaultPrevented).toBe(false); + expect(completed.defaultPrevented).toBe(false); + expect(ran).toBe(0); + input.remove(); + }); + + test("` typeahead, or an ARIA combobox/listbox/menu widget). A bare letter + * on any of these must reach the widget, never arm a leader or fire a chord — + * arming unconditionally `preventDefault`s (§A3 step 4), which would otherwise + * steal a ` / ARIA widget must type, + // never arm, complete, or fire. Mod/Ctrl/Alt chords are global and stay + // unguarded (and, via the completion fall-through below, still disarm). + const hasCommandModifier = event.metaKey || event.ctrlKey || event.altKey; + if (!hasCommandModifier && isEditableTarget(event.target)) return; + + // Step 3 — completion. A leader is pending. + if (pending) { + // A pure-modifier keydown (holding Shift/Control/Alt/Meta mid-sequence + // is human) neither completes nor disarms. + if ( + event.key === "Shift" || + event.key === "Control" || + event.key === "Alt" || + event.key === "Meta" + ) { + return; + } + // Escape disarms and is consumed. + if (event.key === "Escape") { + disarm(); + event.preventDefault(); + return; + } + // Otherwise resolve the two-segment chord " " + // where the completion segment is the FULL normalized chord. Disarm + // first (the sequence is spent either way). + const { leader } = pending; + disarm(); + const sequence = `${leader} ${chord}`; + const seqMatching = DEFAULT_KEYMAP.filter( + (entry) => resolveChord(entry.chord, platform) === sequence, + ); + if (seqMatching.length > 0) { + resolve(seqMatching, event); + return; + } + // No row matches: fall through and process this key as a plain single + // chord in the same keydown, RE-ENTERING the arming step first (so a + // repeated leader re-arms and any other key resolves on its own). + } + + // Step 4 — arming. No leader pending (or the sequence fell through): if + // the normalized chord is a table-derived leader prefix (which, per the + // A2 authoring rule, has no single-chord row of its own) and the key is + // not an auto-repeat, arm and consume. Modifier chords never arm; the + // editable guard already returned for interactive targets. + if (!hasCommandModifier && !event.repeat && leaders.has(chord)) { + pending = { + leader: chord, + timer: setTimeout(disarm, LEADER_TIMEOUT_MS) as unknown as number, + }; + event.preventDefault(); + event.stopPropagation(); + return; + } + + // Step 5 — single-chord path (unchanged): the empty-matching return and + // the three tiers. + const matching = DEFAULT_KEYMAP.filter( + (entry) => resolveChord(entry.chord, platform) === chord, + ); + if (matching.length === 0) return; + resolve(matching, event); + }; + window.addEventListener("keydown", handler); - return () => window.removeEventListener("keydown", handler); + return () => { + window.removeEventListener("keydown", handler); + // Clear any live disarm timer so an uninstall (test teardown / HMR) never + // leaks a timer that fires against a torn-down closure. + disarm(); + }; } diff --git a/apps/ui/src/keyboard/keymap.test.ts b/apps/ui/src/keyboard/keymap.test.ts index 63c0d83a..9ba35150 100644 --- a/apps/ui/src/keyboard/keymap.test.ts +++ b/apps/ui/src/keyboard/keymap.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test"; import type { CommandId } from "./commands"; import { chordSegments, + DEFAULT_KEYMAP, formatChordForDisplay, type KeymapEntry, leaderPrefixes, @@ -37,10 +38,19 @@ describe("shortcutFor", () => { test("undefined for a command with no keymap row (miss)", () => { expect(shortcutFor(id("board.openCardCrossLink"), "other")).toBeUndefined(); - expect(shortcutFor(id("view.backlog"), "other")).toBeUndefined(); expect(shortcutFor(id("nonexistent.command"), "other")).toBeUndefined(); }); + test("a sequence-only command renders its formatted sequence chord", () => { + // view.backlog's only keymap row is the G L sequence (T2, RIG-2484). + expect(shortcutFor(id("view.backlog"), "other")).toBe("G then L"); + }); + + test("a dual-bound command shows its modifier chord (sequence row is later)", () => { + // view.bridge is Mod+B (first) then G B; the modifier row wins. + expect(shortcutFor(id("view.bridge"), "other")).toBe("Ctrl+B"); + }); + test("returns the FIRST matching row for an id bound more than once", () => { // Enter is bound to list.openOrSelect (unscoped) AND comms.send (when:main); // shortcutFor takes the first DEFAULT_KEYMAP row — list.openOrSelect's. @@ -92,6 +102,26 @@ describe("leaderPrefixes", () => { ]; expect(leaderPrefixes(single, "other").size).toBe(0); }); + + test("resolves the leader segment per-platform (a Mod leader → Cmd/Ctrl)", () => { + const modLeader: readonly KeymapEntry[] = [ + { chord: "Mod+X Y", commandId: id("view.bridge") }, + ]; + expect([...leaderPrefixes(modLeader, "mac")]).toEqual(["Cmd+X"]); + expect([...leaderPrefixes(modLeader, "other")]).toEqual(["Ctrl+X"]); + }); + + test("accumulates distinct leaders and dedups rows sharing one leader", () => { + const twoLeaders: readonly KeymapEntry[] = [ + { chord: "G B", commandId: id("view.bridge") }, + { chord: "G L", commandId: id("view.backlog") }, + { chord: "Space X", commandId: id("view.done") }, + ]; + expect([...leaderPrefixes(twoLeaders, "other")].sort()).toEqual([ + "G", + "Space", + ]); + }); }); describe("formatChordForDisplay", () => { @@ -105,3 +135,36 @@ describe("formatChordForDisplay", () => { expect(formatChordForDisplay("G L", "other")).toBe("G then L"); }); }); + +// DEFAULT_KEYMAP authoring invariants for leader sequences (RIG-2484 §A2). +describe("DEFAULT_KEYMAP sequence authoring invariants", () => { + const MODIFIER = /(?:^|\+)(?:Mod|Shift|Alt|Ctrl|Cmd|Meta)(?:\+|$)/; + const sequenceRows = DEFAULT_KEYMAP.filter( + (e) => chordSegments(e.chord).length > 1, + ); + const singleChords = new Set( + DEFAULT_KEYMAP.filter((e) => chordSegments(e.chord).length === 1).map( + (e) => e.chord, + ), + ); + + test("every sequence is exactly two segments", () => { + for (const entry of sequenceRows) { + expect(chordSegments(entry.chord).length).toBe(2); + } + }); + + test("every segment of a sequence is modifier-less", () => { + for (const entry of sequenceRows) { + for (const segment of chordSegments(entry.chord)) { + expect(MODIFIER.test(segment)).toBe(false); + } + } + }); + + test("a sequence's first segment is not also a complete single chord", () => { + for (const entry of sequenceRows) { + expect(singleChords.has(chordSegments(entry.chord)[0])).toBe(false); + } + }); +}); diff --git a/apps/ui/src/keyboard/keymap.ts b/apps/ui/src/keyboard/keymap.ts index 4d5bfb41..8206b65d 100644 --- a/apps/ui/src/keyboard/keymap.ts +++ b/apps/ui/src/keyboard/keymap.ts @@ -110,7 +110,7 @@ export function shortcutFor( platform: Platform, ): string | undefined { const entry = DEFAULT_KEYMAP.find((e) => e.commandId === id); - return entry ? resolveChord(entry.chord, platform) : undefined; + return entry ? formatChordForDisplay(entry.chord, platform) : undefined; } /** @@ -122,7 +122,9 @@ export function shortcutForAria( id: CommandId, platform: Platform, ): string | undefined { - const entry = DEFAULT_KEYMAP.find((e) => e.commandId === id); + const entry = DEFAULT_KEYMAP.find( + (e) => e.commandId === id && chordSegments(e.chord).length === 1, + ); return entry ? resolveChordAria(entry.chord, platform) : undefined; } @@ -171,6 +173,14 @@ export const DEFAULT_KEYMAP: readonly KeymapEntry[] = [ // non-ASCII-letter keys, so a US `Shift+/` normalizes to `?`. { chord: "?", commandId: cmd("view.shortcuts") }, + // Go-to sequences (RIG-2484). Leader "G then " destinations, all + // unscoped/global; each sits AFTER any existing modifier row for the same + // command so shortcutFor/shortcutForAria resolve the modifier chord first. + { chord: "G B", commandId: cmd("view.bridge") }, + { chord: "G L", commandId: cmd("view.backlog") }, + { chord: "G D", commandId: cmd("view.done") }, + { chord: "G S", commandId: cmd("view.settings") }, + // Zones (D5:448-449) { chord: "Mod+1", commandId: cmd("zone.focusLeft") }, { chord: "Mod+2", commandId: cmd("zone.focusMain") }, diff --git a/apps/ui/src/keyboard/shortcuts-model.ts b/apps/ui/src/keyboard/shortcuts-model.ts index c1b703d3..c7c5b12b 100644 --- a/apps/ui/src/keyboard/shortcuts-model.ts +++ b/apps/ui/src/keyboard/shortcuts-model.ts @@ -13,16 +13,21 @@ * iterated — the keymap is the row source). * - Group by the resolved COMMAND's `scope` (NOT the keymap `when` field), * ordered `global, left, main, right, topbar`; keymap order within a group. - * - Render chords via `resolveChord(entry.chord, platform)`. + * - Render chords via `formatChordForDisplay(entry.chord, platform)` (a leader + * sequence renders `"G then B"`; a single chord resolves as before). * - Substring filter (case-insensitive over the lowercased title, each - * keyword, and the resolved chord); an empty query passes every row. + * keyword, and the formatted chord); an empty query passes every row. */ import type { CommandId, CommandRegistry, CommandScope } from "./commands"; -import { type KeymapEntry, type Platform, resolveChord } from "./keymap"; +import { + formatChordForDisplay, + type KeymapEntry, + type Platform, +} from "./keymap"; export interface ShortcutRow { - readonly chord: string; // platform-resolved via resolveChord + readonly chord: string; // platform-resolved + display-formatted via formatChordForDisplay readonly title: string; // command.title readonly commandId: CommandId; } @@ -54,7 +59,7 @@ export function buildShortcutGroups( const command = registry.get(entry.commandId); if (!command) continue; // unregistered → dead chord, omit - const chord = resolveChord(entry.chord, platform); + const chord = formatChordForDisplay(entry.chord, platform); if (needle && !matches(command.title, command.keywords, chord, needle)) { continue; }