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; +} 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;