From 24df2611f07fccd8744516aa6129995bb3b9421c Mon Sep 17 00:00:00 2001 From: mintaka Date: Tue, 25 Aug 2026 13:32:37 -0400 Subject: [PATCH 1/2] feat(keyboard): sequence-grammar chord helpers for leader chords (RIG-2707) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds three pure, table-independent helpers to `keymap.ts` for the leader/mnemonic-chord grammar frozen in `docs/designs/product/compass-leader-chords/design.md` (T1): - `chordSegments(chord)` — split a chord on its single space; a plain chord yields a one-element array, a sequence like `"G B"` yields `["G", "B"]`. Space is a collision-free separator because the literal Space key normalizes to the `"Space"` token in the dispatcher. - `leaderPrefixes(keymap, platform)` — the resolved first segment of every multi-segment row, derived from the table so the dispatcher never hard-codes a leader key. - `formatChordForDisplay(chord, platform)` — a single chord resolves through `resolveChord` (`"Mod+B"` → `"Cmd+B"`); a sequence joins resolved segments with `" then "` (`"G B"` → `"G then B"`). The single formatter behind every display surface. Extends the `KeymapEntry` doc block with the sequence grammar and its three authoring rules (exactly two segments; every segment modifier-less; the leader prefix never doubles as a complete chord). Pure additions: no production caller and no shipped-helper change (`shortcutFor`/`shortcutForAria` hardening and the `DEFAULT_KEYMAP` sequence rows are T2). `formatChordForDisplay` is the shared root — RIG-2484 T2/T3 and RIG-2530's CoachTip (RIG-2703) both consume it. Tests extend `keymap.test.ts` over a fixture keymap (the real table carries no sequence rows until T2): segment splitting, leader-prefix derivation, and single-vs-sequence display formatting on both platforms. Ledger-impact: none. DL-248..DL-252 for this record landed with the design PR (#544); this impl slice ratifies no new decision. Refs RIG-2707 Co-authored-by: Matt Wilkinson --- apps/ui/src/keyboard/keymap.test.ts | 55 ++++++++++++++++++++++++- apps/ui/src/keyboard/keymap.ts | 63 +++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/apps/ui/src/keyboard/keymap.test.ts b/apps/ui/src/keyboard/keymap.test.ts index 5a472d3f..63c0d83a 100644 --- a/apps/ui/src/keyboard/keymap.test.ts +++ b/apps/ui/src/keyboard/keymap.test.ts @@ -1,6 +1,13 @@ import { describe, expect, test } from "bun:test"; import type { CommandId } from "./commands"; -import { shortcutFor, shortcutForAria } from "./keymap"; +import { + chordSegments, + formatChordForDisplay, + type KeymapEntry, + leaderPrefixes, + shortcutFor, + shortcutForAria, +} from "./keymap"; // shortcutFor (RIG-2483, A5/D4) — the single derivation for every shortcut chip: // the first DEFAULT_KEYMAP row bound to an id, resolveChord-resolved. Pure @@ -52,3 +59,49 @@ describe("shortcutForAria", () => { expect(shortcutForAria(id("nonexistent.command"), "other")).toBeUndefined(); }); }); + +// Sequence-grammar helpers (RIG-2484 T1) — pure, table-independent. Tested over +// a FIXTURE keymap because DEFAULT_KEYMAP carries no sequence rows until T2. + +const seqFixture: readonly KeymapEntry[] = [ + { chord: "Mod+B", commandId: id("view.bridge") }, + { chord: "G B", commandId: id("view.bridge") }, + { chord: "G L", commandId: id("view.backlog") }, +]; + +describe("chordSegments", () => { + test("splits a sequence on its single space", () => { + expect(chordSegments("G B")).toEqual(["G", "B"]); + }); + + test("a plain chord yields a one-element array", () => { + expect(chordSegments("Mod+B")).toEqual(["Mod+B"]); + expect(chordSegments("Shift+Enter")).toEqual(["Shift+Enter"]); + }); +}); + +describe("leaderPrefixes", () => { + test("collects the resolved first segment of every sequence row, and nothing else", () => { + const prefixes = leaderPrefixes(seqFixture, "other"); + expect([...prefixes]).toEqual(["G"]); + }); + + test("empty for a table with no sequence rows", () => { + const single: readonly KeymapEntry[] = [ + { chord: "Mod+B", commandId: id("view.bridge") }, + ]; + expect(leaderPrefixes(single, "other").size).toBe(0); + }); +}); + +describe("formatChordForDisplay", () => { + test("a single chord resolves platform-specifically (Mod→Cmd/Ctrl)", () => { + expect(formatChordForDisplay("Mod+B", "mac")).toBe("Cmd+B"); + expect(formatChordForDisplay("Mod+B", "other")).toBe("Ctrl+B"); + }); + + test("a sequence joins resolved segments with ' then '", () => { + expect(formatChordForDisplay("G B", "mac")).toBe("G then B"); + expect(formatChordForDisplay("G L", "other")).toBe("G then L"); + }); +}); diff --git a/apps/ui/src/keyboard/keymap.ts b/apps/ui/src/keyboard/keymap.ts index 91861949..4d5bfb41 100644 --- a/apps/ui/src/keyboard/keymap.ts +++ b/apps/ui/src/keyboard/keymap.ts @@ -47,6 +47,57 @@ export const resolveChord = (chord: string, platform: Platform): string => export const resolveChordAria = (chord: string, platform: Platform): string => chord.replaceAll(MOD, platform === "mac" ? "Meta" : "Control"); +/** + * Split a chord string into its sequence segments on a single space. A plain + * (single-press) chord yields a one-element array; a leader sequence like + * `"G B"` yields `["G", "B"]`. Space is unambiguous as the separator because + * the literal Space key normalizes to the multi-char token `"Space"` + * (`dispatch.ts`), so a raw `" "` never appears as a key name inside a chord. + */ +export function chordSegments(chord: string): string[] { + return chord.split(" "); +} + +/** + * The set of resolved leader keys for `platform`: the `resolveChord`-resolved + * FIRST segment of every multi-segment (sequence) row in `keymap`. Derived from + * the table so the dispatcher never hard-codes a leader key — adding a second + * leader later is a data change, not a runtime change. A single-chord row + * contributes nothing. + */ +export function leaderPrefixes( + keymap: readonly KeymapEntry[], + platform: Platform, +): ReadonlySet { + const prefixes = new Set(); + for (const entry of keymap) { + const segments = chordSegments(entry.chord); + if (segments.length > 1) { + prefixes.add(resolveChord(segments[0], platform)); + } + } + return prefixes; +} + +/** + * The display form of a chord for `platform`. A single chord resolves through + * `resolveChord` (`"Mod+B"` → `"Cmd+B"`); a leader sequence resolves each + * segment and joins them with `" then "` (`"G B"` → `"G then B"`), making + * press order explicit where a bare `"G B"` would read as one simultaneous + * chord. The single formatter behind every display surface (chips, titles, + * the shortcuts overlay). + */ +export function formatChordForDisplay( + chord: string, + platform: Platform, +): string { + const segments = chordSegments(chord); + if (segments.length === 1) return resolveChord(chord, platform); + return segments + .map((segment) => resolveChord(segment, platform)) + .join(" then "); +} + /** * The display chord for a command: the FIRST `DEFAULT_KEYMAP` row bound to `id`, * `resolveChord`-resolved for `platform` (Mod→Cmd/Ctrl). `undefined` when no row @@ -87,6 +138,18 @@ export function shortcutForAria( * scoped entry takes precedence while its zone is active (D5's ranking rule: * "scoped commands rank above global ones when their scope is active"); the * consumer applies that precedence rather than double-firing. + * + * A `chord` may be a LEADER SEQUENCE: two segments separated by one space + * (`"G B"` = press `G` then `B`), resolved for display by + * `formatChordForDisplay` (`"G then B"`) and split by `chordSegments`. The + * dispatcher's leader runtime resolves the completed sequence through the same + * tiers as a single chord. Authoring rules (enforced by a `DEFAULT_KEYMAP` + * invariant test once the first sequence rows land): a sequence is exactly two + * segments; every segment is + * modifier-less (so it inherits the editable-target guard — a modified segment + * would fire while a text field is focused); and a sequence's first segment + * (the leader) must not also be bound as a complete single chord (the leader + * key is reserved, which keeps the runtime's fall-through simple). */ export interface KeymapEntry { readonly chord: string; From c91633fbd7d9fa12e9c14a6905e8e17945c9d859 Mon Sep 17 00:00:00 2001 From: mintaka Date: Tue, 25 Aug 2026 13:42:44 -0400 Subject: [PATCH 2/2] feat(ui): CoachTip coaching tooltip component (RIG-2703) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the reusable coaching Tooltip from the frozen coaching-tooltips design (`docs/designs/product/compass-coaching-tooltips/design.md`, T1 / §A1-A3, A5, D5): a label + keymap-resolved chord that reveals on hover AND focus, in the shipped `.cx-tooltip` box. This is the component only; the adoption sweep across command-backed chrome is T2 (RIG-2704). - `apps/ui/src/components/CoachTip.tsx` — three exports mirroring Kobalte's own anatomy: `COACH_TIP_DELAY_MS = 400` (mirrors the `--cx-tooltip-delay` token, `tokens.css:227`); `CoachTip`, the Kobalte v2-alpha `Tooltip` root with `openDelay` defaulted to that constant and hover+focus reveal (Kobalte's default, `triggerOnFocusOnly` unset); `CoachTipTrigger`, the re-exported polymorphic `Tooltip.Trigger` so call sites author their own element; and `CoachTipContent`, the `Tooltip.Portal` + `Tooltip.Content(class="cx-tooltip")` rendering the label and, right of it, the chord. - Chord is always the keymap-resolved display string — `props.chord ?? shortcutFor(props.command, detectPlatform())`, never hand-authored (DL-234's single-derivation rule). `undefined` → label-only; a `" then "` leader sequence → plain text in the chip's typographic style; any other chord → ``. Because the component only reads `shortcutFor`'s output, it shows `Ctrl+B`/`Cmd+B` today and `G then B` automatically once the leader-chord rows land — no merge-order dependency. Props are never destructured (Solid v2 reactivity). - `apps/ui/src/design/components/tooltip.css` — the box (`.cx-tooltip`, whose first consumer this is) becomes a flex row and gains a `.cx-tooltip-label` sub-part, so the label takes free space and the reused `.cx-palette-shortcut` chord chip right-aligns via its own `margin-left: auto`. Tests (`CoachTip.test.tsx`, `@solidjs/testing-library`): label + chord derived from `shortcutFor` (never a hand-authored expectation); the ARIA wiring Kobalte owns (`role="tooltip"` + `aria-describedby`); focus reveal with no pointer event; the label-only path for a command with no keymap row; and the sequence-aware branch, whose `"G then B"` fixture is derived from `formatChordForDisplay("G B", "other")` so a change to the shared format contract surfaces here as a failing test rather than a giant ``. Stacked on RIG-2707 (the `formatChordForDisplay` sequence-grammar root the sequence branch keys off). Ledger-impact: none. DL-245..247 for this record landed with the design PR (#569); this impl slice ratifies no new decision. Refs RIG-2703 Co-authored-by: Matt Wilkinson --- apps/ui/src/components/CoachTip.test.tsx | 166 ++++++++++++++++++++++ apps/ui/src/components/CoachTip.tsx | 71 +++++++++ apps/ui/src/design/components/tooltip.css | 14 +- 3 files changed, 249 insertions(+), 2 deletions(-) create mode 100644 apps/ui/src/components/CoachTip.test.tsx create mode 100644 apps/ui/src/components/CoachTip.tsx diff --git a/apps/ui/src/components/CoachTip.test.tsx b/apps/ui/src/components/CoachTip.test.tsx new file mode 100644 index 00000000..b836c44c --- /dev/null +++ b/apps/ui/src/components/CoachTip.test.tsx @@ -0,0 +1,166 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { cleanup, render } from "@solidjs/testing-library"; +import type { CommandId } from "../keyboard/commands"; +import { formatChordForDisplay, shortcutFor } from "../keyboard/keymap"; +import { CoachTip, CoachTipContent, CoachTipTrigger } from "./CoachTip"; + +// CoachTip's rendered contract (RIG-2530): a Kobalte Tooltip whose content is a +// control's label + its keymap-resolved chord. Defends: chord derivation via +// shortcutFor (never hand-authored), the ARIA tooltip wiring the primitive +// owns (role="tooltip" + aria-describedby), focus reveal, the label-only path +// when no keymap row exists, and the sequence-aware branch that keeps a leader +// sequence out of ShortcutChip's "+"-split. + +function setPlatform(platform: "mac" | "other"): void { + Object.defineProperty(navigator, "platform", { + value: platform === "mac" ? "MacIntel" : "Linux x86_64", + configurable: true, + }); +} + +const cmd = (id: string) => id as CommandId; + +// Kobalte mounts the portalled content through createPresence on a macrotask, +// so a focus that opens the tooltip is observable only after one setTimeout(0). +async function settle(): Promise { + const { promise, resolve } = Promise.withResolvers(); + setTimeout(resolve, 0); + await promise; +} + +const tooltipOf = (root: HTMLElement) => + root.querySelector('[role="tooltip"]'); + +afterEach(() => { + cleanup(); + setPlatform("other"); +}); + +describe("CoachTip (RIG-2530)", () => { + test("label + chord: view.bridge on other shows the label and a Ctrl+B chip derived from the keymap", async () => { + setPlatform("other"); + const { getByRole, baseElement } = render(() => ( + + + Bridge + + + + )); + + getByRole("button").focus(); + await settle(); + + const tooltip = tooltipOf(baseElement); + expect(tooltip).not.toBeNull(); + expect(tooltip?.textContent).toContain("Bridge"); + + const chip = tooltip?.querySelector(".cx-palette-shortcut"); + expect(chip).not.toBeNull(); + const kbds = Array.from(chip?.querySelectorAll("kbd") ?? []).map( + (k) => k.textContent, + ); + // Grounded in DEFAULT_KEYMAP via shortcutFor — never a hand-authored string. + expect(shortcutFor(cmd("view.bridge"), "other")).toBe("Ctrl+B"); + expect(kbds).toEqual(["Ctrl", "B"]); + }); + + test("aria wiring: focusing the trigger opens the tooltip and links aria-describedby to the content id", async () => { + setPlatform("other"); + const { getByRole, baseElement } = render(() => ( + + + Bridge + + + + )); + + const trigger = getByRole("button"); + trigger.focus(); + await settle(); + + const tooltip = tooltipOf(baseElement); + expect(tooltip).not.toBeNull(); + expect(tooltip?.id).toBeTruthy(); + expect(trigger.getAttribute("aria-describedby")).toBe(tooltip?.id ?? ""); + }); + + test("focus reveal: the tooltip opens on trigger focus with no pointer event", async () => { + const { getByRole, baseElement } = render(() => ( + + + Bridge + + + + )); + + expect(tooltipOf(baseElement)).toBeNull(); + getByRole("button").focus(); + await settle(); + expect(tooltipOf(baseElement)).not.toBeNull(); + }); + + 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(); + + const { getByRole, baseElement } = render(() => ( + + + Backlog + + + + )); + + getByRole("button").focus(); + await settle(); + + const tooltip = tooltipOf(baseElement); + expect(tooltip).not.toBeNull(); + expect(tooltip?.textContent).toContain("Backlog"); + expect(tooltip?.querySelector(".cx-palette-shortcut")).toBeNull(); + expect(tooltip?.querySelector("kbd")).toBeNull(); + }); + + test("sequence handling: a 'then'-sequence chord renders plain text (no kbd split), a '+'-chord renders the kbd chip", async () => { + // The sequence fixture is #544's formatChordForDisplay output (DL-251), + // so a format change surfaces here rather than silently mis-rendering. + const sequence = formatChordForDisplay("G B", "other"); + expect(sequence).toBe("G then B"); + + const seq = render(() => ( + + + Go + + + + )); + seq.getByRole("button").focus(); + await settle(); + const seqTip = tooltipOf(seq.baseElement); + expect(seqTip?.textContent).toContain(sequence); + expect(seqTip?.querySelector("kbd")).toBeNull(); + cleanup(); + + const plain = render(() => ( + + + Bridge + + + + )); + plain.getByRole("button").focus(); + await settle(); + const plainTip = tooltipOf(plain.baseElement); + const kbds = Array.from(plainTip?.querySelectorAll("kbd") ?? []).map( + (k) => k.textContent, + ); + expect(kbds).toEqual(["Ctrl", "B"]); + }); +}); diff --git a/apps/ui/src/components/CoachTip.tsx b/apps/ui/src/components/CoachTip.tsx new file mode 100644 index 00000000..660bd23f --- /dev/null +++ b/apps/ui/src/components/CoachTip.tsx @@ -0,0 +1,71 @@ +// Coaching tooltip (RIG-2530) — the reusable label+chord Tooltip adopted across +// command-backed chrome. Built on the Kobalte v2-alpha `Tooltip` primitive +// (a11y-hard behavior: hover+focus reveal, open-delay timing, Escape dismiss, +// aria-describedby wiring — DL-150), styled by the shipped `.cx-tooltip` box. +// The chord is ALWAYS the keymap-resolved display string (via `shortcutFor`), +// never hand-authored (DL-234's single-derivation rule). Sequence-aware: a +// leader sequence ("G then B") renders as plain text in the chip's style, since +// ShortcutChip splits on "+" and would otherwise emit one giant (A3). + +import { Tooltip } from "@kobalte/core/tooltip"; +import type { Component, ParentProps } from "solid-js"; +import { Show } from "solid-js"; +import "../design/components/tooltip.css"; +import type { CommandId } from "../keyboard/commands"; +import { detectPlatform } from "../keyboard/dispatch"; +import { shortcutFor } from "../keyboard/keymap"; +import { ShortcutChip } from "./ShortcutChip"; + +/** House open delay, mirrors --cx-tooltip-delay (tokens.css:227). */ +export const COACH_TIP_DELAY_MS = 400; + +/** Kobalte Tooltip root with openDelay defaulted to COACH_TIP_DELAY_MS; + * hover+focus reveal is Kobalte's default (triggerOnFocusOnly stays unset). */ +export const CoachTip: Component> = ( + props, +) => ( + + {props.children} + +); + +/** The trigger — Kobalte's polymorphic Trigger, re-exported so call sites author + * with their + * existing attributes. */ +export const CoachTipTrigger = Tooltip.Trigger; + +/** Portal + Content(class="cx-tooltip") rendering `label`, then the chord: + * chord = props.chord ?? shortcutFor(props.command, detectPlatform()); + * undefined → label only; contains " then " → plain-text sequence; otherwise + * . Never destructures props. */ +export const CoachTipContent: Component<{ + label: string; + command?: CommandId; + chord?: string; + /** Fully REPLACES the `.cx-tooltip` box class (ShortcutChip parity) — it does + * not augment it, so a caller passing this must re-include the row layout + * (.cx-tooltip's flex + the .cx-tooltip-label / .cx-palette-shortcut parts). */ + class?: string; +}> = (props) => { + const chord = () => + props.chord ?? + (props.command ? shortcutFor(props.command, detectPlatform()) : undefined); + const isSequence = () => chord()?.includes(" then ") ?? false; + return ( + + + {props.label} + + {(resolved) => ( + } + > + {resolved} + + )} + + + + ); +}; diff --git a/apps/ui/src/design/components/tooltip.css b/apps/ui/src/design/components/tooltip.css index 0d9566dd..87b8ee8f 100644 --- a/apps/ui/src/design/components/tooltip.css +++ b/apps/ui/src/design/components/tooltip.css @@ -1,10 +1,14 @@ /* Tooltip — .cx-tooltip (D3, Kobalte). Elev-1 float, open delay --cx-tooltip-delay (the delay is Kobalte's timing prop — this owns the visual box). Never load-bearing: the same info is reachable elsewhere. - Display surface (no interactive states). Consumes only --cx-* tiers. */ + Display surface (no interactive states). Consumes only --cx-* tiers. + Lays out its content as a row: the label, then a right-aligned chord chip + (.cx-palette-shortcut, reused — its margin-left:auto needs a flex parent). */ .cx-tooltip { - display: block; + display: flex; + align-items: center; + gap: var(--cx-space-2); max-width: 280px; padding: var(--cx-space-1) var(--cx-space-2); border: 1px solid var(--cx-border); @@ -17,3 +21,9 @@ box-shadow: var(--cx-elev-1); z-index: var(--cx-z-overlay); } + +/* Label sub-part — the control's name; takes free space so the chord chip + right-aligns via its own margin-left:auto. */ +.cx-tooltip-label { + flex: 1 1 auto; +}