mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-04 07:19:52 +00:00
fix(studio): preserve playback state on Jump-to-in/out shortcuts (#842)
When the user has the timeline playing and presses A (Jump to in-point)
or E (Jump to out-point), the seek seeks to the marker as expected but
also pauses the playback. The reporter (and the natural UX) expects
playback to keep going from the marker.
Root cause sits in two layers:
1. The `seek` callback in `useTimelinePlayer.ts` unconditionally calls
`setIsPlaying(false)` and `stopRAFLoop()` whenever the store reports
playing. That path is shared with timeline clicks, LayersPanel
navigation, and frame stepping — flipping the default would change
behavior the rest of the app expects.
2. `wrapTimeline` (the GSAP-timeline-backed adapter) calls `tl.pause()`
before `tl.seek(t)`, so even if the callback above stopped pausing,
GSAP-driven compositions would still get paused inside the adapter.
The fix is opt-in at both layers:
- Extend `PlaybackAdapter.seek` with `options?: { keepPlaying?: boolean }`.
Default is omitted/false, preserving existing behavior for every
caller that doesn't pass the option.
- `wrapTimeline.seek` skips the implicit `tl.pause()` when keepPlaying
is set. `createStaticSeekPlaybackAdapter` accepts the new signature
but is a no-op for the flag (it never paused internally).
- `useTimelinePlayer` seek callback grows the same option and forwards
it to adapter.seek(time, options). The reset block (stopRAFLoop,
setIsPlaying(false), shuttle refs) is gated behind !options.keepPlaying.
- Reverse shuttle is always stopped on seek (the RAF reverse tick
cannot survive a seek), so keepPlaying is overridden when the
shuttle was running backward. Documented with an inline comment.
- usePlaybackKeyboard updates its seek param type to match and passes
{ keepPlaying: true } on the A and E handlers only. Frame stepping
(Arrow keys, J/L with K held) keeps the default.
Tests (happy-dom):
- useTimelinePlayer.seek.test.ts covers the callback in three cases:
default seek clears isPlaying, seek with keepPlaying preserves
isPlaying=true, and the option from paused state stays paused.
- playbackAdapter.test.ts (new) covers wrapTimeline: default seek
pauses the GSAP timeline, keepPlaying: true skips the pause,
keepPlaying: false is the explicit default.
Closes part of #834 (sub-bug #2). Sub-bug #1 (playhead should loop
to in-point when exceeding out-point) lives in the RAF tick and is
left for a follow-up PR.
Co-authored-by: Carlos Alcaraz <193642530+calcarazgre646@users.noreply.github.com>
This commit is contained in:
co-authored by
Carlos Alcaraz
parent
4177c32e73
commit
0f2a705259
@@ -22,7 +22,7 @@ interface UsePlaybackKeyboardParams {
|
||||
play: () => void;
|
||||
playBackward: (rate: number) => void;
|
||||
pause: () => void;
|
||||
seek: (time: number) => void;
|
||||
seek: (time: number, options?: { keepPlaying?: boolean }) => void;
|
||||
}
|
||||
|
||||
export function usePlaybackKeyboard({
|
||||
@@ -145,13 +145,15 @@ export function usePlaybackKeyboard({
|
||||
}
|
||||
if (key === "a") {
|
||||
e.preventDefault();
|
||||
seek(usePlayerStore.getState().inPoint ?? 0);
|
||||
seek(usePlayerStore.getState().inPoint ?? 0, { keepPlaying: true });
|
||||
return;
|
||||
}
|
||||
if (key === "e") {
|
||||
e.preventDefault();
|
||||
const { outPoint } = usePlayerStore.getState();
|
||||
seek(outPoint ?? getAdapter()?.getDuration() ?? usePlayerStore.getState().duration);
|
||||
seek(outPoint ?? getAdapter()?.getDuration() ?? usePlayerStore.getState().duration, {
|
||||
keepPlaying: true,
|
||||
});
|
||||
return;
|
||||
}
|
||||
},
|
||||
|
||||
@@ -30,6 +30,40 @@ afterEach(() => {
|
||||
resetPlayerStore();
|
||||
});
|
||||
|
||||
function attachIframeAdapter(api: ReturnType<typeof useTimelinePlayer>) {
|
||||
const iframe = document.createElement("iframe");
|
||||
let currentTime = 0;
|
||||
const adapter = {
|
||||
play: () => {},
|
||||
pause: () => {},
|
||||
seek: (time: number) => {
|
||||
currentTime = time;
|
||||
},
|
||||
getTime: () => currentTime,
|
||||
getDuration: () => 30,
|
||||
isPlaying: () => false,
|
||||
};
|
||||
Object.defineProperty(iframe, "contentWindow", {
|
||||
value: {
|
||||
__player: adapter,
|
||||
postMessage: () => {},
|
||||
scrollTo: () => {},
|
||||
addEventListener: () => {},
|
||||
removeEventListener: () => {},
|
||||
},
|
||||
configurable: true,
|
||||
});
|
||||
Object.defineProperty(iframe, "contentDocument", {
|
||||
value: document.implementation.createHTMLDocument("preview"),
|
||||
configurable: true,
|
||||
});
|
||||
act(() => {
|
||||
api.iframeRef.current = iframe;
|
||||
api.onIframeLoad();
|
||||
});
|
||||
return adapter;
|
||||
}
|
||||
|
||||
describe("useTimelinePlayer seek hydration", () => {
|
||||
it("keeps an external seek request until the iframe adapter is ready", () => {
|
||||
let api: ReturnType<typeof useTimelinePlayer> | null = null;
|
||||
@@ -98,3 +132,90 @@ describe("useTimelinePlayer seek hydration", () => {
|
||||
unsubscribe();
|
||||
});
|
||||
});
|
||||
|
||||
describe("useTimelinePlayer seek keepPlaying option (#834)", () => {
|
||||
it("default seek() clears isPlaying when the store reports playing", () => {
|
||||
let api: ReturnType<typeof useTimelinePlayer> | null = null;
|
||||
const host = document.createElement("div");
|
||||
document.body.append(host);
|
||||
const root = createRoot(host);
|
||||
|
||||
act(() => {
|
||||
root.render(
|
||||
React.createElement(TimelinePlayerHarness, { onValue: (value) => (api = value) }),
|
||||
);
|
||||
});
|
||||
attachIframeAdapter(api!);
|
||||
|
||||
act(() => {
|
||||
usePlayerStore.setState({ isPlaying: true });
|
||||
});
|
||||
|
||||
act(() => {
|
||||
api!.seek(5);
|
||||
});
|
||||
|
||||
expect(usePlayerStore.getState().isPlaying).toBe(false);
|
||||
expect(usePlayerStore.getState().currentTime).toBe(5);
|
||||
|
||||
act(() => {
|
||||
root.unmount();
|
||||
});
|
||||
});
|
||||
|
||||
it("seek(time, { keepPlaying: true }) preserves isPlaying=true so A/E shortcuts don't pause the timeline", () => {
|
||||
let api: ReturnType<typeof useTimelinePlayer> | null = null;
|
||||
const host = document.createElement("div");
|
||||
document.body.append(host);
|
||||
const root = createRoot(host);
|
||||
|
||||
act(() => {
|
||||
root.render(
|
||||
React.createElement(TimelinePlayerHarness, { onValue: (value) => (api = value) }),
|
||||
);
|
||||
});
|
||||
attachIframeAdapter(api!);
|
||||
|
||||
act(() => {
|
||||
usePlayerStore.setState({ isPlaying: true });
|
||||
});
|
||||
|
||||
act(() => {
|
||||
api!.seek(5, { keepPlaying: true });
|
||||
});
|
||||
|
||||
expect(usePlayerStore.getState().isPlaying).toBe(true);
|
||||
expect(usePlayerStore.getState().currentTime).toBe(5);
|
||||
|
||||
act(() => {
|
||||
root.unmount();
|
||||
});
|
||||
});
|
||||
|
||||
it("seek(time, { keepPlaying: true }) from paused state stays paused (no spurious resume)", () => {
|
||||
let api: ReturnType<typeof useTimelinePlayer> | null = null;
|
||||
const host = document.createElement("div");
|
||||
document.body.append(host);
|
||||
const root = createRoot(host);
|
||||
|
||||
act(() => {
|
||||
root.render(
|
||||
React.createElement(TimelinePlayerHarness, { onValue: (value) => (api = value) }),
|
||||
);
|
||||
});
|
||||
attachIframeAdapter(api!);
|
||||
|
||||
expect(usePlayerStore.getState().isPlaying).toBe(false);
|
||||
|
||||
act(() => {
|
||||
api!.seek(5, { keepPlaying: true });
|
||||
});
|
||||
|
||||
expect(usePlayerStore.getState().isPlaying).toBe(false);
|
||||
expect(usePlayerStore.getState().currentTime).toBe(5);
|
||||
|
||||
act(() => {
|
||||
root.unmount();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -321,7 +321,10 @@ export function useTimelinePlayer() {
|
||||
}, [getAdapter, setCurrentTime, setIsPlaying, stopRAFLoop, stopReverseLoop]);
|
||||
|
||||
const seek = useCallback(
|
||||
(time: number) => {
|
||||
(time: number, options?: { keepPlaying?: boolean }) => {
|
||||
// Reverse shuttle is always stopped: the RAF reverse tick can't survive
|
||||
// a seek anyway, so `keepPlaying` only preserves forward playback.
|
||||
const wasReverseShuttle = shuttleDirectionRef.current === "backward";
|
||||
stopReverseLoop();
|
||||
const adapter = getAdapter();
|
||||
if (!adapter) {
|
||||
@@ -330,16 +333,27 @@ export function useTimelinePlayer() {
|
||||
}
|
||||
const duration = Math.max(0, adapter.getDuration());
|
||||
const nextTime = Math.max(0, duration > 0 ? Math.min(duration, time) : time);
|
||||
adapter.seek(nextTime);
|
||||
adapter.seek(nextTime, options);
|
||||
liveTime.notify(nextTime); // Direct DOM updates (playhead, timecode, progress) — no re-render
|
||||
setCurrentTime(nextTime); // sync store so Split/Delete have accurate time
|
||||
stopRAFLoop();
|
||||
if (usePlayerStore.getState().isPlaying) setIsPlaying(false);
|
||||
shuttleDirectionRef.current = null;
|
||||
shuttleSpeedIndexRef.current = 0;
|
||||
if (!options?.keepPlaying || wasReverseShuttle) {
|
||||
stopRAFLoop();
|
||||
if (usePlayerStore.getState().isPlaying) setIsPlaying(false);
|
||||
shuttleDirectionRef.current = null;
|
||||
shuttleSpeedIndexRef.current = 0;
|
||||
}
|
||||
return true;
|
||||
},
|
||||
[getAdapter, pendingSeekRef, setCurrentTime, setIsPlaying, stopRAFLoop, stopReverseLoop],
|
||||
[
|
||||
getAdapter,
|
||||
pendingSeekRef,
|
||||
setCurrentTime,
|
||||
setIsPlaying,
|
||||
stopRAFLoop,
|
||||
stopReverseLoop,
|
||||
shuttleDirectionRef,
|
||||
shuttleSpeedIndexRef,
|
||||
],
|
||||
);
|
||||
|
||||
// Handle seek requests from outside the player loop (e.g. LayersPanel).
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { wrapTimeline } from "./playbackAdapter";
|
||||
import type { TimelineLike } from "./playbackTypes";
|
||||
|
||||
describe("wrapTimeline seek keepPlaying option (#834)", () => {
|
||||
function mockTimeline(): TimelineLike & {
|
||||
play: ReturnType<typeof vi.fn>;
|
||||
pause: ReturnType<typeof vi.fn>;
|
||||
seek: ReturnType<typeof vi.fn>;
|
||||
} {
|
||||
return {
|
||||
play: vi.fn(),
|
||||
pause: vi.fn(),
|
||||
seek: vi.fn(),
|
||||
time: () => 0,
|
||||
duration: () => 10,
|
||||
isActive: () => false,
|
||||
};
|
||||
}
|
||||
|
||||
it("default seek pauses the GSAP timeline before seeking", () => {
|
||||
const tl = mockTimeline();
|
||||
const adapter = wrapTimeline(tl);
|
||||
|
||||
adapter.seek(5);
|
||||
|
||||
expect(tl.pause).toHaveBeenCalledTimes(1);
|
||||
expect(tl.seek).toHaveBeenCalledWith(5);
|
||||
});
|
||||
|
||||
it("seek with { keepPlaying: true } skips the implicit pause", () => {
|
||||
const tl = mockTimeline();
|
||||
const adapter = wrapTimeline(tl);
|
||||
|
||||
adapter.seek(5, { keepPlaying: true });
|
||||
|
||||
expect(tl.pause).not.toHaveBeenCalled();
|
||||
expect(tl.seek).toHaveBeenCalledWith(5);
|
||||
});
|
||||
|
||||
it("seek with { keepPlaying: false } still pauses (explicit default)", () => {
|
||||
const tl = mockTimeline();
|
||||
const adapter = wrapTimeline(tl);
|
||||
|
||||
adapter.seek(5, { keepPlaying: false });
|
||||
|
||||
expect(tl.pause).toHaveBeenCalledTimes(1);
|
||||
expect(tl.seek).toHaveBeenCalledWith(5);
|
||||
});
|
||||
});
|
||||
@@ -134,8 +134,8 @@ export function wrapTimeline(tl: TimelineLike): PlaybackAdapter {
|
||||
return {
|
||||
play: () => tl.play(),
|
||||
pause: () => tl.pause(),
|
||||
seek: (t) => {
|
||||
tl.pause();
|
||||
seek: (t, options) => {
|
||||
if (!options?.keepPlaying) tl.pause();
|
||||
tl.seek(t);
|
||||
},
|
||||
getTime: () => tl.time(),
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
export interface PlaybackAdapter {
|
||||
play: () => void;
|
||||
pause: () => void;
|
||||
seek: (time: number) => void;
|
||||
seek: (time: number, options?: { keepPlaying?: boolean }) => void;
|
||||
getTime: () => number;
|
||||
getDuration: () => number;
|
||||
isPlaying: () => boolean;
|
||||
|
||||
Reference in New Issue
Block a user