feat(studio): fullscreen preview mode (F key)#997
Conversation
Add distraction-free fullscreen mode using the HTML5 Fullscreen API. Press F to enter fullscreen (Esc to exit). When active, the composition fills the screen — timeline, sidebars, and editing overlays are hidden while all playback shortcuts (Space, J/K/L, arrows, etc.) remain functional. A fullscreen toggle button is also added to player controls. Closes #995
jrusso1020
left a comment
There was a problem hiding this comment.
Verdict: COMMENT (would-be APPROVE)
Clean implementation, well-scoped (+118/-6 across 3 files), comprehensive test plan in the PR body. No blocking findings. Posting as COMMENT and pinging James on Slack for the merge greenlight per the trusted-stamper policy.
Sanity checks (no concerns — surfacing because "F key" deserves the explicit audit)
- No F-key conflicts in the studio. Grepped
packages/studio/forevent.key === "f",toLowerCase() === "f",key: "F"shortcut-section entries — clean. The strict modifier guard (!metaKey && !ctrlKey && !altKey && !shiftKey) correctly leaves Cmd+F (browser Find), Ctrl+F (Find), and Shift+F to the browser. isEditableTargetcoverage is comprehensive.packages/studio/src/utils/timelineDiscovery.ts:14-30matchesinput/textarea/select,isContentEditable,role=textbox|searchbox|combobox, and the.cm-editorCodeMirror class. F won't fire while typing anywhere we care about. ✓- iframe-to-parent forwarding works.
previewAppKeyDownHandler(useAppHotkeys.ts:289) delegates iframe-window keydown into the parent'shandleAppKeyDownRef.current. The F-key handler executes in the parent's closure, sodocument.fullscreenElement,document.querySelector("[data-studio-fullscreen-target]"), andcontainerRef.currentall resolve to the parent context. Same-origin iframe propagates user-activation, sorequestFullscreen()succeeds even when F is pressed inside the preview iframe. useSyncExternalStoreshape is correct.subscribe/getSnapshotare declared at module scope (stable refs), the subscription cleanup is tidy, and Object.is comparison onElement | nullre-renders only on actual fullscreen-state change. The null guardfullscreenElement === containerRef.current && fullscreenElement != nullis necessary — without it, initial render withcontainerRef.current === nullwould falsely reportisFullscreen=truewhile no fullscreen is active.- All three exit paths work: Escape (browser-native), F again (state-aware handler), button click (
onToggleFullscreen). ✓
Non-blocking observations
-
Two parallel toggle implementations.
useAppHotkeys.ts:259-263usesdocument.querySelector("[data-studio-fullscreen-target]")whileNLELayout.tsx:271-277usescontainerRef.current. Both resolve to the same element in practice (single NLELayout instance), but they're decoupled — if a future change moves the data attribute, only one path breaks. Optional consolidation: export atoggleStudioFullscreen()helper that both paths call. Not a blocker. -
event.repeatnot guarded. Holding F triggerskeydownwith auto-repeat firing requestFullscreen/exitFullscreen in rapid succession. Browsers tend to no-op back-to-back fullscreen API calls, but a one-linerif (event.repeat) return;at the top of the F-key block eliminates the race and the unsuppressedevent.preventDefault()chatter. Tiny. -
PostHog tracking on button path only. The button onClick (
PlayerControls.tsx:606-609) emitstrackStudioEvent("playback", { action: "fullscreen_toggle", active: !isFullscreen }), but the F-key handler inuseAppHotkeysdoesn't. This is consistent with the existing convention — hf#992's M/Shift+L hotkeys also don't emit telemetry from the hotkey path; only their buttons do. So this isn't a regression and isn't a finding for this PR. Flagging only because: if accurate fullscreen-adoption metrics matter (keyboard vs button is probably 90/10 for an NLE keyboard shortcut), the convention may want a follow-up cleanup that centralizes telemetry behind the toggle action itself rather than the click handler. -
requestFullscreen()rejection swallowed.void containerRef.current.requestFullscreen()andvoid document.querySelector(...).requestFullscreen()both discard promise rejection. If the browser blocks (rare — usually only sandboxed iframes withoutallow="fullscreen", which doesn't apply here), the user sees no feedback. A.catch(() => showToast("Couldn't enter fullscreen", "error"))would help; or rely on the absence of state change and accept silence. Cosmetic. -
Sub-composition breadcrumb hidden in fullscreen. When the user enters fullscreen from inside a sub-composition, the breadcrumb (
NLELayout.tsx:362-367) is hidden alongside the timeline — so the only way to navigate up is to exit fullscreen first (Escape / F / button). Intentional given "distraction-free preview" framing, but worth documenting in the keyboard shortcuts panel description if/when the feature gets a docs pass.
Test coverage note (not a blocker)
No new test added for the F-key handler. useAppHotkeys.ts doesn't have tests today either, so this matches the codebase convention — but hf#992 (the M/Shift+L PR) added usePlaybackKeyboard.test.ts cases for its new shortcuts, including the "doesn't fire in editable target" and "doesn't conflict with the modifier-less L" guard tests. If you want symmetry, the F-key block's predicate (shouldHandleFullscreenHotkey(event) returning bool from modifier + isEditableTarget checks) could be extracted to timelineDiscovery.ts next to shouldHandleTimelineToggleHotkey and unit-tested. Wholly optional.
— Rames Jusso
Summary
Implementation
useAppHotkeyshandles the F key in the consolidated handler (works from both main window and preview iframe)NLELayouttracks fullscreen state viauseSyncExternalStoreonfullscreenchangeevents and conditionally hides timeline/overlaysPlayerControlsreceivesisFullscreen+onToggleFullscreenprops for the toggle buttonTest plan
Closes #995