From 254de3d1c4c1babe4484e0d3112a3c1e3e5c4cc1 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Fri, 21 Aug 2026 11:37:28 -0700 Subject: [PATCH] =?UTF-8?q?fix(studio):=20captions=20UX=20=E2=80=94=20mode?= =?UTF-8?q?=20exit,=20undo,=20autosave=20surfacing,=20honest=20gating=20(#?= =?UTF-8?q?1968)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caption-editing fixes from the studio UX review. This surface held five of the thirteen criticals; the theme is that the editing UI shipped ahead of its apply/persist pipeline, so several controls mutated an in-memory model with no downstream effect, and the mode itself could never be exited. Mode trap: caption edit mode auto-activated on detection and had no exit — `setEditMode(false)` and `reset()` had zero call sites, so the caption overlay replaced normal element editing for the rest of the session, even after switching compositions. The store now resets on composition change (flushing the last debounced edit first), an "Editing captions · Exit" pill sits on the preview, and a re-enter button appears once dismissed. Honest gating of dead surfaces: the Animation tab (31 presets × duration/ease/stagger/intensity) edited state that was never applied to playback nor serialized — wiring it needs a CaptionOverride schema extension in packages/core plus a runtime engine, so the tab is now visibly disabled with an amber "isn't applied to playback or saved yet" notice instead of silently discarding work. Timing edge-drags moved a block that never changed playback and never saved; the handles are gone and the blocks remain as select/seek targets. Double-click split desynced the overlay↔DOM index mapping, so split is out until regeneration exists. Undo: store-level undo/redo (cap 50, 800ms coalescing by edit target) across all ten mutations, with ⌘Z/⇧⌘Z intercepted while caption mode is active and reapplied to the live iframe. Previously ⌘Z reverted an unrelated file edit while the bad caption drag persisted. Autosave: save failures, including non-2xx, raise a persistent "not saved — Retry" banner; the code's own comment called this a data-loss path and it was telemetry-only. Debounced saves flush on unmount instead of being discarded, `beforeunload` flushes and warns while pending, and corrupt overrides JSON is distinguished from a missing file. Input safety and a11y: arrow-key nudge no longer hijacks arrows inside form inputs; numeric fields commit finite values only (typing "-" used to inject NaN into gsap and persist null); "Mixed" shows on multi-select divergence; Escape cancels an in-flight drag and restores the pre-drag transform; ⌘A selects all; caption blocks are keyboard-selectable with a playhead line and click-to-seek (CaptionTimeline's `onSeek` prop existed but nothing passed it); 24px hit areas around the 8px handles; a hint when no boxes are visible; visible input focus styles; tablist semantics. Perf: the 66ms getBoundingClientRect polling loop is replaced with event-driven updates (player-store subscription, preview messages, ResizeObserver, rAF-coalesced); the interval now runs only during playback. Reconciled against main: StudioPreviewArea.tsx was deleted by the Studio revamp (#2291), so the mode pill, the sync-error banner and the re-enter button move to its successor, nle/PreviewOverlays.tsx, and the caption track's onSeek is wired in EditorShell. The per-keyframe onChangeKeyframeEase change that also lived in that file is dropped: main removed the prop, and #1967 now routes the diamond menu's ease action to the focused-ease-segment editor instead. Restacked onto main now that PRs 1962-1967 have squash-merged, so this carries only its own changes. Co-authored-by: Claude Opus 5 (1M context) --- .../components/CaptionAnimationPanel.tsx | 59 +++-- .../captions/components/CaptionOverlay.tsx | 201 +++++++++++++++--- .../components/CaptionOverlayUtils.ts | 54 +++++ .../components/CaptionPropertyPanel.tsx | 72 +++++-- .../captions/components/CaptionTimeline.tsx | 115 ++++------ .../studio/src/captions/components/shared.tsx | 77 ++++++- .../src/captions/hooks/useCaptionSync.ts | 85 ++++++-- packages/studio/src/captions/keyboard.test.ts | 12 ++ packages/studio/src/captions/keyboard.ts | 13 +- packages/studio/src/captions/store.ts | 134 ++++++++++-- .../studio/src/components/EditorShell.tsx | 8 +- .../src/components/nle/PreviewOverlays.tsx | 71 ++++++- packages/studio/src/hooks/useAppHotkeys.ts | 22 ++ .../studio/src/hooks/useCaptionDetection.ts | 24 ++- 14 files changed, 769 insertions(+), 178 deletions(-) diff --git a/packages/studio/src/captions/components/CaptionAnimationPanel.tsx b/packages/studio/src/captions/components/CaptionAnimationPanel.tsx index c120876b5..5d9989e16 100644 --- a/packages/studio/src/captions/components/CaptionAnimationPanel.tsx +++ b/packages/studio/src/captions/components/CaptionAnimationPanel.tsx @@ -61,7 +61,13 @@ const EASE_PRESETS = [ "bounce.out", ]; -import { Section, Row, inputCls } from "./shared"; +import { Section, Row, inputCls, NumberField } from "./shared"; + +// Animation edits currently mutate only the in-memory model: they are not +// applied to the preview and not serialized by buildOverrides, so tuning them +// would be editing blind. Controls stay visible (so the capability is +// discoverable) but disabled until the apply/persist pipeline exists. +const ANIMATION_PIPELINE_WIRED = false; // --------------------------------------------------------------------------- // Animation phase controls @@ -72,6 +78,7 @@ interface AnimationPhaseProps { presets: string[]; animation: CaptionAnimation | null; showIntensity?: boolean; + disabled?: boolean; onChange: (update: Partial) => void; } @@ -80,6 +87,7 @@ function AnimationPhase({ presets, animation, showIntensity, + disabled, onChange, }: AnimationPhaseProps) { const preset = animation?.preset ?? "none"; @@ -93,6 +101,8 @@ function AnimationPhase({ onChange({ duration: Number(e.target.value) })} - className={inputCls} + disabled={disabled} + ariaLabel={`${label} duration`} + onCommit={(v) => onChange({ duration: v })} /> onChange({ stagger: Number(e.target.value) })} - className={inputCls} + disabled={disabled} + ariaLabel={`${label} stagger`} + onCommit={(v) => onChange({ stagger: v })} /> @@ -151,8 +163,13 @@ function AnimationPhase({ max={1} step={0.01} value={intensity} - onChange={(e) => onChange({ intensity: Number(e.target.value) })} - className="flex-1 accent-studio-accent" + disabled={disabled} + aria-label={`${label} intensity`} + onChange={(e) => { + const v = Number(e.target.value); + if (Number.isFinite(v)) onChange({ intensity: v }); + }} + className="flex-1 accent-studio-accent disabled:opacity-40" /> {intensity.toFixed(2)} @@ -222,19 +239,31 @@ export const CaptionAnimationPanel = memo(function CaptionAnimationPanel() { if (!group || !resolvedGroupId || !animation) { return (
-

Select a caption group to edit animations

+

Select a caption word to edit animations

); } + const gated = !ANIMATION_PIPELINE_WIRED; + return (
+ {gated && ( +
+

+ Animation editing isn't applied to playback or saved yet, so these controls are + disabled. +

+
+ )} + {/* Scrollable content */}
@@ -243,6 +272,7 @@ export const CaptionAnimationPanel = memo(function CaptionAnimationPanel() { presets={HIGHLIGHT_PRESETS} animation={animation.highlight} showIntensity + disabled={gated} onChange={handleHighlightChange} /> @@ -250,6 +280,7 @@ export const CaptionAnimationPanel = memo(function CaptionAnimationPanel() { label="Exit" presets={EXIT_PRESETS} animation={animation.exit} + disabled={gated} onChange={handleExitChange} />
@@ -259,7 +290,9 @@ export const CaptionAnimationPanel = memo(function CaptionAnimationPanel() { diff --git a/packages/studio/src/captions/components/CaptionOverlay.tsx b/packages/studio/src/captions/components/CaptionOverlay.tsx index 773d705c3..5681b25a2 100644 --- a/packages/studio/src/captions/components/CaptionOverlay.tsx +++ b/packages/studio/src/captions/components/CaptionOverlay.tsx @@ -1,7 +1,8 @@ import { memo, useState, useCallback, useRef } from "react"; import { useCaptionStore } from "../store"; +import { usePlayerStore } from "../../player"; import { useMountEffect } from "../../hooks/useMountEffect"; -import { shouldHandleCaptionNudgeKey } from "../keyboard"; +import { shouldHandleCaptionNudgeKey, isEditableEventTarget } from "../keyboard"; import { readWordBoxes, getWordEl, @@ -9,6 +10,7 @@ import { getOrCreateWrapper, writeTransform, computeTransformStyle, + registerCaptionIframe, type WordBox, } from "./CaptionOverlayUtils"; @@ -16,7 +18,8 @@ interface CaptionOverlayProps { iframeRef: React.RefObject; } -const HANDLE = 8; +const HANDLE = 8; // visual size of a handle dot +const HANDLE_HIT = 24; // pointer target size (invisible padding around the dot) const ROTATION_OFFSET = 20; // px above the selection box /** Sync canvas state back to the Zustand store so the property panel reflects it. */ @@ -38,6 +41,8 @@ export const CaptionOverlay = memo(function CaptionOverlay({ iframeRef }: Captio const overlayRef = useRef(null); const modelRef = useRef(model); modelRef.current = model; + // Set by the mount effect; lets Escape handling cancel an in-flight drag. + const cancelDragRef = useRef<(() => boolean) | null>(null); // Interaction mode — only one active at a time const interactionRef = useRef< @@ -78,8 +83,14 @@ export const CaptionOverlay = memo(function CaptionOverlay({ iframeRef }: Captio useMountEffect(() => { if (!isEditMode) return; + + // Let undo/redo (useAppHotkeys) reapply restored models to this iframe. + const unregisterIframe = registerCaptionIframe(iframeRef); + let prevBoxes: WordBox[] = []; + let rafId: number | null = null; const tick = () => { + rafId = null; const iframe = iframeRef.current; const m = modelRef.current; const overlay = overlayRef.current; @@ -95,15 +106,98 @@ export const CaptionOverlay = memo(function CaptionOverlay({ iframeRef }: Captio prevBoxes = next; setWordBoxes(next); }; - const id = setInterval(tick, 66); + // Coalesce bursts of triggers (player store updates, resize) into one + // layout read per frame — replaces the old unconditional 66ms polling. + const scheduleTick = () => { + if (rafId === null) rafId = requestAnimationFrame(tick); + }; + + // Event sources: player state changes (seek/scrub/select), preview + // messages, element resize, window resize. + const unsubPlayer = usePlayerStore.subscribe(scheduleTick); + const handleMessage = (e: MessageEvent) => { + const data = e.data; + if (data?.source === "hf-preview") scheduleTick(); + }; + window.addEventListener("message", handleMessage); + window.addEventListener("resize", scheduleTick); + const resizeObserver = new ResizeObserver(scheduleTick); + if (iframeRef.current) resizeObserver.observe(iframeRef.current); + if (overlayRef.current) resizeObserver.observe(overlayRef.current); + + // During playback caption groups animate continuously — a light interval + // is the only reliable driver, but it runs ONLY while playing. + let playInterval: ReturnType | null = null; + const syncPlaybackInterval = () => { + const playing = usePlayerStore.getState().isPlaying; + if (playing && playInterval === null) { + playInterval = setInterval(scheduleTick, 66); + } else if (!playing && playInterval !== null) { + clearInterval(playInterval); + playInterval = null; + } + }; + const unsubPlayback = usePlayerStore.subscribe(syncPlaybackInterval); + syncPlaybackInterval(); tick(); - // Arrow key nudge for selected words + const getWin = (): Window | null => { + try { + return iframeRef.current?.contentWindow ?? null; + } catch { + return null; + } + }; + + const cancelActiveDrag = (): boolean => { + const i = interactionRef.current; + if (!i) return false; + const win = getWin(); + if (win) { + // Restore the pre-drag transform instead of committing the partial one. + writeTransform(i.wordEl, win, i.origTX, i.origTY, i.origScale, i.origRotation); + } + interactionRef.current = null; + return true; + }; + cancelDragRef.current = cancelActiveDrag; + const handleKeyDown = (e: KeyboardEvent) => { - const { selectedSegmentIds: sel, model: m } = useCaptionStore.getState(); + const store = useCaptionStore.getState(); + const { selectedSegmentIds: sel, model: m } = store; + + // Escape: cancel an in-flight drag first, else clear the selection. + if (e.key === "Escape") { + if (cancelActiveDrag()) { + e.preventDefault(); + scheduleTick(); + return; + } + if (sel.size > 0) { + e.preventDefault(); + store.clearSelection(); + } + return; + } + + // ⌘A / Ctrl+A selects every caption word (unless typing in a field). + if ( + (e.metaKey || e.ctrlKey) && + e.key.toLowerCase() === "a" && + !e.shiftKey && + !e.altKey && + !isEditableEventTarget(e.target) + ) { + e.preventDefault(); + store.selectAll(); + return; + } + if (sel.size === 0 || !m) return; const arrow = e.key; - if (!shouldHandleCaptionNudgeKey(e)) return; + // Pass the event target so arrows inside inputs keep their native + // behavior instead of nudging the canvas word. + if (!shouldHandleCaptionNudgeKey(e, e.target)) return; e.preventDefault(); const step = e.shiftKey ? 10 : 1; @@ -129,12 +223,21 @@ export const CaptionOverlay = memo(function CaptionOverlay({ iframeRef }: Captio break; } } + scheduleTick(); }; window.addEventListener("keydown", handleKeyDown); return () => { - clearInterval(id); + unregisterIframe(); + unsubPlayer(); + unsubPlayback(); + if (playInterval !== null) clearInterval(playInterval); + if (rafId !== null) cancelAnimationFrame(rafId); + resizeObserver.disconnect(); + window.removeEventListener("message", handleMessage); + window.removeEventListener("resize", scheduleTick); window.removeEventListener("keydown", handleKeyDown); + cancelDragRef.current = null; }; }); @@ -297,6 +400,13 @@ export const CaptionOverlay = memo(function CaptionOverlay({ iframeRef }: Captio onPointerUp={handlePointerUp} onLostPointerCapture={handlePointerUp} > + {wordBoxes.length === 0 && model && model.segments.size > 0 && ( +
+ + No captions visible at this frame — scrub to a caption, or select one in the track below + +
+ )} {wordBoxes.map((box) => { const isSelected = selectedSegmentIds.has(box.segmentId); return ( @@ -325,23 +435,33 @@ export const CaptionOverlay = memo(function CaptionOverlay({ iframeRef }: Captio > {isSelected && ( <> - {/* Rotation handle — circle above the box */} + {/* Rotation handle — 24px hit area around an 8px dot */}
startRotate(box, e)} - /> + > +
+
{/* Line from box to rotation handle */}
- {/* Scale handles — four corners */} + {/* Scale handles — four corners, 24px hit areas around 8px dots */} {[ - { right: -HANDLE / 2, bottom: -HANDLE / 2, cursor: "nwse-resize" }, - { left: -HANDLE / 2, top: -HANDLE / 2, cursor: "nwse-resize" }, - { right: -HANDLE / 2, top: -HANDLE / 2, cursor: "nesw-resize" }, - { left: -HANDLE / 2, bottom: -HANDLE / 2, cursor: "nesw-resize" }, - ].map((pos, idx) => ( + { + corner: { right: -HANDLE_HIT / 2, bottom: -HANDLE_HIT / 2 }, + cursor: "nwse-resize", + }, + { + corner: { left: -HANDLE_HIT / 2, top: -HANDLE_HIT / 2 }, + cursor: "nwse-resize", + }, + { + corner: { right: -HANDLE_HIT / 2, top: -HANDLE_HIT / 2 }, + cursor: "nesw-resize", + }, + { + corner: { left: -HANDLE_HIT / 2, bottom: -HANDLE_HIT / 2 }, + cursor: "nesw-resize", + }, + ].map(({ corner, cursor }, idx) => (
startScale(box.groupIndex, box.wordIndex, box.segmentId, e) } - /> + > +
+
))} )} diff --git a/packages/studio/src/captions/components/CaptionOverlayUtils.ts b/packages/studio/src/captions/components/CaptionOverlayUtils.ts index 473958617..c4d5bb9f5 100644 --- a/packages/studio/src/captions/components/CaptionOverlayUtils.ts +++ b/packages/studio/src/captions/components/CaptionOverlayUtils.ts @@ -205,6 +205,60 @@ export function writeTransform( } } +// --------------------------------------------------------------------------- +// Iframe registry — lets non-component code (undo/redo in useAppHotkeys) +// reapply a restored model's transforms to the live preview. +// --------------------------------------------------------------------------- + +let registeredIframe: React.RefObject | null = null; + +export function registerCaptionIframe(ref: React.RefObject): () => void { + registeredIframe = ref; + return () => { + if (registeredIframe === ref) registeredIframe = null; + }; +} + +/** + * True when the caption preview iframe is mounted AND visible. The caption + * store's isEditMode stays true while the preview is merely hidden (e.g. + * storyboard view), so hotkeys must not route ⌘Z to the caption stack unless + * the user can actually see the captions the undo would change. + */ +export function isCaptionPreviewVisible(): boolean { + const iframe = registeredIframe?.current; + return Boolean(iframe?.isConnected && iframe.offsetParent !== null); +} + +/** Reapply every segment's transform from a (restored) model to the preview DOM. */ +export function applyCaptionModelToIframe(model: { + groupOrder: string[]; + groups: Map; + segments: Map; +}): void { + const iframe = registeredIframe?.current; + if (!iframe) return; + let win: Window | null = null; + try { + win = iframe.contentWindow; + } catch { + return; + } + if (!win) return; + for (let gi = 0; gi < model.groupOrder.length; gi++) { + const group = model.groups.get(model.groupOrder[gi]); + if (!group) continue; + for (let wi = 0; wi < group.segmentIds.length; wi++) { + const seg = model.segments.get(group.segmentIds[wi]); + if (!seg) continue; + const wordEl = getWordEl(iframe, gi, wi); + if (!wordEl) continue; + const s = seg.style; + writeTransform(wordEl, win, s.x ?? 0, s.y ?? 0, s.scaleX ?? 1, s.rotation ?? 0); + } + } +} + /** Compute style deltas from the current wrapper transform — used by syncToStore in the overlay. */ export function computeTransformStyle(el: HTMLElement, iframeWin: Window): Record { const wrapper = getOrCreateWrapper(el); diff --git a/packages/studio/src/captions/components/CaptionPropertyPanel.tsx b/packages/studio/src/captions/components/CaptionPropertyPanel.tsx index 3ac838c04..37850f1d7 100644 --- a/packages/studio/src/captions/components/CaptionPropertyPanel.tsx +++ b/packages/studio/src/captions/components/CaptionPropertyPanel.tsx @@ -2,7 +2,29 @@ import { memo, useCallback, useState } from "react"; import { useCaptionStore } from "../store"; import type { CaptionStyle } from "../types"; import { CaptionAnimationPanel } from "./CaptionAnimationPanel"; -import { Section, Row, inputCls } from "./shared"; +import { Section, Row, NumberField } from "./shared"; + +/** True when the given style key differs across the selected segments. */ +function isMixedValue( + model: { segments: Map }> } | null, + selectedSegmentIds: Set, + key: "x" | "y" | "scaleX" | "rotation", + fallback: number, +): boolean { + if (!model || selectedSegmentIds.size < 2) return false; + let first: number | undefined; + let seen = false; + for (const segId of selectedSegmentIds) { + const v = model.segments.get(segId)?.style[key] ?? fallback; + if (!seen) { + first = v; + seen = true; + } else if (v !== first) { + return true; + } + } + return false; +} // --------------------------------------------------------------------------- // Main component @@ -180,6 +202,11 @@ export const CaptionPropertyPanel = memo(function CaptionPropertyPanel({ const rotation = effectiveStyle.rotation ?? 0; const scaleX = effectiveStyle.scaleX ?? 1; + const xMixed = isMixedValue(model, selectedSegmentIds, "x", 0); + const yMixed = isMixedValue(model, selectedSegmentIds, "y", 0); + const scaleMixed = isMixedValue(model, selectedSegmentIds, "scaleX", 1); + const rotationMixed = isMixedValue(model, selectedSegmentIds, "rotation", 0); + // Count label const countLabel = selectedSegmentIds.size === 1 ? "1 word" : `${selectedSegmentIds.size} words`; @@ -191,9 +218,11 @@ export const CaptionPropertyPanel = memo(function CaptionPropertyPanel({ {countLabel}
{/* Tab switcher */} -
+
+
+ {captionSyncError && ( +
+ {captionSyncError} + + +
+ )} + + ); } return ( @@ -270,6 +328,15 @@ export function PreviewOverlays({ isPlaying={isPlaying} /> {gestureOverlay} + {captionModelPresent && captionDismissed && ( + + )} ); } diff --git a/packages/studio/src/hooks/useAppHotkeys.ts b/packages/studio/src/hooks/useAppHotkeys.ts index 4325e8118..505f80790 100644 --- a/packages/studio/src/hooks/useAppHotkeys.ts +++ b/packages/studio/src/hooks/useAppHotkeys.ts @@ -7,6 +7,11 @@ import type { LeftSidebarHandle } from "../components/sidebar/LeftSidebar"; import { STUDIO_MOTION_PATH } from "../components/editor/studioMotion"; import { isTypingTarget } from "../utils/typingTarget"; import { isEditableTarget } from "../utils/timelineDiscovery"; +import { useCaptionStore } from "../captions/store"; +import { + applyCaptionModelToIframe, + isCaptionPreviewVisible, +} from "../captions/components/CaptionOverlayUtils"; import { shouldIgnoreHistoryShortcut } from "../utils/studioHelpers"; import { canSplitElement } from "../utils/timelineElementSplit"; import { trackStudioEvent } from "../utils/studioTelemetry"; @@ -413,6 +418,23 @@ export function useAppHotkeys({ const applyHistory = useCallback( async (direction: "undo" | "redo") => { + // Caption edits live in their own in-memory stack. While caption edit + // mode is active, ⌘Z must revert the caption edit — not an unrelated + // earlier file edit (which would ALSO leave the caption change intact). + const captionState = useCaptionStore.getState(); + // Only when the caption preview is actually visible: isEditMode stays + // true while the preview is hidden (storyboard view), and eating ⌘Z + // there would pop invisible caption edits instead of file history. + if (captionState.isEditMode && isCaptionPreviewVisible()) { + const restored = direction === "undo" ? captionState.undo() : captionState.redo(); + if (restored) { + applyCaptionModelToIframe(restored); + showToast(`${direction === "undo" ? "Undid" : "Redid"} caption edit`, "info"); + return; + } + // Empty caption stack: fall through to beat/file history as usual. + } + // Beat edits interleave with file history by timestamp; handle them first. if (tryApplyBeatHistory(direction, editHistory.state, showToast)) return; diff --git a/packages/studio/src/hooks/useCaptionDetection.ts b/packages/studio/src/hooks/useCaptionDetection.ts index c16a18a86..243af8901 100644 --- a/packages/studio/src/hooks/useCaptionDetection.ts +++ b/packages/studio/src/hooks/useCaptionDetection.ts @@ -1,4 +1,4 @@ -import { useEffect } from "react"; +import { useEffect, useRef } from "react"; import { useCaptionStore } from "../captions/store"; import { acceptStudioRuntimeMessage } from "../player/lib/runtimeProtocol"; import { useCaptionSync } from "../captions/hooks/useCaptionSync"; @@ -25,6 +25,24 @@ export function useCaptionDetection({ captionSync, setRightCollapsed, }: UseCaptionDetectionParams) { + // Switching compositions must drop the previous comp's caption state — a + // stale model + full-canvas overlay otherwise blocks normal element editing + // on the new composition (and edit mode could never be exited). + const prevCompPathRef = useRef(activeCompPath); + // eslint-disable-next-line no-restricted-syntax + useEffect(() => { + if (prevCompPathRef.current !== activeCompPath) { + prevCompPathRef.current = activeCompPath; + const store = useCaptionStore.getState(); + if (store.model || store.isEditMode) { + // Flush the last debounced caption edit before the reset destroys the + // model — otherwise a save landing after the switch writes nothing. + store.retrySave?.(); + store.reset(); + } + } + }, [activeCompPath]); + // eslint-disable-next-line no-restricted-syntax useEffect(() => { if (!projectId) return; @@ -32,7 +50,9 @@ export function useCaptionDetection({ let activating = false; const tryActivateCaptions = () => { - if (useCaptionStore.getState().isEditMode || activating) { + const captionState = useCaptionStore.getState(); + // `dismissed` = user explicitly exited caption editing; don't re-trap them. + if (captionState.isEditMode || captionState.dismissed || activating) { return; }