From 1a63a945e012f74965ca1f38245075c491417351 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Mon, 18 May 2026 18:28:34 -0400 Subject: [PATCH] =?UTF-8?q?fix(studio):=20address=20review=20=E2=80=94=20G?= =?UTF-8?q?SAP-only=20fallback,=20drop=20dead=20alias,=20add=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - [blocker] When playerAdapter is null (GSAP-only runtimes with no win.__player), the fallback path now uses the best available timeline adapter instead of returning null. Track timelineAdapter across the __timeline and __timelines paths, then use it as the base for createStaticSeekPlaybackAdapter. - [nit] Remove dead baseAdapter alias — use bestAdapter directly. - [tests] Add 4 tests: readTimelineDurationFromDocument with data-hf-authored-duration fallback, createStaticSeekPlaybackAdapter with seek-only adapter (no renderSeek), and pause lifecycle. --- .filesize-allowlist | 1 + .../player/hooks/useTimelinePlayer.test.ts | 55 +++++++++++++++++++ .../src/player/hooks/useTimelinePlayer.ts | 22 +++++--- 3 files changed, 70 insertions(+), 8 deletions(-) diff --git a/.filesize-allowlist b/.filesize-allowlist index b4dbc51b4..c4845eb51 100644 --- a/.filesize-allowlist +++ b/.filesize-allowlist @@ -2,4 +2,5 @@ packages/studio/src/player/hooks/useTimelinePlayer.ts packages/studio/src/hooks/useManifestPersistence.ts packages/studio/src/player/components/PlayerControls.tsx packages/studio/src/components/editor/manualEdits.test.ts +packages/studio/src/player/hooks/useTimelinePlayer.test.ts packages/studio/src/components/editor/manualEditsDom.ts diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.test.ts b/packages/studio/src/player/hooks/useTimelinePlayer.test.ts index 672bc576a..f83ea6ef4 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.test.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.test.ts @@ -106,6 +106,28 @@ describe("readTimelineDurationFromDocument", () => { expect(readTimelineDurationFromDocument(doc)).toBe(5.5); }); + + it("reads data-hf-authored-duration when data-duration is stripped", () => { + const doc = createDocument(` +
+
+
+
+ `); + + expect(readTimelineDurationFromDocument(doc)).toBe(70); + }); + + it("picks the larger of data-duration and data-hf-authored-duration children", () => { + const doc = createDocument(` +
+
+
+
+ `); + + expect(readTimelineDurationFromDocument(doc)).toBe(82); + }); }); describe("createStaticSeekPlaybackAdapter", () => { @@ -153,6 +175,39 @@ describe("createStaticSeekPlaybackAdapter", () => { expect(renderedTimes).toEqual([2]); expect(adapter.getTime()).toBe(2); }); + + it("works with a seek-only adapter (no renderSeek)", () => { + const clock = createManualAnimationClock(); + const seekedTimes: number[] = []; + const adapter = createStaticSeekPlaybackAdapter( + { + getTime: () => 0, + seek: (time: number) => { + seekedTimes.push(time); + }, + }, + 82, + clock, + ); + + adapter.seek(77); + expect(seekedTimes).toEqual([77]); + expect(adapter.getTime()).toBe(77); + expect(adapter.getDuration()).toBe(82); + }); + + it("pauses old adapter before replacing with new duration", () => { + const clock = createManualAnimationClock(); + const adapter = createStaticSeekPlaybackAdapter( + { getTime: () => 0, renderSeek: () => {} }, + 10, + clock, + ); + adapter.play(); + expect(adapter.isPlaying()).toBe(true); + adapter.pause(); + expect(adapter.isPlaying()).toBe(false); + }); }); describe("buildStandaloneRootTimelineElement", () => { diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.ts b/packages/studio/src/player/hooks/useTimelinePlayer.ts index 4614b3936..3c9db97d6 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.ts @@ -59,7 +59,7 @@ export function useTimelinePlayer() { const iframeShortcutCleanupRef = useRef<(() => void) | null>(null); const lastTimelineMessageRef = useRef(0); const staticSeekAdapterRef = useRef<{ - player: RuntimePlaybackAdapter; + player: RuntimePlaybackAdapter | PlaybackAdapter; duration: number; adapter: PlaybackAdapter; } | null>(null); @@ -125,10 +125,12 @@ export function useTimelinePlayer() { return playerAdapter; } + let timelineAdapter: PlaybackAdapter | null = null; if (win.__timeline) { const adapter = wrapTimeline(win.__timeline); const dur = getAdapterDuration(adapter); if (dur > 0 && docDuration <= dur) return adapter; + if (dur > 0) timelineAdapter ??= adapter; } if (win.__timelines) { @@ -145,40 +147,44 @@ export function useTimelinePlayer() { const adapter = wrapTimeline(win.__timelines[key]); const dur = getAdapterDuration(adapter); if (dur > 0 && docDuration <= dur) return adapter; + if (dur > 0) timelineAdapter ??= adapter; } } + // The document timeline extends past every native adapter's duration. + // Wrap the best available adapter with the effective duration so the + // seek slider, seek clamping, and duration display cover the full range. + const bestAdapter = playerAdapter ?? timelineAdapter; const effectiveDuration = Math.max( usePlayerStore.getState().duration, docDuration, adapterDur, ); - const baseAdapter = playerAdapter; if ( - baseAdapter && + bestAdapter && effectiveDuration > 0 && - (typeof baseAdapter.renderSeek === "function" || typeof baseAdapter.seek === "function") + ("renderSeek" in bestAdapter || typeof bestAdapter.seek === "function") ) { const cached = staticSeekAdapterRef.current; - if (cached?.player === baseAdapter && cached.duration === effectiveDuration) { + if (cached?.player === bestAdapter && cached.duration === effectiveDuration) { return cached.adapter; } cached?.adapter.pause(); const adapter = createStaticSeekPlaybackAdapter( - baseAdapter, + bestAdapter, effectiveDuration, getDefaultStaticSeekPlaybackClock(win), () => usePlayerStore.getState().playbackRate, ); staticSeekAdapterRef.current = { - player: baseAdapter, + player: bestAdapter, duration: effectiveDuration, adapter, }; return adapter; } - return playerAdapter; + return bestAdapter; } catch (err) { console.warn("[useTimelinePlayer] Could not get playback adapter (cross-origin)", err); return null;