feat(studio): harden ui primitives — focus rings, keyboard tooltips, dialog hook

[vanceingalls]

Summary

Base of the studio UX-review stack (148 findings audited across the studio; 13 critical). This PR hardens the shared components/ui primitives that every later PR in the stack builds on.

Changes

• Button / IconButton: visible focus-visible outline (studio accent); disabled:pointer-events-none removed (replaced with disabled:cursor-not-allowed, hover/active gated behind enabled:) so disabled buttons can host explain-why tooltips.
• Tooltip: keyboard support (onFocus/onBlur triggers), role="tooltip", Escape-to-hide, viewport flip (top↔bottom) + horizontal clamping. API unchanged — all ~28 call sites unaffected.
• HyperframesLoader: role="status" on the loader; determinate track is a real role="progressbar" with aria-valuenow/min/max (was aria-hidden).
• VideoFrameThumbnail: error event resolves to a static fallback-label tile instead of an infinite shimmer; motion-reduce guard.
• NEW useDialogBehavior: shared modal contract — document-level Escape, Tab focus trap, focus-first-on-open, focus-restore-on-close, canClose() veto for dirty-draft guards. Adopted by every modal later in the stack.
• NEW SearchInput: shared search primitive with required aria-label, panel-input token style (kills the two-divergent-search-styles inconsistency in the sidebar).
• studio.css: hf-toast-in/out + hf-backdrop-in keyframes with prefers-reduced-motion guards (the previous animate-in fade-in classes were dead — no tailwindcss-animate plugin exists).

Verification

• oxlint 0 errors, oxfmt clean, tsc --noEmit clean at stack top
• Full studio suite at stack top: 1189 tests pass

Stack

PR 1/7 of the studio UX-review fixes. Merges bottom-up; the stack top is fully green (tsc + 1189 tests). Some shared-file edits span PRs, so intermediate branches may not typecheck in isolation.

🤖 Generated with Claude Code

[vanceingalls]

• fix(studio): captions UX — mode exit, undo, autosave surfacing, honest gating #1968
• fix(studio): player UX — keyboard timeline, honest waveform, keyframe menu actions #1967
• fix(studio): sidebar/panels UX — asset delete confirm, rename, search trap, undo #1966
• fix(studio): editor panel UX — commit safety, keyboard a11y, wired BlockParamsPanel #1965
• fix(studio): shell UX — data-loss guards, error surfacing, dialog contracts, toasts #1964
• feat(studio,studio-server,cli): render cancel end-to-end + renders/nle/storyboard UX #1963
• feat(studio): harden ui primitives — focus rings, keyboard tooltips, dialog hook #1962 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miga-heygen]

PR #1962 — feat(studio): harden ui primitives — focus rings, keyboard tooltips, dialog hook

VERDICT: Approve (with two nits)

SUMMARY: Strong foundational PR for the studio UX stack. Every change is purposeful: accessibility gaps are closed properly, focus management follows WCAG patterns, and the new primitives (useDialogBehavior, SearchInput) are well-shaped for downstream adoption. The enabled: prefix swap on button variants is the kind of subtle correctness that prevents real user confusion (disabled buttons that swallow tooltip hover via pointer-events-none is a known pain point). Code quality is high throughout.

---

FINDINGS

Tooltip — Escape listener + useDialogBehavior Escape listener: stacking (low, informational)

When a tooltip is visible inside a dialog, pressing Escape will fire both the Tooltip's document.addEventListener("keydown") (bubble phase) and useDialogBehavior's document.addEventListener("keydown", ..., true) (capture phase). The dialog handler calls e.stopPropagation() but that stops bubble-phase propagation from capture — the Tooltip listener fires first in bubble phase... actually, wait: the dialog registers in capture (true), so the dialog's handler fires first, calls stopPropagation(), and the Tooltip's bubble-phase listener never fires. That's correct for the "dialog owns Escape" case. But if the tooltip is shown outside a dialog context, the Tooltip listener works fine standalone. Good — the layering is sound.

Tooltip show — clears stale timer on re-entry (good catch)

The added if (timerRef.current) clearTimeout(timerRef.current); at the top of show prevents double-firing when pointer rapidly re-enters. The original code would leak a second setTimeout without clearing the first. Well spotted.

Tooltip — onFocus/onBlur on the wrapper span

The <span className="contents"> wrapper with onFocus/onBlur means focus events from any focusable descendant will bubble up and trigger the tooltip. This is the right behavior — if you Tab to a button inside a Tooltip wrapper, the tooltip appears. Good pattern for keyboard discoverability.

1. useDialogBehavior — focus trap queries focusables once per keypress (nit)

Array.from(el.querySelectorAll<HTMLElement>(FOCUSABLE)) runs on every Tab keypress. For the studio's modals (typically <20 focusable elements) this is negligible, but if a future modal has dynamic content where focusables appear/disappear, the fresh query is actually correct — caching would miss newly added buttons. So this is fine as-is. Mentioning it only because it's a common review question.

2. useDialogBehavior — focus restore fires in cleanup (nit, minor timing)

Focus restore runs in the useEffect cleanup: restoreRef.current?.focus(). If the component unmounts (rather than open toggling to false), the cleanup still runs, which is correct — the previously focused element gets focus back either way. One edge case: if React batches the unmount with other DOM mutations, the restore target might have been removed from the DOM by the time .focus() runs. In practice this is harmless (.focus() on a detached element is a no-op), but worth noting for posterity.

3. Button — outline-none resets browser default before focus-visible ring (correct)

outline-none focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-1 focus-visible:outline-studio-accent — the outline-none base ensures only keyboard-initiated focus shows the ring (no mouse click outline). The focus-visible pseudo-class then re-applies a styled outline. This is the standard Tailwind pattern and it's applied consistently to both Button and IconButton.

4. SearchInput — aria-label required via TypeScript interface (excellent)

interface SearchInputProps extends Omit<InputHTMLAttributes<HTMLInputElement>, "type"> {
  "aria-label": string;
}

This enforces at compile time that every search input has an accessible name. Placeholder text alone isn't sufficient for screen readers (WCAG 1.3.1). Making it a required prop rather than a lint rule is the stronger guarantee. Well done.

5. HyperframesLoader — role="progressbar" with live values (correct)

The progress bar now has role="progressbar" with aria-valuemin={0}, aria-valuemax={100}, and aria-valuenow={Math.round(boundedProgress * 100)}. Note boundedProgress is 0-1 (normalized) and the ARIA values are 0-100 (percentage). The Math.round(boundedProgress * 100) conversion is correct. The parent role="status" on the loader container means screen readers get live-region announcements. Good pairing.

6. VideoFrameThumbnail — motion-reduce:animate-none (correct)

Adding motion-reduce:animate-none to the shimmer state respects prefers-reduced-motion. The error fallback is a static tile with text — no motion, no infinite shimmer. Clean resolution.

7. .fallowrc.jsonc additions — large but justified

63 lines of fallow suppressions. The comments explain these are line-shifted inherited complexity re-flagged by the UX sweep, plus small a11y handlers tripping CRAP thresholds without coverage data. Reviewed individually in the stack. Reasonable for a 148-finding sweep across ~90 files.

8. CSS keyframes — prefers-reduced-motion guards (correct)

Toast and backdrop animations both have @media (prefers-reduced-motion: reduce) blocks that kill animation. The toast exit also sets opacity: 0 so the exit state is reached immediately without motion. Correct — reduced-motion users see instant transitions, not frozen mid-animation frames.

---

PONYTAIL

Two small tightenings, neither blocking:

1. Tooltip aria-describedby linkage: The role="tooltip" div has no id, and the trigger <span> has no aria-describedby. Screen readers see the tooltip role but can't programmatically associate it with the trigger. Consider generating a stable id (e.g., useId()) and wiring aria-describedby on the trigger span. This is WCAG 4.1.2 territory — the visual tooltip works, but the programmatic association is missing.

2. useDialogBehavior FOCUSABLE selector: Missing summary element and [contenteditable] from the selector. These are natively focusable but won't be trapped. Low risk for the studio (unlikely to have contenteditable in a modal), but the selector could be more complete for a "shared contract" primitive.

net: -0 lines possible — this is already tight. The fallow suppressions are the bulk of the additions and they're justified. Ship.

---

DIFF_STATS: +357 / -22 across 8 files (2 new: SearchInput.tsx, useDialogBehavior.ts)

Review by Miga

[miguel-heygen]

Stack review for PR 1/7.

Audited: .fallowrc.jsonc, packages/studio/src/components/ui/Button.tsx, Tooltip.tsx, SearchInput.tsx, useDialogBehavior.ts, HyperframesLoader.tsx, and styles/studio.css at head 679bf1b.

The disabled-button tooltip fix is the right shape: disabled buttons keep pointer hover so wrappers can explain disabled state, while hover/active styling is scoped to enabled:. Tooltip keyboard focus, Escape dismissal, and viewport flipping are locally sound, and useDialogBehavior centralizes the studio modal contract without taking markup ownership away from the callers. The latest head also adds explicit fallow suppressions for the primitives that are intentionally introduced before their downstream stack consumers.

Nits already covered by Miga remain non-blocking: tooltip trigger/description linkage could be stronger with aria-describedby, and the focusable selector can grow summary/[contenteditable] later.

Verdict: APPROVE
Reasoning: I did not find a blocker in this PR’s UI primitive delta; the prior Fallow boundary issue is addressed on the latest head, though CI still needs to finish green before merge.

— Magi

[vanceingalls]

Addressed both ponytail items at head 13bc5d26f: Tooltip now wires aria-describedby via useId() (trigger ↔ bubble association, WCAG 4.1.2), and the useDialogBehavior FOCUSABLE selector gained summary + [contenteditable="true"].

🤖 Generated with Claude Code

[miguel-heygen]

Current-head re-review at 13bc5d26f344987e2df397f9521984b54c917a0c after Vance's follow-up.

Audited: packages/studio/src/components/ui/Tooltip.tsx and packages/studio/src/components/ui/useDialogBehavior.ts at the current head, focusing on the two earlier non-blocking nits. The tooltip now generates a stable useId() id, wires aria-describedby only while visible, and applies the same id to the role="tooltip" bubble. The dialog focusable selector now includes summary and [contenteditable="true"], closing the shared-primitive selector gap.

CI is green on the current head, including Graphite mergeability.

Verdict: APPROVE
Reasoning: The current-head delta resolves the previously noted accessibility nits without changing the surrounding tooltip/dialog lifecycle, and I found no new blocker in the touched primitive surfaces.

— Magi