fix(studio): address review — GSAP-only fallback, drop dead alias, add tests

- [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.
This commit is contained in:
Miguel Ángel
2026-05-18 18:28:34 -04:00
parent 10af9655f2
commit 1a63a945e0
3 changed files with 70 additions and 8 deletions
+1
View File
@@ -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
@@ -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(`
<div data-composition-id="main">
<div data-composition-id="sub-a" data-start="0" data-hf-authored-duration="8"></div>
<div data-composition-id="sub-b" data-start="60" data-hf-authored-duration="10"></div>
</div>
`);
expect(readTimelineDurationFromDocument(doc)).toBe(70);
});
it("picks the larger of data-duration and data-hf-authored-duration children", () => {
const doc = createDocument(`
<div data-composition-id="main">
<div data-start="0" data-duration="5"></div>
<div data-composition-id="ext" data-start="74" data-hf-authored-duration="8"></div>
</div>
`);
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", () => {
@@ -59,7 +59,7 @@ export function useTimelinePlayer() {
const iframeShortcutCleanupRef = useRef<(() => void) | null>(null);
const lastTimelineMessageRef = useRef<number>(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;