From 24df2611f07fccd8744516aa6129995bb3b9421c Mon Sep 17 00:00:00 2001 From: mintaka Date: Tue, 25 Aug 2026 13:32:37 -0400 Subject: [PATCH 1/4] 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/4] 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; +} From a881232997c68671242b810de4e4315c8fcc67b3 Mon Sep 17 00:00:00 2001 From: mintaka Date: Tue, 25 Aug 2026 14:05:31 -0400 Subject: [PATCH 3/4] feat(ui): adopt CoachTip across command-backed chrome + register sidebar toggles (RIG-2704) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coaching-tooltip adoption sweep (T2 of the coaching-tooltips design): convert every command-backed `title=` in the shell chrome to a `CoachTip`, and register the two sidebar-toggle commands so their coached chord actually dispatches. - spine.ts / store.ts: register `sidebar.toggleLeft`/`sidebar.toggleRight` (global scope, no hand-authored shortcut — the chord derives from the keymap, D4) beside their existing store behavior. The chords `Mod+Shift+\` /`Mod+\` were declared in the keymap but registered nowhere (dead) — registering them next to the behavior they already drive is DL-229-compliant. The `toggleLeft`/ `toggleRight` closures move above `createKeyboardSpine` so they can be threaded into its deps; the click path is unchanged. - App.tsx: convert the topbar Bridge tab (view.bridge) and the two glyph-only sidebar toggles. Drop native `title=`, keep `aria-keyshortcuts`, and add `aria-label` to the two glyph toggles (§A5 — a bare block glyph is not an accessible name). - LeftSidebar.tsx: convert the four view buttons (Bridge/Backlog/Done/Settings). Drop native `title=`, keep `aria-keyshortcuts`. Backlog/Done have no keymap row yet, so their CoachTip is label-only. - Keep-native boundary (§A4) held: new-folder, pin/unpin, disabled subscribe/ join, role/status/truncation titles keep their native `title=` (no registered command → nothing to coach). - Chord is never hand-authored: `CoachTipContent` resolves it via `shortcutFor(command, platform)`; the now-unused `bridgeChord`/`chord` helpers and `shortcutFor` imports are removed from both files. Tests (43 pass across spine/App/LeftSidebar/e2e): tooltip-reveal-on-focus with keymap-derived chord, `title`-dropped + `aria-keyshortcuts`-kept guard, glyph-toggle accessible-name via aria-label, keep-native new-folder boundary, the two new registrations (metadata + run), and live coached-chord dispatch of both sidebar toggles (registry resolution + real keydown). Also updates the pre-existing RIG-2483 Palette test that asserted the LeftSidebar buttons' native `title` chord — that display chord now rides a CoachTip, so the test asserts the `title` is absent (a native title would double-tooltip) while `aria-keyshortcuts` is unchanged. Ledger-impact: none (decisions landed with the design PR #569; DL-245..247). RIG-2704 --- apps/ui/src/App.test.tsx | 122 +++++++++++++++++ apps/ui/src/App.tsx | 96 ++++++++----- apps/ui/src/components/LeftSidebar.test.tsx | 70 ++++++++++ apps/ui/src/components/LeftSidebar.tsx | 141 ++++++++++---------- apps/ui/src/components/Palette.test.tsx | 10 +- apps/ui/src/keyboard-e2e.test.tsx | 42 ++++++ apps/ui/src/keyboard/spine.test.ts | 31 +++++ apps/ui/src/keyboard/spine.ts | 18 +++ apps/ui/src/store.ts | 7 +- 9 files changed, 431 insertions(+), 106 deletions(-) diff --git a/apps/ui/src/App.test.tsx b/apps/ui/src/App.test.tsx index 5173f07e..28531838 100644 --- a/apps/ui/src/App.test.tsx +++ b/apps/ui/src/App.test.tsx @@ -1,6 +1,9 @@ import { describe, expect, test } from "bun:test"; import { flush as flushSync } from "solid-js"; import { STUB_CHANNELS, STUB_MESSAGES, STUB_TOPICS } from "./comms-stub"; +import type { CommandId } from "./keyboard/commands"; +import { detectPlatform } from "./keyboard/dispatch"; +import { shortcutFor } from "./keyboard/keymap"; import { STUB_AGENTS } from "./stub-data"; import { flush, mountApp } from "./test-router"; @@ -197,3 +200,122 @@ describe("App shell (T7)", () => { expect(leftPresent()).toBe(true); }); }); + +// Coaching-tooltip adoption sweep (RIG-2530 T2). The topbar Bridge tab and the +// two glyph-only sidebar toggles convert from a native `title=` to a CoachTip; +// the toggles' dead chords are registered so they now dispatch. These assert +// the observable adoption contract: the tooltip reveals on focus, no `title` +// double-tooltips, `aria-keyshortcuts` survives, and the glyph toggles keep a +// non-glyph accessible name via the added `aria-label`. + +// Kobalte portals its tooltip content on a macrotask, so a focus that opens it +// is observable only after one setTimeout(0). +async function settle(): Promise { + const { promise, resolve } = Promise.withResolvers(); + setTimeout(resolve, 0); + await promise; +} + +describe("coaching tooltips (RIG-2530 T2)", () => { + test("the Bridge tab opens a coaching tooltip on focus showing the label + chord", async () => { + const { container } = mountApp("/backlog"); + const tab = navViewTabs(container).find((t) => + t.textContent?.includes("Bridge"), + ); + expect(tab).toBeDefined(); + + tab?.focus(); + await settle(); + + // Kobalte portals the tooltip content to document.body. + const tooltip = + document.body.querySelector('[role="tooltip"]'); + expect(tooltip).not.toBeNull(); + expect(tooltip?.textContent).toContain("Bridge"); + // Chord derived from the keymap, never hand-authored (D4). + const chip = tooltip?.querySelector(".cx-palette-shortcut"); + const kbds = Array.from(chip?.querySelectorAll("kbd") ?? []).map( + (k) => k.textContent, + ); + expect(shortcutFor("view.bridge" as CommandId, detectPlatform())).toBe( + "Ctrl+B", + ); + expect(kbds).toEqual(["Ctrl", "B"]); + }); + + test("converted controls drop `title` but keep `aria-keyshortcuts`", () => { + const { container } = mountApp(); + const bridgeTab = navViewTabs(container).find((t) => + t.textContent?.includes("Bridge"), + ); + expect(bridgeTab?.hasAttribute("title")).toBe(false); + expect(bridgeTab?.getAttribute("aria-keyshortcuts")).toBeTruthy(); + + for (const label of ["Toggle left sidebar", "Toggle right sidebar"]) { + const toggle = container.querySelector( + `.pane-toggle[aria-label="${label}"]`, + ); + expect(toggle).not.toBeNull(); + expect(toggle?.hasAttribute("title")).toBe(false); + expect(toggle?.getAttribute("aria-keyshortcuts")).toBeTruthy(); + } + }); + + test("the glyph-only sidebar toggles are named by aria-label, not the bare glyph", () => { + const { container } = mountApp(); + const left = container.querySelector( + '.pane-toggle[aria-label="Toggle left sidebar"]', + ); + const right = container.querySelector( + '.pane-toggle[aria-label="Toggle right sidebar"]', + ); + expect(left).not.toBeNull(); + expect(right).not.toBeNull(); + // The visible content is a decorative block glyph; the accessible name + // must come from aria-label, never the glyph. + expect(left?.getAttribute("aria-label")).toBe("Toggle left sidebar"); + expect(right?.getAttribute("aria-label")).toBe("Toggle right sidebar"); + expect(left?.textContent?.trim()).not.toBe(""); + expect(left?.getAttribute("aria-label")).not.toBe( + left?.textContent?.trim(), + ); + }); + + test("both sidebar toggles are now live: their coached chords dispatch", async () => { + const { store } = mountApp(); + expect(store.leftOpen()).toBe(true); + expect(store.rightOpen()).toBe(true); + + // Both commands the sweep coaches resolve in the registry (dispatch path), + // not only the keymap (display path) — the drift the A4 boundary guards. + expect( + store.keyboard.registry.get("sidebar.toggleLeft" as CommandId), + ).toBeDefined(); + expect( + store.keyboard.registry.get("sidebar.toggleRight" as CommandId), + ).toBeDefined(); + + // Mod+Shift+\ → toggleLeft; Mod+\ → toggleRight (keymap rows), now that + // the commands are registered. + window.dispatchEvent( + new KeyboardEvent("keydown", { + key: "\\", + ctrlKey: true, + shiftKey: true, + bubbles: true, + }), + ); + await flush(); + expect(store.leftOpen()).toBe(false); + + window.dispatchEvent( + new KeyboardEvent("keydown", { + key: "\\", + ctrlKey: true, + bubbles: true, + }), + ); + await flush(); + expect(store.rightOpen()).toBe(false); + }); +}); diff --git a/apps/ui/src/App.tsx b/apps/ui/src/App.tsx index 0be2a744..bc946fd5 100644 --- a/apps/ui/src/App.tsx +++ b/apps/ui/src/App.tsx @@ -8,6 +8,11 @@ import "./design/components/card.css"; import "./design/components/menu.css"; import "./design/components/shortcuts.css"; import "./app.css"; +import { + CoachTip, + CoachTipContent, + CoachTipTrigger, +} from "./components/CoachTip"; import { LeftSidebar } from "./components/LeftSidebar"; import { Palette } from "./components/Palette"; import { RightSidebar } from "./components/RightSidebar"; @@ -17,7 +22,7 @@ import { UsageBar } from "./components/UsageBar"; import { useStore } from "./context"; import type { CommandId } from "./keyboard/commands"; import { detectPlatform, installKeymap } from "./keyboard/dispatch"; -import { shortcutFor, shortcutForAria } from "./keyboard/keymap"; +import { shortcutForAria } from "./keyboard/keymap"; // The Compass ADE shell — an Orca-inspired layout over the compass.v1 surface // (docs/specs/product/compass.md). A CSS grid: a topbar, a left agent-folder @@ -58,10 +63,9 @@ const App: Component = (props) => { store.keyboard.activeZone, ), ); - // Point-of-use chip parity (RIG-2483, D10): the topbar Bridge tab announces - // its chord via aria-keyshortcuts + title, resolved from the keymap through + // Point-of-use coaching (RIG-2530): the topbar Bridge tab announces its chord + // via aria-keyshortcuts + a CoachTip tooltip, resolved from the keymap through // shortcutFor (D4) — matching the LeftSidebar view buttons. - const bridgeChord = shortcutFor("view.bridge" as CommandId, detectPlatform()); const bridgeAria = shortcutForAria( "view.bridge" as CommandId, detectPlatform(), @@ -80,18 +84,24 @@ const App: Component = (props) => {
- - + + store.toggleLeft()} + > + ▐ + + + + + store.toggleRight()} + > + ▌ + + +
diff --git a/apps/ui/src/components/LeftSidebar.test.tsx b/apps/ui/src/components/LeftSidebar.test.tsx index ffd154bf..3b774ed5 100644 --- a/apps/ui/src/components/LeftSidebar.test.tsx +++ b/apps/ui/src/components/LeftSidebar.test.tsx @@ -3,6 +3,9 @@ import { fireEvent, render } from "@solidjs/testing-library"; import { flush } from "solid-js"; import { STUB_CHANNELS, STUB_COMMS_STATE } from "../comms-stub"; import { StoreContext } from "../context"; +import type { CommandId } from "../keyboard/commands"; +import { detectPlatform } from "../keyboard/dispatch"; +import { shortcutFor } from "../keyboard/keymap"; import { type AppStore, createAppStore } from "../store"; import { STUB_AGENTS } from "../stub-data"; import { testQueryClient } from "../test-support"; @@ -346,3 +349,70 @@ describe("LeftSidebar (T5)", () => { expect(serverAcpRow?.querySelector(".agent-activity")).toBeNull(); }); }); + +// Coaching-tooltip adoption sweep (RIG-2530 T2). The four view buttons +// (Bridge/Backlog/Done/Settings) convert from a native `title=` to a CoachTip; +// the new-folder button stays native (no registered command → nothing to +// coach, the A4/D4 boundary). These assert the observable adoption contract. + +// Kobalte portals its tooltip content on a macrotask. +async function settle(): Promise { + const { promise, resolve } = Promise.withResolvers(); + setTimeout(resolve, 0); + await promise; +} + +const viewButtons = (container: HTMLElement): HTMLElement[] => [ + ...container.querySelectorAll("button.bridge-link"), +]; + +describe("LeftSidebar coaching tooltips (RIG-2530 T2)", () => { + test("the Bridge button opens a coaching tooltip on focus with label + chord", async () => { + const { container } = mountSidebar(); + const bridge = viewButtons(container).find((b) => + b.textContent?.includes("Bridge"), + ); + expect(bridge).toBeDefined(); + + bridge?.focus(); + await settle(); + + const tooltip = + document.body.querySelector('[role="tooltip"]'); + expect(tooltip).not.toBeNull(); + expect(tooltip?.textContent).toContain("Bridge"); + const chip = tooltip?.querySelector(".cx-palette-shortcut"); + const kbds = Array.from(chip?.querySelectorAll("kbd") ?? []).map( + (k) => k.textContent, + ); + expect(shortcutFor("view.bridge" as CommandId, detectPlatform())).toBe( + "Ctrl+B", + ); + expect(kbds).toEqual(["Ctrl", "B"]); + }); + + test("every converted view button drops `title`, keeps `aria-keyshortcuts` where a chord exists, and has a text accessible name", () => { + const { container } = mountSidebar(); + const buttons = viewButtons(container); + expect(buttons.length).toBe(4); + for (const b of buttons) { + expect(b.hasAttribute("title")).toBe(false); + // Text-labelled buttons carry their accessible name from visible text — + // no aria-label needed. + expect(b.hasAttribute("aria-label")).toBe(false); + expect(b.textContent?.trim()).not.toBe(""); + } + // Bridge + Settings have keymap rows → aria-keyshortcuts present. + const bridge = buttons.find((b) => b.textContent?.includes("Bridge")); + expect(bridge?.getAttribute("aria-keyshortcuts")).toBeTruthy(); + }); + + test("the keep-native new-folder button still carries its native title (sweep boundary held)", () => { + const { container } = mountSidebar(); + const newFolder = [ + ...container.querySelectorAll("button.icon-btn"), + ].find((b) => b.getAttribute("title") === "New folder"); + expect(newFolder).toBeDefined(); + expect(newFolder?.getAttribute("title")).toBe("New folder"); + }); +}); diff --git a/apps/ui/src/components/LeftSidebar.tsx b/apps/ui/src/components/LeftSidebar.tsx index e55e17d7..3a7abcf9 100644 --- a/apps/ui/src/components/LeftSidebar.tsx +++ b/apps/ui/src/components/LeftSidebar.tsx @@ -15,8 +15,9 @@ import type { Channel } from "../comms-stub"; import { useStore } from "../context"; import type { CommandId } from "../keyboard/commands"; import { detectPlatform } from "../keyboard/dispatch"; -import { shortcutFor, shortcutForAria } from "../keyboard/keymap"; +import { shortcutForAria } from "../keyboard/keymap"; import { type Agent, type AgentTreeNode, agentTree } from "../stub-data"; +import { CoachTip, CoachTipContent, CoachTipTrigger } from "./CoachTip"; import { StateDot } from "./StateDot"; /** An agent leaf row in the tree — the per-agent select button, plus a hover @@ -423,13 +424,11 @@ export const LeftSidebar: Component = () => { // Backlog view badge: the pre-active tier (Todo + Backlog) the human triages. const backlogCount = () => backlogIssues(store.issues()).length + store.assignedIssues().length; - // Point-of-use shortcut chips (RIG-2483, D10): the view buttons that fire the - // D6-seeded show* paths announce their chord via aria-keyshortcuts + title, - // resolved from the keymap through shortcutFor (never hand-authored — D4). - // view.backlog/view.done have no keymap row yet, so shortcutFor is undefined - // and the attribute is simply omitted. + // Point-of-use coaching (RIG-2530): the view buttons announce their chord via + // aria-keyshortcuts + a CoachTip tooltip, resolved from the keymap through + // shortcutFor inside CoachTipContent (never hand-authored — D4). view.backlog/ + // view.done have no keymap row yet, so the tooltip is label-only there. const platform = detectPlatform(); - const chord = (id: string) => shortcutFor(id as CommandId, platform); const ariaChord = (id: string) => shortcutForAria(id as CommandId, platform); return ( diff --git a/apps/ui/src/components/Palette.test.tsx b/apps/ui/src/components/Palette.test.tsx index 7b278da4..db04868a 100644 --- a/apps/ui/src/components/Palette.test.tsx +++ b/apps/ui/src/components/Palette.test.tsx @@ -237,7 +237,7 @@ describe("Palette (RIG-2483)", () => { expect(container.querySelector(".cx-palette-loading")).toBeNull(); }); - test("the LeftSidebar view buttons carry aria-keyshortcuts in WAI-ARIA tokens while title keeps the display chord", () => { + test("the LeftSidebar view buttons carry aria-keyshortcuts in WAI-ARIA tokens; the display chord moved to a CoachTip (RIG-2530), so no native title", () => { setPlatform("other"); const { container } = mountApp("/"); const links = Array.from( @@ -245,10 +245,12 @@ describe("Palette (RIG-2483)", () => { ); const bridge = links.find((b) => b.textContent?.includes("Bridge")); const settings = links.find((b) => b.textContent?.includes("Settings")); - // view.bridge → Mod+B, view.settings → Mod+, — aria uses Control (WAI-ARIA - // token), the display title keeps Ctrl. + // 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 + // double-tooltip, so it must be absent. expect(bridge?.getAttribute("aria-keyshortcuts")).toBe("Control+B"); - expect(bridge?.getAttribute("title")).toContain("Ctrl+B"); + expect(bridge?.getAttribute("title")).toBeNull(); expect(settings?.getAttribute("aria-keyshortcuts")).toBe("Control+,"); }); }); diff --git a/apps/ui/src/keyboard-e2e.test.tsx b/apps/ui/src/keyboard-e2e.test.tsx index 2d8c2d11..ee82dbca 100644 --- a/apps/ui/src/keyboard-e2e.test.tsx +++ b/apps/ui/src/keyboard-e2e.test.tsx @@ -305,3 +305,45 @@ describe("shortcuts overlay (RIG-2482)", () => { expect(event.defaultPrevented).toBe(false); }); }); + +// Coached-chord dispatch (RIG-2530 T2). Every command id the adoption sweep +// coaches must resolve in the command REGISTRY (dispatch path), not merely the +// keymap (display path) — the drift the A4 boundary exists to prevent. The two +// sidebar toggles are the load-bearing case: T2 registered them beside their +// store behavior, so their coached chord now actually fires. +describe("coached-chord dispatch (RIG-2530 T2)", () => { + // Every command id the sweep coaches. view.backlog/view.done are coached + // label-only (no keymap row yet) but must still resolve in the registry. + const COACHED_COMMANDS = [ + "view.bridge", + "view.backlog", + "view.done", + "view.settings", + "sidebar.toggleLeft", + "sidebar.toggleRight", + ] as const; + + test("every coached command id resolves in the command registry", () => { + const { store } = mountApp("/"); + for (const id of COACHED_COMMANDS) { + expect(store.keyboard.registry.get(id as CommandId)).toBeDefined(); + } + }); + + test("the sidebar-toggle chords now dispatch (live after T2 registration)", async () => { + setPlatform("other"); + const { store } = mountApp("/"); + expect(store.leftOpen()).toBe(true); + expect(store.rightOpen()).toBe(true); + + // Mod+Shift+\ → sidebar.toggleLeft (keymap.ts). + press({ key: "\\", ctrlKey: true, shiftKey: true }); + await flush(); + expect(store.leftOpen()).toBe(false); + + // Mod+\ → sidebar.toggleRight. + press({ key: "\\", ctrlKey: true }); + await flush(); + expect(store.rightOpen()).toBe(false); + }); +}); diff --git a/apps/ui/src/keyboard/spine.test.ts b/apps/ui/src/keyboard/spine.test.ts index ee28a9fa..ebf4b470 100644 --- a/apps/ui/src/keyboard/spine.test.ts +++ b/apps/ui/src/keyboard/spine.test.ts @@ -23,6 +23,8 @@ function stubDeps( showDone: () => void; showSettings: () => void; togglePalette: () => void; + toggleLeft: () => void; + toggleRight: () => void; }> = {}, ) { return { @@ -32,6 +34,8 @@ function stubDeps( showDone: () => {}, showSettings: () => {}, togglePalette: () => {}, + toggleLeft: () => {}, + toggleRight: () => {}, ...overrides, }; } @@ -168,4 +172,31 @@ describe("createKeyboardSpine", () => { expect(backlog).toBe(1); expect(done).toBe(1); }); + + test("registers sidebar.toggleLeft/toggleRight as global commands beside their store behavior (RIG-2530 T2/D1)", () => { + const spine = createKeyboardSpine(stubDeps()); + for (const [seed, title] of [ + ["sidebar.toggleLeft", "Toggle left sidebar"], + ["sidebar.toggleRight", "Toggle right sidebar"], + ] as const) { + const cmd = spine.registry.get(id(seed)); + expect(cmd).toBeDefined(); + expect(cmd?.title).toBe(title); + expect(cmd?.scope).toBe("global"); + // No hand-authored shortcut — the chord derives from the keymap (D4). + expect(cmd?.shortcut).toBeUndefined(); + } + }); + + test("sidebar.toggleLeft/toggleRight run() fire their toggle legs", () => { + let left = 0; + let right = 0; + const spine = createKeyboardSpine( + stubDeps({ toggleLeft: () => left++, toggleRight: () => right++ }), + ); + spine.registry.get(id("sidebar.toggleLeft"))?.run(); + spine.registry.get(id("sidebar.toggleRight"))?.run(); + expect(left).toBe(1); + expect(right).toBe(1); + }); }); diff --git a/apps/ui/src/keyboard/spine.ts b/apps/ui/src/keyboard/spine.ts index 821217ee..082e38aa 100644 --- a/apps/ui/src/keyboard/spine.ts +++ b/apps/ui/src/keyboard/spine.ts @@ -74,6 +74,8 @@ export function createKeyboardSpine(deps: { showDone: () => void; showSettings: () => void; togglePalette: () => void; + toggleLeft: () => void; + toggleRight: () => void; }): KeyboardSpine { const registry = createCommandRegistry(); const viewBridge: Command = { @@ -124,6 +126,22 @@ export function createKeyboardSpine(deps: { run: () => deps.showDone(), }; registry.register(viewDone); + const sidebarToggleLeft: Command = { + id: "sidebar.toggleLeft" as CommandId, + title: "Toggle left sidebar", + keywords: ["sidebar", "left", "toggle", "pane"], + scope: "global", + run: () => deps.toggleLeft(), + }; + registry.register(sidebarToggleLeft); + const sidebarToggleRight: Command = { + id: "sidebar.toggleRight" as CommandId, + title: "Toggle right sidebar", + keywords: ["sidebar", "right", "toggle", "pane"], + scope: "global", + run: () => deps.toggleRight(), + }; + registry.register(sidebarToggleRight); const groups = new Set(); diff --git a/apps/ui/src/store.ts b/apps/ui/src/store.ts index 4ed1bcde..765920a7 100644 --- a/apps/ui/src/store.ts +++ b/apps/ui/src/store.ts @@ -1992,6 +1992,8 @@ export function createAppStore(options: AppStoreOptions): AppStore { if (paletteOpen()) closePalette(); else openPalette(); }; + const toggleLeft = () => setLeftOpen((v) => !v); + const toggleRight = () => setRightOpen((v) => !v); // The keyboard spine (RIG-2456): created here, after the `show*`/toggle // closures exist, so `view.bridge` + the RIG-2482/2483 seeds are registered // next to their behavior. App.tsx installs the one window keymap listener over @@ -2004,6 +2006,8 @@ export function createAppStore(options: AppStoreOptions): AppStore { showDone, showSettings, togglePalette, + toggleLeft, + toggleRight, }); const setTrackerConfig = (cfg: TrackerConfig) => { @@ -2014,9 +2018,6 @@ export function createAppStore(options: AppStoreOptions): AppStore { seam = createFixtureTrackerSeam(cfg); }; - const toggleLeft = () => setLeftOpen((v) => !v); - const toggleRight = () => setRightOpen((v) => !v); - const isAgentCollapsed = (agentId: string) => collapsed().has(agentId); const toggleAgent = (agentId: string) => setCollapsed((prev) => { From 0449f69eaff193b094181ba9af284c97bc70e287 Mon Sep 17 00:00:00 2001 From: mintaka Date: Tue, 25 Aug 2026 14:27:05 -0400 Subject: [PATCH 4/4] docs(ui): document the shipped CoachTip consumer + retarget the ShortcutChip note (RIG-2705) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - components.md §Tooltip: note the shipped `CoachTip` consumer of `.cx-tooltip` and its content contract — label + keymap-resolved chord (`shortcutFor`, never hand-authored): plus-chords reuse `ShortcutChip`, `" then "` leader sequences render as plain text, a command with no keymap row is label-only. - ShortcutChip.tsx: the `class`-prop comment cited a `.cx-tooltip` host as a future (RIG-2530) consumer; that host shipped as `CoachTip`, so the comment now names it. Docs/comment only; no behavior change. markdownlint + biome clean. Ledger-impact: none (DL-245..247 landed with the design PR #569). RIG-2705 --- apps/ui/src/components/ShortcutChip.tsx | 4 ++-- apps/ui/src/design/components.md | 9 ++++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/apps/ui/src/components/ShortcutChip.tsx b/apps/ui/src/components/ShortcutChip.tsx index 5b544a49..e08ae39a 100644 --- a/apps/ui/src/components/ShortcutChip.tsx +++ b/apps/ui/src/components/ShortcutChip.tsx @@ -2,8 +2,8 @@ // resolved chord ("Ctrl+K", "Shift+Enter") on "+" and renders the // `.cx-palette-shortcut` chip. The chord it renders is ALWAYS the // resolveChord-resolved display string (via `shortcutFor`); it never resolves -// `Mod` itself. The `class` prop lets a future `.cx-menu-item`/`.cx-tooltip` -// host (RIG-2530) restyle the box while reusing the same split rendering. +// `Mod` itself. The `class` prop lets a host like `CoachTip`'s `.cx-tooltip` +// (RIG-2530) restyle the box while reusing the same split rendering. // // OQ-5: chords render as resolved text ("Ctrl+K"); mapping to "⌘K" on mac is a // one-component change here later. diff --git a/apps/ui/src/design/components.md b/apps/ui/src/design/components.md index 9aba77ec..c88a2e3c 100644 --- a/apps/ui/src/design/components.md +++ b/apps/ui/src/design/components.md @@ -456,9 +456,16 @@ Agent tree + channel/topic rows; caret, state dot, pin affordance. ## Tooltip (Kobalte) - **Class:** `.cx-tooltip`. +- **Consumer:** `CoachTip` (`components/CoachTip.tsx`) — the shipped + label+chord coaching tooltip adopted across the command-backed chrome + (topbar Bridge tab, the four LeftSidebar view buttons, the two sidebar + toggles). `CoachTipContent` renders the control's label, then its chord + resolved from the keymap via `shortcutFor` (never hand-authored): a plus-chord + reuses `ShortcutChip`, a `" then "` leader sequence renders as plain text, and + a command with no keymap row is label-only. - **States:** display surface (elev-1); open delay `--cx-tooltip-delay` (400ms) is Kobalte's timing prop. Never load-bearing — the same info is - reachable elsewhere. + reachable elsewhere (a converted control keeps its `aria-keyshortcuts`). - **Tokens:** `--cx-bg-raised`, `--cx-border`, `--cx-text`, `--cx-font-ui`, `--cx-text-xs`, `--cx-space-1/-2`, `--cx-radius-sm`, `--cx-elev-1`, `--cx-z-overlay`.