Skip to content

fix(studio): a focused video or audio player keeps its arrow and play keys - #4854

Merged
miguel-heygen merged 2 commits into
mainfrom
fix/studio-media-focus-keys
Oct 1, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
fix/studio-media-focus-keys

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

What changes

With a video or audio player focused (the asset preview's native controls), the keys Studio's shortcuts used to take from it now go to that player only: the arrow keys and Space, plus the playback letters (J, K, L and the rest) and the canvas, caption and snap keys. Before, the playback shortcuts claimed the arrows first: the timeline playhead stepped, the key never reached the player, and the playhead move closed the asset preview.

Why it was broken

Every Studio key handler asks "does the focused element own this key?" before acting. The shared answer (isTypingTarget) covers text fields, editors, switches and comboboxes, but not a native media player, which seeks and plays with these keys itself. The playback shortcuts and the canvas nudge both relied on it.

What this does

  • ownsPlainKeys in utils/typingTarget.ts (added by feat(studio): fade handles ride the fade's end and the waveform shows the fade #4844 for sliders) now has one list of controls that own their plain keys: [role='slider'], video[controls] and audio[controls], on top of anything isTypingTarget claims. There is no second predicate.
  • The playback shortcuts (shouldIgnorePlaybackShortcutTarget, which the timeline player's key handler also goes through), the canvas arrow nudge, the caption-word arrow nudge and the preview's S/G snap keys use it, and so does the app's plain-key dispatcher (feat(studio): fade handles ride the fade's end and the waveform shows the fade #4844), so Delete on a focused player no longer deletes the selected clip.
  • Three handlers kept their own copy of the text-input check: the timeline's N (snapping), K (add keyframe) and the automation selection keys (Delete, Cmd+C, Cmd+V). They now call isTypingTarget, so a focused picker such as the font combobox keeps its keys there too.
  • The slideshow panel's in-panel Cmd+Z checked only INPUT and TEXTAREA, so Cmd+Z in a contenteditable or a select undid the panel instead of the text. It now calls isTypingTarget (a Cmd shortcut belongs to a text field, not to a slider).
  • The playback shortcuts' own selector list dropped its slider, combobox, switch and textbox rows: ownsPlainKeys runs first and already claims them.
  • shouldHandleTimelineDeleteKey was dead code (re-exported from Timeline.tsx, called only by its own test) with its own copy of the typing check. Deleted with its tests.
  • Deliberate: N, K and the automation keys now also ignore a focused switch or combobox, the same as the app hotkeys already do. After flipping a switch (focus stays on it), K does nothing until focus moves.
  • The snap keys keep their existing exclusion of a focused preview iframe: a key aimed at the composition is not a Studio shortcut.

Handlers checked

Handler Keys Predicate
Playback shortcuts (usePlaybackKeyboard) arrows, Space, J/K/L and the other playback letters ownsPlainKeys (changed)
Timeline player keys (useTimelinePlayer) same, via shouldIgnorePlaybackShortcutTarget ownsPlainKeys (inherited)
Canvas nudge (useDomEditNudge) arrows ownsPlainKeys (changed)
Caption-word nudge (captions/keyboard.ts) arrows ownsPlainKeys (changed)
Snap and grid (SnapToolbar) S, G ownsPlainKeys (was its own tag check)
Snapping toggle (TimelineToolbar) N isTypingTarget (was its own tag check)
Add keyframe (useKeyframeKeyboard) K isTypingTarget (was its own copy)
Automation selection (useAutomationSelectionKeyboard) Delete, Backspace, Escape, Cmd+C, Cmd+V isTypingTarget (was its own copy)
Slideshow panel undo (SlideshowPanel) Cmd+Z inside the panel isTypingTarget (was an INPUT/TEXTAREA tag check)
App plain keys (dispatchPlainKey) letters, Delete ownsPlainKeys (from #4844; now also covers players)
App chords (dispatchModifierKey, useAppHotkeys) Cmd shortcuts isTypingTarget, unchanged: a slider or player does not own Cmd+Z, copy or group
Undo and redo (shouldIgnoreHistoryShortcut) Cmd+Z, Cmd+Shift+Z isTypingTarget, unchanged, same reason
Timeline row navigation (timelineKeyboardNavigation) arrows inside the timeline rows not affected: it only runs with focus inside the timeline

Tests that fail without the fix

  • usePlaybackKeyboard.test.ts: "leaves the timeline alone while a
  • useDomEditNudge.test.tsx: "does not nudge the selection while a
  • captions/keyboard.test.ts: "leaves the arrows to a focused
  • SnapToolbar.test.tsx: "leaves S and G alone while a combobox / select / switch / video player has focus" (four rows).
  • typingTarget.test.ts: "is true for a native player with controls and for anything typing claims".
  • useAppHotkeys.test.ts: "leaves Delete to a focused
  • TimelineToolbar.test.tsx: "N toggles snapping, but not while a picker that owns typing has focus".
  • useKeyframeKeyboard.test.tsx: "leaves K to a focused picker that owns typing, like a font combobox".
  • useAutomationSelectionKeyboard.test.tsx: "is inert while a picker that owns typing has focus".
  • SlideshowPanel.test.ts: "leaves Cmd+Z to a text field, contenteditable and select included".

A video without controls still lets the playback keys step the timeline (tested).

Before

A small fixture project with one 20 s video asset not yet on the timeline. The asset preview is open with its video paused at 0:02 and focused:

The asset preview open, video at 0:02, focused

ArrowRight: the timeline playhead steps from frame 0 to frame 1, the video never sees the key, and the preview closes because the playhead moved.

Before: playhead 0f to 1f, the video preview closed

After

The same steps: the playhead stays on frame 0, the video seeks from 2.00 s to 2.20 s, and the preview stays open.

After: playhead stays at 0f, the video seeks to 2.20 s

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Edit accuracy: 846 passing here, 846 on the base branch

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (2)

@miguel-heygen
miguel-heygen force-pushed the fix/studio-media-focus-keys branch 5 times, most recently from 01fd658 to 75cd15c Compare October 1, 2026 20:01
@miguel-heygen
miguel-heygen force-pushed the fix/studio-media-focus-keys branch from 75cd15c to 13d9b2d Compare October 1, 2026 21:05
@miguel-heygen
miguel-heygen marked this pull request as ready for review October 1, 2026 21:48
@miguel-heygen
miguel-heygen merged commit a59d93a into main Oct 1, 2026
68 checks passed
@miguel-heygen
miguel-heygen deleted the fix/studio-media-focus-keys branch October 1, 2026 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant