diff --git a/packages/studio-server/src/routes/files.test.ts b/packages/studio-server/src/routes/files.test.ts index dfdcdcb9f8..90699abd5a 100644 --- a/packages/studio-server/src/routes/files.test.ts +++ b/packages/studio-server/src/routes/files.test.ts @@ -389,6 +389,145 @@ gsap.set("#box", { rotation: 45 }); expect(payload.content).not.toContain("opacity"); }); + // ── Canvas z-order / patch-target regression suite ──────────────────────── + // A right-click "move to back" on an id-less element (e.g. a caption `.sub` + // div) once serialized `target.id: null`, which findUnsafeDomPatchValues + // rejected as `body.target.id`, bricking the edit. The RULE: `target.id` is + // metadata, not a layout value — a null there is genuinely invalid and stays + // rejected; the fix is that the client omits an absent id instead of sending + // null, so the patch degrades to a hfId / selector + selectorIndex match. + it("rejects a null target.id in a DOM patch (documents the rule)", async () => { + const projectDir = createProjectDir(); + writeFileSync( + join(projectDir, "index.html"), + '
A
B
', + ); + const app = new Hono(); + registerFileRoutes(app, createAdapter(projectDir)); + + const before = readFileSync(join(projectDir, "index.html"), "utf-8"); + const response = await app.request( + "http://localhost/projects/demo/file-mutations/patch-element/index.html", + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + target: { id: null, selector: ".sub", selectorIndex: 1 }, + operations: [{ type: "inline-style", property: "z-index", value: "0" }], + }), + }, + ); + const payload = (await response.json()) as { error?: string; fields?: string[] }; + + expect(response.status).toBe(400); + expect(payload.error).toContain("unsafe values"); + expect(payload.fields).toContain("body.target.id"); + expect(readFileSync(join(projectDir, "index.html"), "utf-8")).toBe(before); + }); + + it("z-reorder with an omitted id degrades to a selector patch (id-less element)", async () => { + const projectDir = createProjectDir(); + writeFileSync( + join(projectDir, "index.html"), + '
A
B
', + ); + const app = new Hono(); + registerFileRoutes(app, createAdapter(projectDir)); + + const response = await app.request( + "http://localhost/projects/demo/file-mutations/patch-element/index.html", + { + method: "POST", + headers: { "Content-Type": "application/json" }, + // id omitted (undefined) — the fixed client shape for an id-less element. + body: JSON.stringify({ + target: { selector: ".sub", selectorIndex: 1 }, + operations: [{ type: "inline-style", property: "z-index", value: "0" }], + }), + }, + ); + const payload = (await response.json()) as { changed?: boolean; content?: string }; + + expect(response.status).toBe(200); + expect(payload.changed).toBe(true); + // The SECOND `.sub` (selectorIndex 1) is the one restacked, not the first. + expect(payload.content).toContain('
A
'); + expect(payload.content).toContain("z-index: 0"); + }); + + it("duplicate-id document: a selector+index patch hits the right element, not a rejection", async () => { + const projectDir = createProjectDir(); + // Two elements share id="main" AND class="root" (mirrors the user's project, + // where sub-compositions each carry id="main"). Match by selector + index. + writeFileSync( + join(projectDir, "index.html"), + '
first
' + + '
second
', + ); + const app = new Hono(); + registerFileRoutes(app, createAdapter(projectDir)); + + const response = await app.request( + "http://localhost/projects/demo/file-mutations/patch-element/index.html", + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + target: { selector: ".root", selectorIndex: 1 }, + operations: [{ type: "inline-style", property: "z-index", value: "0" }], + }), + }, + ); + const payload = (await response.json()) as { changed?: boolean; content?: string }; + + expect(response.status).toBe(200); + expect(payload.changed).toBe(true); + // First "main" untouched; second one restacked. + expect(payload.content).toContain('
first
'); + expect(payload.content).toContain("z-index: 0"); + }); + + // Sibling canvas commits (position / size / text) carry real string ids like + // "v-hero" / "vo-part1" / "main". The guard must accept them — it only rejects + // null / non-finite numbers, never inspects the id string — so these never hit + // the z-order "unsafe values" variant. + it.each([ + { + label: "position", + id: "v-hero", + op: { type: "inline-style", property: "left", value: "40px" }, + }, + { + label: "size", + id: "vo-part1", + op: { type: "inline-style", property: "width", value: "320px" }, + }, + { + label: "text", + id: "main", + op: { type: "text-content", property: "textContent", value: "Hi" }, + }, + ])("accepts a $label commit with a real fixture id ($id)", async ({ id, op }) => { + const projectDir = createProjectDir(); + writeFileSync(join(projectDir, "index.html"), `
x
`); + const app = new Hono(); + registerFileRoutes(app, createAdapter(projectDir)); + + const response = await app.request( + "http://localhost/projects/demo/file-mutations/patch-element/index.html", + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ target: { id }, operations: [op] }), + }, + ); + const payload = (await response.json()) as { changed?: boolean; error?: string }; + + expect(response.status).toBe(200); + expect(payload.error).toBeUndefined(); + expect(payload.changed).toBe(true); + }); + it("update-from-property returns 400 for a non-fromTo animation", async () => { const projectDir = createProjectDir(); const TO_COMP = ``; + + const seqDir = createProjectDir(); + writeHtml(seqDir, "seq.html", TWO_TWEENS); + const seqApp = new Hono(); + registerFileRoutes(seqApp, createAdapter(seqDir)); + await seqApp.request("http://localhost/projects/demo/gsap-mutations/seq.html", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ type: "shift-positions", targetSelector: "#a", delta: 1 }), + }); + const seqRes = await seqApp.request("http://localhost/projects/demo/gsap-mutations/seq.html", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ type: "shift-positions", targetSelector: "#b", delta: 0.5 }), + }); + const seqAfter = ((await seqRes.json()) as { after: string }).after; + + const batchDir = createProjectDir(); + writeHtml(batchDir, "batch.html", TWO_TWEENS); + const batchApp = new Hono(); + registerFileRoutes(batchApp, createAdapter(batchDir)); + const batchRes = await batchApp.request( + "http://localhost/projects/demo/gsap-mutations/batch.html", + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + type: "shift-positions-batch", + shifts: [ + { targetSelector: "#a", delta: 1 }, + { targetSelector: "#b", delta: 0.5 }, + ], + }), + }, + ); + const batch = (await batchRes.json()) as { ok: boolean; changed: boolean; after: string }; + + expect(batchRes.status).toBe(200); + expect(batch.ok).toBe(true); + expect(batch.changed).toBe(true); + // Batching #a then #b in one write == applying them as two sequential single shifts. + expect(batch.after).toBe(seqAfter); + }); + + it("reports no GSAP mutation for shift-positions-batch in a file with no GSAP script", async () => { + // Same contract as its shift-positions / scale-positions siblings: a file with + // no GSAP block is a no-op {ok, changed:false}, not a 400. + const projectDir = createProjectDir(); + const app = new Hono(); + registerFileRoutes(app, createAdapter(projectDir)); + + const res = await app.request("http://localhost/projects/demo/gsap-mutations/index.html", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + type: "shift-positions-batch", + shifts: [{ targetSelector: "#box", delta: 1 }], + }), + }); + const result = (await res.json()) as { ok?: boolean; changed?: boolean; mutated?: boolean }; + + expect(res.status).toBe(200); + expect(result.ok).toBe(true); + expect(result.changed).toBe(false); + expect(result.mutated).toBe(false); + }); + + it("rejects a shift-positions-batch with a missing/non-array `shifts` field (400)", async () => { + const projectDir = createProjectDir(); + writeHtml( + projectDir, + "comp.html", + ``, + ); + const app = new Hono(); + registerFileRoutes(app, createAdapter(projectDir)); + + const res = await app.request("http://localhost/projects/demo/gsap-mutations/comp.html", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ type: "shift-positions-batch" }), + }); + const result = (await res.json()) as { error?: string }; + + expect(res.status).toBe(400); + expect(result.error).toContain("shifts"); + }); }); diff --git a/packages/studio-server/src/routes/files.ts b/packages/studio-server/src/routes/files.ts index 64124957c2..8ad12293ba 100644 --- a/packages/studio-server/src/routes/files.ts +++ b/packages/studio-server/src/routes/files.ts @@ -773,6 +773,13 @@ type GsapMutationRequest = targetSelector: string; delta: number; } + | { + // Batched shift: fold shiftPositionsInScript over N selectors in one write. + // Lets a multi-clip timeline move (ripple / insert) shift every affected + // clip's tweens atomically instead of one racing server round-trip per clip. + type: "shift-positions-batch"; + shifts: Array<{ targetSelector: string; delta: number }>; + } | { type: "scale-positions"; targetSelector: string; @@ -815,6 +822,7 @@ const HOLD_SYNC_MUTATION_TYPES = new Set([ // Time-shift / time-scale tweens, which can move a keyframed position tween's start // across t=0, flipping hold need; stale holds are not repositioned by these ops. "shift-positions", + "shift-positions-batch", "scale-positions", // Retargets keyframed position tweens to a cloned element's selector; the old hold is // keyed to the prior selector, so holds must be rebuilt for the new target. @@ -1101,6 +1109,14 @@ function executeGsapMutationAcorn( if (!targetSelector || !Number.isFinite(delta) || delta === 0) return block.scriptText; return shiftPositionsInScript(block.scriptText, targetSelector, delta); } + case "shift-positions-batch": { + let script = block.scriptText; + for (const s of body.shifts) { + if (!s.targetSelector || !Number.isFinite(s.delta) || s.delta === 0) continue; + script = shiftPositionsInScript(script, s.targetSelector, s.delta); + } + return script; + } case "scale-positions": { const { targetSelector, oldStart, oldDuration, newStart, newDuration } = body; if ( @@ -1463,6 +1479,15 @@ async function executeGsapMutationRecast( const { shiftPositionsInScript } = parser; return shiftPositionsInScript(block.scriptText, targetSelector, delta); } + case "shift-positions-batch": { + const { shiftPositionsInScript } = parser; + let script = block.scriptText; + for (const s of body.shifts) { + if (!s.targetSelector || !Number.isFinite(s.delta) || s.delta === 0) continue; + script = shiftPositionsInScript(script, s.targetSelector, s.delta); + } + return script; + } case "scale-positions": { const { targetSelector, oldStart, oldDuration, newStart, newDuration } = body; if ( @@ -2024,6 +2049,11 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void { if (unsafeFields.length > 0) { return rejectUnsafeMutationValues(c, unsafeFields); } + // Defensive shape guard: a malformed shift-positions-batch (missing/non-array + // `shifts`) would otherwise TypeError inside `for (const s of body.shifts)`. + if (body.type === "shift-positions-batch" && !Array.isArray(body.shifts)) { + return c.json({ error: "shift-positions-batch requires a `shifts` array" }, 400); + } let html = readFileSync(res.absPath, "utf-8"); let block = extractGsapScriptBlock(html); @@ -2046,7 +2076,12 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void { } block = extractGsapScriptBlock(html); } - if (!block && (body.type === "shift-positions" || body.type === "scale-positions")) { + if ( + !block && + (body.type === "shift-positions" || + body.type === "scale-positions" || + body.type === "shift-positions-batch") + ) { return c.json({ ok: true, changed: false, diff --git a/packages/studio/src/player/components/timelineZoom.test.ts b/packages/studio/src/player/components/timelineZoom.test.ts index 3e4a35cac5..8b2fc00572 100644 --- a/packages/studio/src/player/components/timelineZoom.test.ts +++ b/packages/studio/src/player/components/timelineZoom.test.ts @@ -1,12 +1,15 @@ import { describe, expect, it } from "vitest"; import { clampTimelineZoomPercent, + computePinnedZoomPercent, getNextTimelineZoomPercent, getPinchTimelineZoomPercent, getTimelinePixelsPerSecond, getTimelineZoomPercent, MAX_TIMELINE_ZOOM_PERCENT, MIN_TIMELINE_ZOOM_PERCENT, + timelineZoomPercentToSlider, + timelineSliderToZoomPercent, } from "./timelineZoom"; describe("clampTimelineZoomPercent", () => { @@ -81,3 +84,67 @@ describe("getPinchTimelineZoomPercent", () => { expect(getPinchTimelineZoomPercent(-10000, "manual", 100)).toBe(MAX_TIMELINE_ZOOM_PERCENT); }); }); + +describe("timelineZoomPercentToSlider", () => { + it("maps min zoom to slider position 0", () => { + expect(timelineZoomPercentToSlider(MIN_TIMELINE_ZOOM_PERCENT)).toBeCloseTo(0, 5); + }); + + it("maps max zoom to slider position 100", () => { + expect(timelineZoomPercentToSlider(MAX_TIMELINE_ZOOM_PERCENT)).toBeCloseTo(100, 5); + }); + + it("maps 100% to the log midpoint between 10 and 2000", () => { + const expected = ((Math.log(100) - Math.log(10)) / (Math.log(2000) - Math.log(10))) * 100; + expect(timelineZoomPercentToSlider(100)).toBeCloseTo(expected, 3); + }); +}); + +describe("timelineSliderToZoomPercent", () => { + it("maps slider 0 to min zoom", () => { + expect(timelineSliderToZoomPercent(0)).toBe(MIN_TIMELINE_ZOOM_PERCENT); + }); + + it("maps slider 100 to max zoom", () => { + expect(timelineSliderToZoomPercent(100)).toBe(MAX_TIMELINE_ZOOM_PERCENT); + }); +}); + +describe("computePinnedZoomPercent", () => { + it("returns 100 when current pps equals the fit pps (a no-op pin at the current fit)", () => { + expect(computePinnedZoomPercent(42, 42)).toBe(100); + }); + + it("reproduces the current pps: percent × fitPps / 100 === currentPps", () => { + const fitPps = 20; + const currentPps = 50; // user zoomed in 2.5× + const percent = computePinnedZoomPercent(currentPps, fitPps); + expect(percent).toBe(250); + // Round-trips through getTimelinePixelsPerSecond back to the on-screen pps. + expect(getTimelinePixelsPerSecond(fitPps, "manual", percent)).toBeCloseTo(currentPps, 5); + }); + + it("clamps a pin that would exceed the manual-zoom bounds", () => { + // currentPps 10000 / fitPps 1 = 1_000_000% → clamped to MAX. + expect(computePinnedZoomPercent(10000, 1)).toBe(MAX_TIMELINE_ZOOM_PERCENT); + // Tiny ratio → clamped up to MIN. + expect(computePinnedZoomPercent(0.001, 1000)).toBe(MIN_TIMELINE_ZOOM_PERCENT); + }); + + it("falls back to 100 for unusable inputs (a safe no-op pin)", () => { + expect(computePinnedZoomPercent(Number.NaN, 20)).toBe(100); + expect(computePinnedZoomPercent(50, 0)).toBe(100); + expect(computePinnedZoomPercent(-5, 20)).toBe(100); + expect(computePinnedZoomPercent(50, Number.POSITIVE_INFINITY)).toBe(100); + }); +}); + +describe("timelineZoomPercentToSlider / timelineSliderToZoomPercent round-trip", () => { + for (const percent of [10, 100, 500, 2000]) { + it(`round-trips ${percent}% within ±1%`, () => { + const slider = timelineZoomPercentToSlider(percent); + const back = timelineSliderToZoomPercent(slider); + expect(Math.abs(back - percent) / percent).toBeLessThan(0.01); + }); + } +}); diff --git a/packages/studio/src/player/components/timelineZoom.ts b/packages/studio/src/player/components/timelineZoom.ts index 0ac4a92b27..15baa15667 100644 --- a/packages/studio/src/player/components/timelineZoom.ts +++ b/packages/studio/src/player/components/timelineZoom.ts @@ -18,6 +18,34 @@ export function getTimelineZoomPercent(zoomMode: ZoomMode, manualZoomPercent: nu return zoomMode === "fit" ? 100 : clampTimelineZoomPercent(manualZoomPercent); } +/** + * The manual-zoom percent that, applied to `fitPixelsPerSecond`, reproduces the + * CURRENT on-screen pixels-per-second exactly. Used to PIN the timeline zoom on + * the first edit so a duration change (which recomputes fit-pps) no longer + * rescales every clip: we switch `zoomMode` to "manual" with this percent, so + * `getTimelinePixelsPerSecond` keeps returning today's pps regardless of the new + * fit basis. + * + * Since `pps = fitPps * (percent / 100)` in manual mode, and while fitting + * `pps === fitPps`, the pinned percent is `currentPps / fitPps * 100`. Clamped to + * the manual-zoom range so the pin can't land outside the slider's bounds; falls + * back to 100 (a no-op pin at the current fit) when either input is unusable. + */ +export function computePinnedZoomPercent( + currentPixelsPerSecond: number, + fitPixelsPerSecond: number, +): number { + if ( + !Number.isFinite(currentPixelsPerSecond) || + currentPixelsPerSecond <= 0 || + !Number.isFinite(fitPixelsPerSecond) || + fitPixelsPerSecond <= 0 + ) { + return 100; + } + return clampTimelineZoomPercent((currentPixelsPerSecond / fitPixelsPerSecond) * 100); +} + export function getTimelinePixelsPerSecond( fitPixelsPerSecond: number, zoomMode: ZoomMode, @@ -47,3 +75,26 @@ export function getPinchTimelineZoomPercent( if (!Number.isFinite(deltaY) || deltaY === 0) return current; return clampTimelineZoomPercent(current * Math.exp(-deltaY * PINCH_ZOOM_SENSITIVITY)); } + +const LOG_MIN = Math.log(MIN_TIMELINE_ZOOM_PERCENT); +const LOG_MAX = Math.log(MAX_TIMELINE_ZOOM_PERCENT); + +/** + * Maps a zoom percent (10–2000) to a slider position (0–100) using a log scale. + * Log scale is used because the range spans 200×; linear would compress the + * low end (10–100%) into a tiny sliver of the slider. + */ +export function timelineZoomPercentToSlider(percent: number): number { + const clamped = clampTimelineZoomPercent(percent); + return ((Math.log(clamped) - LOG_MIN) / (LOG_MAX - LOG_MIN)) * 100; +} + +/** + * Maps a slider position (0–100) to a zoom percent (10–2000) using a log scale. + * Inverse of `timelineZoomPercentToSlider`. + */ +export function timelineSliderToZoomPercent(slider: number): number { + const clampedSlider = Math.max(0, Math.min(100, slider)); + const logValue = LOG_MIN + (clampedSlider / 100) * (LOG_MAX - LOG_MIN); + return clampTimelineZoomPercent(Math.exp(logValue)); +} diff --git a/packages/studio/src/player/hooks/useTimelineSyncCallbacks.test.ts b/packages/studio/src/player/hooks/useTimelineSyncCallbacks.test.ts new file mode 100644 index 0000000000..1b0a9c710c --- /dev/null +++ b/packages/studio/src/player/hooks/useTimelineSyncCallbacks.test.ts @@ -0,0 +1,219 @@ +// @vitest-environment happy-dom +import { describe, expect, it } from "vitest"; +import { + resolveReloadSeekTime, + resolveTimelineTotalDuration, + revealIframe, +} from "./useTimelineSyncCallbacks"; +import { readTimelineDurationFromDocument } from "../lib/timelineDOM"; + +// Minimal stand-in — revealIframe only touches `style.visibility`. Avoids +// depending on a DOM environment (this test file runs under node). +function fakeIframe(visibility: string): HTMLIFrameElement { + return { style: { visibility } } as unknown as HTMLIFrameElement; +} + +describe("revealIframe", () => { + it("clears a hidden iframe's visibility (undoes refreshPlayer's hide)", () => { + const iframe = fakeIframe("hidden"); + revealIframe(iframe); + expect(iframe.style.visibility).toBe(""); + }); + + it("leaves an already-visible iframe untouched (idempotent)", () => { + const iframe = fakeIframe(""); + revealIframe(iframe); + expect(iframe.style.visibility).toBe(""); + }); + + it("no-ops on a null iframe", () => { + expect(() => revealIframe(null)).not.toThrow(); + }); +}); + +describe("resolveReloadSeekTime", () => { + it("restores the pending seek saved by refreshPlayer (the primary reload path)", () => { + expect( + resolveReloadSeekTime({ + pendingSeek: 7.2, + requestedSeek: null, + storeCurrentTime: 7.2, + duration: 20, + }), + ).toBe(7.2); + }); + + it("honors a deep-link seek request when no pending seek exists", () => { + expect( + resolveReloadSeekTime({ + pendingSeek: null, + requestedSeek: 12.5, + storeCurrentTime: 0, + duration: 20, + }), + ).toBe(12.5); + }); + + it("THE BUG: a second overlapping reload (pending seek already consumed) restores the store playhead, not 0", () => { + // Drop → reload #1 consumes pendingSeek and seeks/syncs to 7.2. A staggered + // second reload (refreshPreviewDocumentVersion 80/300ms bumps) then finds the + // slot empty — the old code reset the playhead to 0 here. + expect( + resolveReloadSeekTime({ + pendingSeek: null, + requestedSeek: null, + storeCurrentTime: 7.2, + duration: 20, + }), + ).toBe(7.2); + }); + + it("fresh project load starts at 0 (store resets currentTime on project switch)", () => { + expect( + resolveReloadSeekTime({ + pendingSeek: null, + requestedSeek: null, + storeCurrentTime: 0, + duration: 20, + }), + ).toBe(0); + }); + + it("clamps to duration when content shrank past the playhead (the one sanctioned move)", () => { + expect( + resolveReloadSeekTime({ + pendingSeek: 18, + requestedSeek: null, + storeCurrentTime: 18, + duration: 9, + }), + ).toBe(9); + }); + + it("a pending seek of 0 is an explicit position, not a missing value", () => { + expect( + resolveReloadSeekTime({ + pendingSeek: 0, + requestedSeek: 12, + storeCurrentTime: 5, + duration: 20, + }), + ).toBe(0); + }); + + it("returns the guarded target unclamped when the duration is non-finite (no seek(NaN))", () => { + // Mid-reload the adapter can report a NaN duration; Math.min(target, NaN) would + // be NaN and seek(NaN). The guarded target must pass through unclamped instead. + expect( + resolveReloadSeekTime({ + pendingSeek: 7.2, + requestedSeek: null, + storeCurrentTime: 7.2, + duration: Number.NaN, + }), + ).toBe(7.2); + // A zero/negative duration is equally unusable — pass the target through. + expect( + resolveReloadSeekTime({ + pendingSeek: 7.2, + requestedSeek: null, + storeCurrentTime: 7.2, + duration: 0, + }), + ).toBe(7.2); + }); + + it("guards against non-finite and negative targets", () => { + expect( + resolveReloadSeekTime({ + pendingSeek: Number.NaN, + requestedSeek: null, + storeCurrentTime: 5, + duration: 20, + }), + ).toBe(0); + expect( + resolveReloadSeekTime({ + pendingSeek: -3, + requestedSeek: null, + storeCurrentTime: 5, + duration: 20, + }), + ).toBe(0); + }); +}); + +describe("resolveTimelineTotalDuration", () => { + it("THE BUG: a clip-manifest total shorter than the authored root duration never wins (stale '0:44/0:40')", () => { + // Fixture shape: root data-duration=44.5, furthest clip ends at 40. A runtime + // that measures the manifest from the furthest clip end reports 40s; playback + // still runs the full 44.5s, so the transport total must stay 44.5, not 40. + expect( + resolveTimelineTotalDuration({ + manifestDurationSeconds: 40, + authoredRootDurationSeconds: 44.5, + }), + ).toBe(44.5); + }); + + it("lets clips extend the total PAST the authored root (content can grow the timeline)", () => { + expect( + resolveTimelineTotalDuration({ + manifestDurationSeconds: 50, + authoredRootDurationSeconds: 44.5, + }), + ).toBe(50); + }); + + it("falls back to the manifest total when the root authors no duration", () => { + expect( + resolveTimelineTotalDuration({ + manifestDurationSeconds: 40, + authoredRootDurationSeconds: 0, + }), + ).toBe(40); + }); + + it("uses the authored root when the manifest is loop-inflated / non-finite", () => { + expect( + resolveTimelineTotalDuration({ + manifestDurationSeconds: Number.POSITIVE_INFINITY, + authoredRootDurationSeconds: 44.5, + }), + ).toBe(44.5); + expect( + resolveTimelineTotalDuration({ + manifestDurationSeconds: 9000, // beyond the 7200s sanity cap + authoredRootDurationSeconds: 44.5, + }), + ).toBe(44.5); + }); + + it("returns 0 when neither source yields a usable duration", () => { + expect( + resolveTimelineTotalDuration({ + manifestDurationSeconds: Number.NaN, + authoredRootDurationSeconds: -1, + }), + ).toBe(0); + }); + + it("floors the manifest at the authored root read from the fixture's DOM shape", () => { + // Reconstruct the fixture: root authored at 44.5s, last clip (v-letters) + // ends at 34 + 6 = 40s. readTimelineDurationFromDocument must report the + // authored 44.5, and flooring the 40s manifest total against it yields 44.5. + const doc = document.implementation.createHTMLDocument("fixture"); + doc.body.innerHTML = + '
' + + '' + + "
"; + const authoredRootDurationSeconds = readTimelineDurationFromDocument(doc); + expect(authoredRootDurationSeconds).toBe(44.5); + expect( + resolveTimelineTotalDuration({ + manifestDurationSeconds: 1200 / 30, // durationInFrames measured from clip end + authoredRootDurationSeconds, + }), + ).toBe(44.5); + }); +}); diff --git a/packages/studio/src/player/hooks/useTimelineSyncCallbacks.ts b/packages/studio/src/player/hooks/useTimelineSyncCallbacks.ts index c943680b09..4992efb5c0 100644 --- a/packages/studio/src/player/hooks/useTimelineSyncCallbacks.ts +++ b/packages/studio/src/player/hooks/useTimelineSyncCallbacks.ts @@ -19,6 +19,7 @@ import { createImplicitTimelineLayersFromDOM, buildStandaloneRootTimelineElement, getTimelineElementSelector, + readTimelineDurationFromDocument, } from "../lib/timelineDOM"; import { normalizePreviewViewport, @@ -41,6 +42,76 @@ interface UseTimelineSyncCallbacksParams { applyPreviewAudioState: () => void; } +/** + * Where should the player seek when the preview (re)loads? + * Priority: explicit pending seek (saved by refreshPlayer right before a + * reload) → store-level seek request (deep-link `?t=` hydration) → the store's + * last known playhead. The last fallback makes the playhead RELOAD-INVARIANT: + * edits persist + reload the preview, sometimes more than once (App's + * refreshPreviewDocumentVersion staggers extra bumps at 80/300ms), and the + * consume-once pendingSeekRef meant any reload after the first found the slot + * empty and reset the playhead to 0 — the "dropped a file and the playhead + * jumped to 0" bug. Falling back to the store's playhead means every reload + * restores position; a fresh project load still starts at 0 because the store + * resets currentTime on project switch. Invariant: an edit NEVER moves the + * playhead (the clamp below is the one sanctioned move — content shrank past it). + */ +/** + * Undo the `visibility: hidden` that refreshPlayer sets across a full reload. + * Safe to call when the iframe was never hidden (idempotent no-op). Every reload + * completion + failure path funnels through here so the preview can never get + * stuck invisible. + */ +export function revealIframe(iframe: HTMLIFrameElement | null): void { + if (iframe && iframe.style.visibility === "hidden") { + iframe.style.visibility = ""; + } +} + +export function resolveReloadSeekTime(input: { + pendingSeek: number | null; + requestedSeek: number | null; + storeCurrentTime: number; + duration: number; +}): number { + const target = input.pendingSeek ?? input.requestedSeek ?? input.storeCurrentTime; + if (!Number.isFinite(target) || target <= 0) return 0; + // Only clamp to duration when it's a usable positive number. A non-finite or + // non-positive duration (e.g. the adapter reports NaN mid-reload) would turn + // Math.min(target, NaN) into NaN and seek(NaN); return the guarded target + // unclamped instead so the playhead lands at the intended position. + if (!Number.isFinite(input.duration) || input.duration <= 0) return target; + return Math.min(target, input.duration); +} + +/** Reject non-finite, non-positive, and absurdly large (loop-inflated) values. */ +function sanitizeDurationSeconds(value: number): number { + return Number.isFinite(value) && value > 0 && value < 7200 ? value : 0; +} + +/** + * The transport TOTAL a clip-manifest message should write to the store. + * + * The manifest's `durationInFrames` measures the runtime timeline; some runtimes + * report only the furthest clip end and ignore the root composition's authored + * `data-duration`. When that manifest total is SHORTER than the authored root + * duration, writing it makes the readout stale (playback still runs the full + * authored window — the user saw "0:44/0:40" on a root authored at 44.5s whose + * last clip ends at 40s). The authored root duration is the floor for the total, + * so the readout can never sit below what the file declares. A manifest total + * that is LONGER (clips extend past the root) still wins — content can only grow + * the timeline, never shrink it below the authored window. + */ +export function resolveTimelineTotalDuration(input: { + manifestDurationSeconds: number; + authoredRootDurationSeconds: number; +}): number { + return Math.max( + sanitizeDurationSeconds(input.manifestDurationSeconds), + sanitizeDurationSeconds(input.authoredRootDurationSeconds), + ); +} + export function useTimelineSyncCallbacks({ iframeRef, probeIntervalRef, @@ -154,8 +225,14 @@ export function useTimelineSyncCallbacks({ const rawDuration = data.durationInFrames / 30; // Clamp non-finite or absurdly large durations — the runtime can emit // Infinity when it detects a loop-inflated GSAP timeline without an - // explicit data-duration on the root composition. - const newDuration = Number.isFinite(rawDuration) && rawDuration < 7200 ? rawDuration : 0; + // explicit data-duration on the root composition. Floor the manifest total + // at the authored root `data-duration` so a runtime that measures only the + // furthest clip end (shorter than the authored window) can't leave a stale, + // too-short total in the transport (the "0:44/0:40" bug). + const newDuration = resolveTimelineTotalDuration({ + manifestDurationSeconds: rawDuration, + authoredRootDurationSeconds: readTimelineDurationFromDocument(iframeDoc), + }); const effectiveDuration = newDuration > 0 ? newDuration : usePlayerStore.getState().duration; const clampedEls = effectiveDuration > 0 @@ -218,12 +295,32 @@ export function useTimelineSyncCallbacks({ // never reaches pendingSeekRef). Reconciling with the store here is what makes a // deep-linked `?t=` land instead of starting at 0. const storeSeek = usePlayerStore.getState().requestedSeekTime; - const seekTo = pendingSeekRef.current ?? storeSeek; + const startTime = resolveReloadSeekTime({ + pendingSeek: pendingSeekRef.current, + requestedSeek: storeSeek, + storeCurrentTime: usePlayerStore.getState().currentTime, + duration: adapter.getDuration(), + }); pendingSeekRef.current = null; if (storeSeek != null) usePlayerStore.getState().clearSeekRequest(); - const startTime = seekTo != null ? Math.min(seekTo, adapter.getDuration()) : 0; + // Force a REAL render at startTime, not a no-op. After a post-edit reload the + // freshly rebuilt GSAP timeline can already report being at `startTime` + // internally (the reload restores the same playhead), so a single + // `adapter.seek(startTime)` is a GSAP no-op — `tl.seek(t)` at the current time + // doesn't re-evaluate. That's why a just-dropped clip stayed invisible until + // the user nudged the playhead: its element's state was never applied at the + // restore position. Seeking to a DIFFERENT guard value first (a hair off, or 0 + // when startTime is already ~0) guarantees the follow-up seek to `startTime` + // crosses a time boundary and re-renders every clip — including the new one. + const guardTime = startTime > 0.001 ? Math.max(0, startTime - 0.001) : 0.001; + adapter.seek(guardTime); adapter.seek(startTime); + // The correct frame is now rendered — reveal the iframe that refreshPlayer hid + // for the reload, so the user sees the restored frame directly (never the raw + // all-clips DOM). Cleared unconditionally: any later failure path must not leave + // the preview stuck invisible. + revealIframe(iframeRef.current); // Keep non-React listeners such as the capture link and time display in sync // with the initial adapter seek on iframe load. liveTime.notify(startTime); @@ -332,6 +429,9 @@ export function useTimelineSyncCallbacks({ trySettle(); } window.removeEventListener("message", onMessage); + // Never leave the preview stuck invisible if the runtime never settled + // (initializeAdapter reveals on success; this covers the give-up case). + revealIframe(iframeRef.current); }, 5000) as unknown as ReturnType; }, [initializeAdapter, iframeRef, probeIntervalRef, applyPreviewAudioState]); diff --git a/packages/studio/src/utils/studioUiPreferences.test.ts b/packages/studio/src/utils/studioUiPreferences.test.ts index c5dbd50968..5e335cf5d3 100644 --- a/packages/studio/src/utils/studioUiPreferences.test.ts +++ b/packages/studio/src/utils/studioUiPreferences.test.ts @@ -50,3 +50,41 @@ describe("studio UI preferences", () => { }); }); }); + +describe("timelineSnapEnabled preference", () => { + it("round-trips through storage", () => { + const storage = createStorage(); + writeStudioUiPreferences({ timelineSnapEnabled: false }, storage); + expect(readStudioUiPreferences(storage).timelineSnapEnabled).toBe(false); + }); + + it("ignores non-boolean values", () => { + const storage = createStorage(); + storage.setItem("hf-studio-ui-preferences", JSON.stringify({ timelineSnapEnabled: "yes" })); + expect(readStudioUiPreferences(storage).timelineSnapEnabled).toBeUndefined(); + }); +}); + +describe("timeline zoom pin persistence", () => { + it("round-trips a pinned manual zoom (survives the post-edit reload)", () => { + const storage = createStorage(); + writeStudioUiPreferences( + { timelineZoomMode: "manual", timelineManualZoomPercent: 250 }, + storage, + ); + const prefs = readStudioUiPreferences(storage); + expect(prefs.timelineZoomMode).toBe("manual"); + expect(prefs.timelineManualZoomPercent).toBe(250); + }); + + it("ignores an invalid zoom mode and a non-finite percent", () => { + const storage = createStorage(); + storage.setItem( + "hf-studio-ui-preferences", + JSON.stringify({ timelineZoomMode: "zoomy", timelineManualZoomPercent: "big" }), + ); + const prefs = readStudioUiPreferences(storage); + expect(prefs.timelineZoomMode).toBeUndefined(); + expect(prefs.timelineManualZoomPercent).toBeUndefined(); + }); +}); diff --git a/packages/studio/src/utils/studioUiPreferences.ts b/packages/studio/src/utils/studioUiPreferences.ts index 13804959a6..6548a58eba 100644 --- a/packages/studio/src/utils/studioUiPreferences.ts +++ b/packages/studio/src/utils/studioUiPreferences.ts @@ -16,6 +16,18 @@ export interface StudioUiPreferences { gridVisible?: boolean; gridSpacing?: number; snapToGrid?: boolean; + /** Timeline magnet: snap clip drags/trims/drops to playhead, clip edges, and beats. */ + timelineSnapEnabled?: boolean; + /** Transport + ruler readout mode: timecode or frame number. */ + timeDisplayMode?: "time" | "frame"; + /** + * Timeline zoom mode. Persisted so a zoom PINNED on the first edit survives the + * post-edit iframe reload — otherwise the store reset to "fit" and the duration + * change rescaled every clip (the blink-fix's rescale symptom). + */ + timelineZoomMode?: "fit" | "manual"; + /** Manual timeline zoom percent, paired with `timelineZoomMode: "manual"`. */ + timelineManualZoomPercent?: number; } const STUDIO_UI_PREFERENCES_KEY = "hf-studio-ui-preferences"; @@ -88,6 +100,21 @@ function readStorage(storage: Storage | null): StudioUiPreferences { if (typeof parsed.snapToGrid === "boolean") { preferences.snapToGrid = parsed.snapToGrid; } + if (typeof parsed.timelineSnapEnabled === "boolean") { + preferences.timelineSnapEnabled = parsed.timelineSnapEnabled; + } + if (parsed.timeDisplayMode === "time" || parsed.timeDisplayMode === "frame") { + preferences.timeDisplayMode = parsed.timeDisplayMode; + } + if (parsed.timelineZoomMode === "fit" || parsed.timelineZoomMode === "manual") { + preferences.timelineZoomMode = parsed.timelineZoomMode; + } + if ( + typeof parsed.timelineManualZoomPercent === "number" && + Number.isFinite(parsed.timelineManualZoomPercent) + ) { + preferences.timelineManualZoomPercent = parsed.timelineManualZoomPercent; + } return preferences; } catch { return {}; diff --git a/packages/studio/src/utils/timelineInspector.test.ts b/packages/studio/src/utils/timelineInspector.test.ts new file mode 100644 index 0000000000..318f3cbb02 --- /dev/null +++ b/packages/studio/src/utils/timelineInspector.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from "vitest"; +import { isAudioTimelineElement, isMusicTrack, resolveBeatSourceTrack } from "./timelineInspector"; +import type { TimelineElement } from "../player"; + +// Minimal element factory for tests +function el( + overrides: Partial< + Pick + >, +): Pick { + return { + tag: "audio", + src: "assets/track.mp3", + id: "el-1", + domId: "el-1", + timelineRole: undefined, + duration: 10, + ...overrides, + }; +} + +describe("isAudioTimelineElement", () => { + it("is true for audio tag", () => { + expect(isAudioTimelineElement(el({ tag: "audio" }))).toBe(true); + }); + + it("is true for music/sfx/narration semantic tags", () => { + expect(isAudioTimelineElement(el({ tag: "music" }))).toBe(true); + expect(isAudioTimelineElement(el({ tag: "sfx" }))).toBe(true); + expect(isAudioTimelineElement(el({ tag: "narration" }))).toBe(true); + }); + + it("is true for img/div with an audio src extension", () => { + expect(isAudioTimelineElement(el({ tag: "div", src: "assets/bg.mp3" }))).toBe(true); + expect(isAudioTimelineElement(el({ tag: "div", src: "assets/fx.wav" }))).toBe(true); + }); + + it("is false for video and image elements", () => { + expect(isAudioTimelineElement(el({ tag: "video", src: "assets/clip.mp4" }))).toBe(false); + expect(isAudioTimelineElement(el({ tag: "img", src: "assets/logo.svg" }))).toBe(false); + }); + + it("returns false for null/undefined", () => { + expect(isAudioTimelineElement(null)).toBe(false); + expect(isAudioTimelineElement(undefined)).toBe(false); + }); +}); + +describe("isMusicTrack", () => { + it("is true when timelineRole is 'music'", () => { + expect(isMusicTrack(el({ timelineRole: "music" }))).toBe(true); + }); + + it("is false for explicit non-music roles", () => { + expect(isMusicTrack(el({ timelineRole: "sfx" }))).toBe(false); + expect(isMusicTrack(el({ timelineRole: "voiceover" }))).toBe(false); + }); + + it("matches music-like ids when no role is set", () => { + expect(isMusicTrack(el({ domId: "bgm", timelineRole: undefined }))).toBe(true); + expect(isMusicTrack(el({ domId: "background_music", timelineRole: undefined }))).toBe(true); + expect(isMusicTrack(el({ domId: "soundtrack", timelineRole: undefined }))).toBe(true); + }); + + it("is false for generic ids with no role", () => { + expect(isMusicTrack(el({ domId: "my_audio_file", timelineRole: undefined }))).toBe(false); + expect(isMusicTrack(el({ domId: "drop_1", timelineRole: undefined }))).toBe(false); + }); + + it("is false for non-audio elements even with a music-like id", () => { + expect(isMusicTrack(el({ tag: "img", src: "assets/logo.svg", domId: "bgm" }))).toBe(false); + }); +}); + +describe("resolveBeatSourceTrack", () => { + it("returns null when there are no elements", () => { + expect(resolveBeatSourceTrack([])).toBeNull(); + }); + + it("returns null when there are no audio elements", () => { + const elements = [el({ tag: "img", src: "assets/logo.png" })]; + expect(resolveBeatSourceTrack(elements)).toBeNull(); + }); + + it("returns isFallback=false for an explicitly tagged music track", () => { + const music = el({ timelineRole: "music" }); + const result = resolveBeatSourceTrack([music]); + expect(result).not.toBeNull(); + expect(result!.isFallback).toBe(false); + expect(result!.element).toBe(music); + }); + + it("prefers the explicit music track over a longer untagged clip", () => { + const music = el({ timelineRole: "music", duration: 30 }); + const other = el({ id: "drop_1", domId: "drop_1", timelineRole: undefined, duration: 120 }); + const result = resolveBeatSourceTrack([music, other]); + expect(result!.element).toBe(music); + expect(result!.isFallback).toBe(false); + }); + + it("falls back to the longest untagged audio clip when no music track exists", () => { + const short = el({ id: "drop_1", domId: "drop_1", duration: 5, timelineRole: undefined }); + const long = el({ id: "drop_2", domId: "drop_2", duration: 60, timelineRole: undefined }); + const result = resolveBeatSourceTrack([short, long]); + expect(result).not.toBeNull(); + expect(result!.isFallback).toBe(true); + expect(result!.element).toBe(long); + }); + + it("excludes explicitly non-music roles (sfx, voiceover) from the fallback", () => { + const sfx = el({ id: "sfx_1", domId: "sfx_1", timelineRole: "sfx", duration: 30 }); + const vo = el({ id: "vo_1", domId: "vo_1", timelineRole: "voiceover", duration: 20 }); + expect(resolveBeatSourceTrack([sfx, vo])).toBeNull(); + }); + + it("returns isFallback=true for an untagged clip with a non-music id", () => { + const clip = el({ id: "my_audio_file", domId: "my_audio_file", timelineRole: undefined }); + const result = resolveBeatSourceTrack([clip]); + expect(result!.isFallback).toBe(true); + }); +}); diff --git a/packages/studio/src/utils/timelineInspector.ts b/packages/studio/src/utils/timelineInspector.ts index b1297e1900..4a33e8fc2f 100644 --- a/packages/studio/src/utils/timelineInspector.ts +++ b/packages/studio/src/utils/timelineInspector.ts @@ -4,7 +4,7 @@ const AUDIO_TIMELINE_TAGS = new Set(["audio", "music", "sfx", "sound", "narratio const AUDIO_SOURCE_EXT_RE = /\.(aac|flac|m4a|mp3|ogg|opus|wav)(?:[?#].*)?$/i; const MUSIC_ID_RE = /\b(music|bgm|soundtrack|background[-_]?music)\b/i; -function isAudioTimelineElement( +export function isAudioTimelineElement( element: Pick | null | undefined, ): boolean { if (!element) return false; @@ -29,3 +29,34 @@ export function isMusicTrack( const id = element.domId ?? element.id ?? ""; return MUSIC_ID_RE.test(id); } + +/** + * Resolve the best audio source for beat analysis. An explicitly tagged or + * named music track wins; when none is present (e.g. an audio file dropped + * from Finder with a generic id), the LONGEST untagged audio clip is used as a + * fallback. Ties on duration resolve to the FIRST such clip encountered (the loop + * keeps the current best on `>` only), i.e. discovery/DOM order wins. + * Returns the element and whether it was found via the fallback path. + * + * The `isMusicTrack` predicate is unchanged so beat-snap and drag-exclusion + * logic remain unaffected by this fallback. + */ +export function resolveBeatSourceTrack( + elements: readonly Pick< + TimelineElement, + "tag" | "src" | "id" | "domId" | "timelineRole" | "duration" + >[], +): { element: (typeof elements)[number]; isFallback: boolean } | null { + const explicit = elements.find(isMusicTrack); + if (explicit) return { element: explicit, isFallback: false }; + + // Fallback: pick the longest audio clip (skipping explicitly non-music roles + // like "sfx" or "voiceover" to avoid triggering beat analysis on those). + let best: (typeof elements)[number] | null = null; + for (const el of elements) { + if (!isAudioTimelineElement(el)) continue; + if (el.timelineRole && el.timelineRole !== "music") continue; + if (!best || el.duration > best.duration) best = el; + } + return best ? { element: best, isFallback: true } : null; +}