feat: add Studio NLE playback controls#530
Conversation
7a5c858 to
ceecfd7
Compare
vanceingalls
left a comment
There was a problem hiding this comment.
Staff review: requesting changes.
The NLE control set is useful and most of the playback mechanics are going in the right direction, but the keyboard shortcut scoping needs tightening before this lands.
packages/studio/src/player/hooks/useTimelinePlayer.ts installs capture-phase keydown listeners on window and the iframe, then handles Space, ArrowLeft/ArrowRight, and J/K/L for any target that is not an input/textarea/select/contenteditable. That is broader than the PR intent of preview/timeline-surface shortcuts and it breaks existing interactive controls:
- focused buttons are not excluded, so pressing Space on Loop, the time/frame toggle, speed menu, timeline toggle, or any other Studio button toggles playback instead of activating the focused button
- the seek bar is
role="slider"and already has its own arrow-key handler inPlayerControls; the capture listener runs first, steps a frame, and prevents default before the slider-local behavior gets a clean event - other focusable Studio UI outside the preview/timeline surface can now unexpectedly play, pause, or seek the composition
Please either scope these shortcuts to the preview/timeline surface explicitly, or expand the target guard to ignore native/focusable interactive controls such as button, a[href], [role="button"], [role="slider"], [role="spinbutton"], and similar controls. I would also add regression coverage for Space on a focused toolbar button and ArrowRight on the seek slider so this does not drift again.
I did not find another blocker in the frame math, loop handling, or reverse-shuttle implementation from this patch.
ceecfd7 to
a45f900
Compare
|
@vanceingalls addressed the shortcut scoping blocker. Changes made:
Validation:
|
vanceingalls
left a comment
There was a problem hiding this comment.
Staff follow-up review: still requesting changes.
The earlier focused-button / seek-slider scoping blocker is largely addressed: the shortcut guard now ignores native controls and the slider role, and the targeted unit tests pass. I found two remaining shortcut-scope regressions before this is ready:
-
packages/studio/src/player/hooks/useTimelinePlayer.ts:handlePlaybackKeyDownhandles Space, ArrowLeft/ArrowRight, and J/K/L without excluding modified key chords. Because the listener is installed capture-phase onwindow, chords likeAlt+ArrowLeft,Cmd/Ctrl+L, orCmd/Ctrl+Kcan be consumed as frame-step/shuttle commands instead of reaching browser/app shortcuts. Please ignore playback shortcuts whenmetaKey,ctrlKey, oraltKeyis set, except for any explicitly supported modifier combo. -
packages/studio/src/player/hooks/useTimelinePlayer.ts: Arrow playback shortcuts still conflict with caption editing.CaptionOverlayalready uses Arrow keys to nudge selected caption words; the new capture listener sees those Arrow events first and seeks a frame before the caption nudge handler runs. Please scope Arrow frame-stepping to the preview/timeline playback surface, or otherwise suppress it while caption edit mode has a selected caption word / caption overlay is active.
Validation I ran locally on the fetched branch:
oxfmt --checkon changed Studio filesoxlinton changed Studio filesbun run --filter @hyperframes/studio test -- src/player/lib/time.test.ts src/player/store/playerStore.test.ts src/player/components/PlayerControls.test.ts src/player/hooks/useTimelinePlayer.test.tsbun run --filter @hyperframes/studio typecheck
|
@vanceingalls addressed the remaining shortcut-scope issues; current head is Changes made:
Validation:
Review was re-requested after validation. |
Merge activity
|
Problem
HyperFrames Studio made frame-accurate playback review slower than expected for editor-style workflows. Issue #527 called out missing loop playback, frame display/jump controls, preview-focused Space handling, frame stepping, and NLE-style J/K/L shuttle controls.
What this fixes
Root cause
The Studio playback layer only exposed mouse scrubbing, basic play/pause, a seconds-based readout, and slider-local arrow-key nudges. The global Space shortcut was also gated to
document.body, so it stopped working once the actual preview/editor surface had focus. Studio needed a single playback-control layer above the runtime adapter that could translate editor keyboard intent into deterministic seek/play/pause operations.Verification
Local checks
bun installbun run --filter @hyperframes/core build:hyperframes-runtimebunx oxfmt --check packages/studio/src/player/lib/time.ts packages/studio/src/player/lib/time.test.ts packages/studio/src/player/store/playerStore.ts packages/studio/src/player/store/playerStore.test.ts packages/studio/src/player/hooks/useTimelinePlayer.ts packages/studio/src/player/components/PlayerControls.tsx packages/studio/src/player/components/PlayerControls.test.ts packages/studio/src/components/nle/NLEPreview.tsxbunx oxlint packages/studio/src/player/lib/time.ts packages/studio/src/player/lib/time.test.ts packages/studio/src/player/store/playerStore.ts packages/studio/src/player/store/playerStore.test.ts packages/studio/src/player/hooks/useTimelinePlayer.ts packages/studio/src/player/components/PlayerControls.tsx packages/studio/src/player/components/PlayerControls.test.ts packages/studio/src/components/nle/NLEPreview.tsxbun run --filter @hyperframes/studio test -- src/player/lib/time.test.ts src/player/store/playerStore.test.ts src/player/components/PlayerControls.test.ts src/player/hooks/useTimelinePlayer.test.ts-> 4 files passed, 52 tests passedbun run --filter @hyperframes/studio typecheckbun run --filter @hyperframes/studio buildgit diff --checkBrowser verification
/tmp/hf-studio-nle-controlswith an animated 10s GSAP timeline.bun run --filter @hyperframes/cli dev -- preview /tmp/hf-studio-nle-controlsathttp://localhost:5194.agent-browserto verify:current / totalframesNotes
qa-artifacts/studio-nle-controls/frame-controls.pngqa-artifacts/studio-nle-controls/playback-controls.webm