fix(studio): player UX — keyboard timeline, honest waveform, keyframe menu actions

[vanceingalls]

Summary

Player + timeline fixes from the studio UX review. Standouts: the keyframe context menu dropped two fully-wired actions (Change Ease and Copy Properties were plumbed all the way through Timeline.tsx but never rendered), and a failed waveform decode silently rendered fabricated sine-wave peaks that users would then trim against.

Changes

Restored dead features:

• KeyframeDiamondContextMenu renders "Change Ease" (ease list with current-ease checkmark) and "Copy Properties" (async, "Copied!"/"Copy failed"); divider + distinct treatment isolate "Delete All Keyframes" from the single-delete (they were adjacent and identically styled).

Honest media states:

• AudioWaveform: fake-peaks fallback removed — failures render a visually distinct dashed flat line + "waveform unavailable", cached so it doesn't refetch-loop.
• Player error event shows an inline error + Retry (was: silently cleared the loading overlay, leaving a blank preview); asset-wait overlay logs the promised debug line and offers "Continue anyway" after 3s.
• VideoThumbnail errors show a static placeholder instead of an infinite shimmer.

Keyboard access:

• Timeline clips: role="button", focusable, Enter/Space selection, focus ring (selection gates all downstream keyframe/property editing and was mouse-only).
• Keyframe diamonds: keyboard-activatable (onClick gated on event.detail === 0 so pointer paths don't double-fire), labeled "Keyframe at N%".
• SpeedMenu: Escape closes, aria-haspopup/aria-expanded, menuitemradio, CSS hover, visible disabled state. ShortcutsPanel: Escape closes (a keyboard-help panel keyboard users couldn't dismiss), focus in/restore, scoped section titles disambiguate duplicate K/Del bindings, new "⇧Click — split all tracks" row (the gesture existed but was documented nowhere). Context menus get menu roles + arrow-key nav via a small shared helper. Seek slider: Home/End + fixed aria-valuenow clobber.

Error-prevention:

• Beat dots: hit area 12→24px; delete moved off double-click to ⌥-click (a stuttered drag could destroy a beat).
• EditModal: draft prompt persists (Escape/outside-click no longer destroys typed text); clipboard failures show an inline alert.

Craft: clip hover 100→150ms; seek-thumb hover scale 1.25→1.10; overlay fade-out ease-in.

Verification

• vitest src/player/: 249 pass / 0 fail; oxlint 0 errors across all 65 player files; tsc clean

Stack

PR 6/7 of the studio UX-review fixes.

🤖 Generated with Claude Code

[vanceingalls]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• fix(studio): captions UX — mode exit, undo, autosave surfacing, honest gating #1968
• fix(studio): player UX — keyboard timeline, honest waveform, keyframe menu actions #1967 👈 (View in Graphite)
• 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
• main

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

[miga-heygen]

PR Review — fix(studio): player UX — keyboard timeline, honest waveform, keyframe menu actions

VERDICT: Approve (comment-only per process)

Summary

Strong PR that addresses three interconnected gaps across 17 files: (1) dead features that were fully wired through Timeline.tsx props but never rendered (Change Ease, Copy Properties), (2) a dishonest waveform fallback that fabricated plausible-looking peaks on decode failure, and (3) keyboard inaccessibility of timeline clips, keyframe diamonds, and several menus. The changes are well-scoped for a PR 6/7 in a stack and each fix is independently motivated.

What works well

• Honest waveform is the headline improvement. The old fakePeaks function generated sine-wave-modulated data with a seeded PRNG that looked convincingly like a real waveform — exactly the kind of thing a user would trim or beat-align against. Replacing it with a dashed flat line + "waveform unavailable" text, cached via decodeFailed Set, is the right call. The refetch-loop prevention via the Set + decodeError state pair is clean.
• Dead feature resurrection done right. onChangeEase and onCopyProperties were declared in the props interface, threaded through Timeline.tsx, but never destructured or rendered. The new context menu properly wires them with current-ease checkmark display, collapsible ease list, and async copy with inline status feedback.
• menuKeyboardNav.ts — compact shared utility following APG menu patterns (arrow keys, Home/End, focus-on-mount, focus-restore-on-unmount). Correctly scopes its listener to the menu container so it can't conflict with the seek bar's Home/End handler. Good that it filters for :not(:disabled) items.
• event.detail === 0 guard on keyframe diamond onClick is the textbook technique for keyboard-only activation on elements that also have onPointerUp handlers. Prevents the double-fire correctly.
• Beat dot delete migration from double-click to ⌥-click is a genuine UX safety fix — a stuttered drag attempt on a 12px target (now 24px, also good) absolutely could register as a double-click.

Findings

Nit — aria-valuenow removal (PlayerControls.tsx):
The hardcoded aria-valuenow={0} is removed, leaving useSeekBarDrag.ts:145 as the sole writer via setAttribute. This is correct since the dynamic value overwrites the static one on every seek. One subtlety: before the first seek fires, there's no aria-valuenow on the element at all. ARIA spec says aria-valuenow is required for role="slider", and the initial state is conceptually "0". In practice screen readers handle the omission gracefully since aria-valuemin={0} implies the starting value, so this is fine — just noting the spec-letter gap.

Nit — onCopyProperties type broadening (KeyframeDiamondContextMenu.tsx):
The prop type changed from (elementId, percentage) => void to => Promise<boolean> | boolean | void. The Timeline.tsx callsite returns copyTextToClipboard(...) which is Promise<boolean>, so the union is well-motivated. The result === false check in handleCopyProperties correctly distinguishes "returned false" from "returned void/undefined" (void falls through to the success path). This is the right behavior: callers that don't return anything are assumed to succeed.

Nit — ShortcutsPanel now uses useContextMenuDismiss for its panel (ShortcutsPanel.tsx):
Nice consolidation. Worth noting that useContextMenuDismiss adds both mousedown and keydown listeners on document, while the old code only had mousedown. The new behavior adds Escape-to-close, which the PR description calls out. Focus-in on open (panelBodyRef.current?.focus()) and focus-restore on close (via the cleanup return) are correct and make the panel keyboard-navigable. The triggerRef on the button ensures focus returns to the trigger, not the panel.

Observation — maxHeight on keyframe context menu:
The ease list can get tall (20+ GSAP ease options). The max-h-40 on the inner scroll container plus overflow-y-auto + maxHeight: calc(100vh - ...) on the outer menu is a double-scroll prevention pattern. Confirmed: the outer maxHeight accommodates the non-ease items, and the inner max-h-40 (160px) keeps the ease sub-list scrollable independently. This is correct — the menu itself won't overflow the viewport.

Observation — motion-reduce:animate-none additions:
Applied to both AudioWaveform and VideoThumbnail shimmer states. Good accessibility practice.

Ponytail lens

This PR is the kind of work that separates a real product from a demo. Fabricating waveform data when decode fails is the sort of "works in the demo" shortcut that causes real user harm downstream — someone trims audio to a beat that doesn't exist, publishes, and gets a jarring cut. The explicit degraded state ("waveform unavailable" with a dashed line) respects the user's intelligence. The dead features being wired-but-never-rendered suggests a rushed PR boundary in the original work — this PR cleans that up without fanfare. The keyboard accessibility additions are thorough without being over-engineered: menuKeyboardNav.ts is 47 lines, covers the APG essentials, and is already shared across three menus. No complaints.

Diff stats

+464 / -141 across 17 files (1 new: menuKeyboardNav.ts)

CI: all required checks pass (preflight, preview parity, player-perf, regression).

---

Review by Miga

🤖 Generated with Claude Code

[miguel-heygen]

Stack review for PR 6/7.

Audited: packages/studio/src/player/components/AudioWaveform.tsx, menuKeyboardNav.ts, KeyframeDiamondContextMenu.tsx, Player.tsx, PlayerControls.tsx, TimelineClipDiamonds.tsx, and the related menu/timeline wiring at head 81d53b6.

The waveform change is the right product call: decode failures now render an explicit unavailable state and cache failure by URL instead of fabricating plausible peaks. The menu keyboard helper is compact, scoped to menu containers, filters disabled items, and restores focus. The keyframe menu re-surfaces previously threaded-but-unrendered actions without broadening the timeline mutation surface more than necessary.

Miga’s ARIA nit on the initial seekbar aria-valuenow is valid but not blocking for this delta.

Verdict: APPROVE
Reasoning: The audited player changes remove a misleading waveform fallback and complete menu/keyframe interactions without introducing a blocking accessibility or state-management regression.

— Magi