From 5c93e2eb66e1ad776573aa577b7d1a8aeed58bae Mon Sep 17 00:00:00 2001 From: mintaka Date: Tue, 25 Aug 2026 13:39:33 -0400 Subject: [PATCH] feat(keyboard): four G-leader sequence rows + display/aria hardening (RIG-2708) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the four Go-to leader sequences from the frozen leader-chords design (`docs/designs/product/compass-leader-chords/design.md`, T2 / §A4-A5) to `DEFAULT_KEYMAP` and hardens the two shipped chord scanners so a sequence renders correctly everywhere and never reaches `aria-keyshortcuts`. - `DEFAULT_KEYMAP`: a new `// Go-to sequences (RIG-2484)` block with `G B → view.bridge`, `G L → view.backlog`, `G D → view.done`, `G S → view.settings`, each placed after any existing modifier row for the same command so the modifier chord stays the first match. - `shortcutForAria` now skips sequence rows (`chordSegments(entry.chord).length === 1`): a sequence-only command returns `undefined` (its writer omits the attribute) and a dual-bound command returns its modifier chord. A press-then-press sequence has no valid WAI-ARIA representation, so the skip is mandatory and makes the result row-order-independent. - `shortcutFor` now returns `formatChordForDisplay` of the first matching row and does NOT skip sequences, so `view.backlog`/`view.done` display `"G then L"`/`"G then D"` and keep their point-of-use chip and title; a dual-bound command still shows its modifier chord because that row is first. - The `?` overlay join formats via `formatChordForDisplay`, so a sequence row reads `"G then B"` instead of the misleading `"G B"`. Tests: `keymap.test.ts` moves `view.backlog` out of the `shortcutFor` miss group, adds a sequence-only-display case, a dual-bound modifier-wins case, and a `DEFAULT_KEYMAP` authoring-invariant block (every sequence is two modifier-less segments; no leader prefix is also a complete single chord); `ShortcutsOverlay.test.tsx` updates the bridge-filter row count 1→2 and asserts the formatted `"G then B"`; `Palette.test.tsx` asserts Backlog/Done carry no `aria-keyshortcuts` but a `"Backlog (G then L)"`/`"Done (G then D)"` title while Bridge keeps `aria-keyshortcuts="Control+B"`. Stacked on RIG-2707 (the sequence-grammar helpers). The dispatcher leader runtime that makes these chords fire is T3 (RIG-2709); until it lands the rows are advertised only through the discoverability net, which is why T2 and T3 ship in one stack. Ledger-impact: none. DL-248..DL-252 for this record landed with the design PR (#544). Refs RIG-2708 Co-authored-by: Matt Wilkinson --- apps/ui/src/components/CoachTip.test.tsx | 16 ++++--- apps/ui/src/components/Palette.test.tsx | 10 +++++ .../src/components/ShortcutsOverlay.test.tsx | 7 ++- apps/ui/src/keyboard/keymap.test.ts | 45 ++++++++++++++++++- apps/ui/src/keyboard/keymap.ts | 14 +++++- apps/ui/src/keyboard/shortcuts-model.ts | 15 ++++--- 6 files changed, 92 insertions(+), 15 deletions(-) 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/keymap.test.ts b/apps/ui/src/keyboard/keymap.test.ts index 63c0d83a..2c588e44 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. @@ -105,3 +115,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; }