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("` / ARIA + * combobox/listbox/menu types, never arms or fires) → completion (when a leader + * is armed: a pure-modifier key is ignored, Escape disarms and is consumed, else + * the leader disarms and the two-segment chord `" "` resolves; a + * sequence that matches no row falls through and re-enters arming in the same + * keydown) → arming (a table-derived leader prefix with no command modifier and + * not an auto-repeat arms and is consumed, starting a `LEADER_TIMEOUT_MS` disarm + * timer) → single-chord path. A completed sequence and a single chord run the + * SAME three-tier resolution. Leaders are derived from the keymap + * (`leaderPrefixes`), never hard-coded; the uninstaller clears any live timer. */ import type { CommandId, CommandRegistry } from "./commands"; -import { DEFAULT_KEYMAP, type Platform, resolveChord } from "./keymap"; +import { + DEFAULT_KEYMAP, + type KeymapEntry, + leaderPrefixes, + type Platform, + resolveChord, +} from "./keymap"; import type { RovingGroupHandle } from "./roving"; import type { FocusZone } from "./zones"; @@ -62,12 +81,22 @@ function isGroupRelative(id: CommandId): boolean { return id.startsWith("list.") || id.startsWith("board."); } -/** Whether an event target owns its own text keys (comms composer, etc.). */ +/** + * Whether an event target owns its own text keys (comms composer, native + * ``'s typeahead or the palette listbox's search keys (§A1). + */ function isEditableTarget(target: EventTarget | null): boolean { return ( target instanceof HTMLInputElement || target instanceof HTMLTextAreaElement || - (target instanceof HTMLElement && target.isContentEditable) + target instanceof HTMLSelectElement || + (target instanceof HTMLElement && + (target.isContentEditable || + target.closest('[role="combobox"],[role="listbox"],[role="menu"]') !== + null)) ); } @@ -84,6 +113,21 @@ export function detectPlatform(): Platform { : "other"; } +/** + * A pending leader sequence: the resolved first-segment chord (`"G"`) plus the + * live disarm timer handle. `null` when no sequence is armed. Per-install + * closure state — no module-level global (matches App.tsx's install/onCleanup + * discipline). + */ +type PendingLeader = { leader: string; timer: number } | null; + +/** + * How long an armed leader waits for its completion key before self-disarming + * (§A3, OQ1 ratified at 1000 ms). Exported so the runtime and its tests share + * the one value. + */ +export const LEADER_TIMEOUT_MS = 1000; + /** * Install the global keymap. Adds one `keydown` listener and returns the * uninstaller (removes exactly that listener). `active` yields the focused @@ -106,21 +150,28 @@ export function installKeymap( activeZone: () => FocusZone | null = () => null, ): () => void { const platform: Platform = detectPlatform(); + // The leader-prefix set is table-derived and platform-fixed, so compute it + // once at install rather than per keydown. + const leaders = leaderPrefixes(DEFAULT_KEYMAP, platform); - const handler = (event: KeyboardEvent): void => { - const chord = eventToChord(event, platform); - const matching = DEFAULT_KEYMAP.filter( - (entry) => resolveChord(entry.chord, platform) === chord, - ); - if (matching.length === 0) return; - - // Editable-target guard: a modifier-less chord (arrows, Enter, Space, - // Home/End, and bare Shift combos) never fires while focus is in a text - // field — the composer keeps its local keys. Mod/Ctrl/Alt chords are - // global and are NOT guarded. - const hasCommandModifier = event.metaKey || event.ctrlKey || event.altKey; - if (!hasCommandModifier && isEditableTarget(event.target)) return; + // Per-install pending-leader state (§A3). `null` unless a leader is armed. + let pending: PendingLeader = null; + const disarm = (): void => { + if (pending) { + clearTimeout(pending.timer); + pending = null; + } + }; + // Resolve a set of matching rows through the ratified three tiers (active + // group → scoped → global). Factored so a single chord and a completed + // leader sequence run byte-identical resolution — the completion path passes + // the sequence-matched rows, the single-chord path passes the single-chord + // rows, and both flow through here. + const resolve = ( + matching: readonly KeymapEntry[], + event: KeyboardEvent, + ): void => { // Tier 1 — active group. Route a group-relative chord to the group; a // `true` return handles it (and suppresses native activation), a `false` // (declines) falls through to the next tier. @@ -167,6 +218,83 @@ export function installKeymap( } }; + const handler = (event: KeyboardEvent): void => { + // Step 1 — normalize. + const chord = eventToChord(event, platform); + + // Step 2 — editable-target guard FIRST, before any leader logic and + // before the empty-matching return, but only for modifier-less keys. A + // bare key in a text field / native