Skip to content

Commit 0f2a705

Browse files
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>
1 parent 4177c32 commit 0f2a705

6 files changed

Lines changed: 200 additions & 13 deletions

File tree

packages/studio/src/player/hooks/usePlaybackKeyboard.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ interface UsePlaybackKeyboardParams {
2222
play: () => void;
2323
playBackward: (rate: number) => void;
2424
pause: () => void;
25-
seek: (time: number) => void;
25+
seek: (time: number, options?: { keepPlaying?: boolean }) => void;
2626
}
2727

2828
export function usePlaybackKeyboard({
@@ -145,13 +145,15 @@ export function usePlaybackKeyboard({
145145
}
146146
if (key === "a") {
147147
e.preventDefault();
148-
seek(usePlayerStore.getState().inPoint ?? 0);
148+
seek(usePlayerStore.getState().inPoint ?? 0, { keepPlaying: true });
149149
return;
150150
}
151151
if (key === "e") {
152152
e.preventDefault();
153153
const { outPoint } = usePlayerStore.getState();
154-
seek(outPoint ?? getAdapter()?.getDuration() ?? usePlayerStore.getState().duration);
154+
seek(outPoint ?? getAdapter()?.getDuration() ?? usePlayerStore.getState().duration, {
155+
keepPlaying: true,
156+
});
155157
return;
156158
}
157159
},

packages/studio/src/player/hooks/useTimelinePlayer.seek.test.ts

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,40 @@ afterEach(() => {
3030
resetPlayerStore();
3131
});
3232

33+
function attachIframeAdapter(api: ReturnType<typeof useTimelinePlayer>) {
34+
const iframe = document.createElement("iframe");
35+
let currentTime = 0;
36+
const adapter = {
37+
play: () => {},
38+
pause: () => {},
39+
seek: (time: number) => {
40+
currentTime = time;
41+
},
42+
getTime: () => currentTime,
43+
getDuration: () => 30,
44+
isPlaying: () => false,
45+
};
46+
Object.defineProperty(iframe, "contentWindow", {
47+
value: {
48+
__player: adapter,
49+
postMessage: () => {},
50+
scrollTo: () => {},
51+
addEventListener: () => {},
52+
removeEventListener: () => {},
53+
},
54+
configurable: true,
55+
});
56+
Object.defineProperty(iframe, "contentDocument", {
57+
value: document.implementation.createHTMLDocument("preview"),
58+
configurable: true,
59+
});
60+
act(() => {
61+
api.iframeRef.current = iframe;
62+
api.onIframeLoad();
63+
});
64+
return adapter;
65+
}
66+
3367
describe("useTimelinePlayer seek hydration", () => {
3468
it("keeps an external seek request until the iframe adapter is ready", () => {
3569
let api: ReturnType<typeof useTimelinePlayer> | null = null;
@@ -98,3 +132,90 @@ describe("useTimelinePlayer seek hydration", () => {
98132
unsubscribe();
99133
});
100134
});
135+
136+
describe("useTimelinePlayer seek keepPlaying option (#834)", () => {
137+
it("default seek() clears isPlaying when the store reports playing", () => {
138+
let api: ReturnType<typeof useTimelinePlayer> | null = null;
139+
const host = document.createElement("div");
140+
document.body.append(host);
141+
const root = createRoot(host);
142+
143+
act(() => {
144+
root.render(
145+
React.createElement(TimelinePlayerHarness, { onValue: (value) => (api = value) }),
146+
);
147+
});
148+
attachIframeAdapter(api!);
149+
150+
act(() => {
151+
usePlayerStore.setState({ isPlaying: true });
152+
});
153+
154+
act(() => {
155+
api!.seek(5);
156+
});
157+
158+
expect(usePlayerStore.getState().isPlaying).toBe(false);
159+
expect(usePlayerStore.getState().currentTime).toBe(5);
160+
161+
act(() => {
162+
root.unmount();
163+
});
164+
});
165+
166+
it("seek(time, { keepPlaying: true }) preserves isPlaying=true so A/E shortcuts don't pause the timeline", () => {
167+
let api: ReturnType<typeof useTimelinePlayer> | null = null;
168+
const host = document.createElement("div");
169+
document.body.append(host);
170+
const root = createRoot(host);
171+
172+
act(() => {
173+
root.render(
174+
React.createElement(TimelinePlayerHarness, { onValue: (value) => (api = value) }),
175+
);
176+
});
177+
attachIframeAdapter(api!);
178+
179+
act(() => {
180+
usePlayerStore.setState({ isPlaying: true });
181+
});
182+
183+
act(() => {
184+
api!.seek(5, { keepPlaying: true });
185+
});
186+
187+
expect(usePlayerStore.getState().isPlaying).toBe(true);
188+
expect(usePlayerStore.getState().currentTime).toBe(5);
189+
190+
act(() => {
191+
root.unmount();
192+
});
193+
});
194+
195+
it("seek(time, { keepPlaying: true }) from paused state stays paused (no spurious resume)", () => {
196+
let api: ReturnType<typeof useTimelinePlayer> | null = null;
197+
const host = document.createElement("div");
198+
document.body.append(host);
199+
const root = createRoot(host);
200+
201+
act(() => {
202+
root.render(
203+
React.createElement(TimelinePlayerHarness, { onValue: (value) => (api = value) }),
204+
);
205+
});
206+
attachIframeAdapter(api!);
207+
208+
expect(usePlayerStore.getState().isPlaying).toBe(false);
209+
210+
act(() => {
211+
api!.seek(5, { keepPlaying: true });
212+
});
213+
214+
expect(usePlayerStore.getState().isPlaying).toBe(false);
215+
expect(usePlayerStore.getState().currentTime).toBe(5);
216+
217+
act(() => {
218+
root.unmount();
219+
});
220+
});
221+
});

packages/studio/src/player/hooks/useTimelinePlayer.ts

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,10 @@ export function useTimelinePlayer() {
321321
}, [getAdapter, setCurrentTime, setIsPlaying, stopRAFLoop, stopReverseLoop]);
322322

323323
const seek = useCallback(
324-
(time: number) => {
324+
(time: number, options?: { keepPlaying?: boolean }) => {
325+
// Reverse shuttle is always stopped: the RAF reverse tick can't survive
326+
// a seek anyway, so `keepPlaying` only preserves forward playback.
327+
const wasReverseShuttle = shuttleDirectionRef.current === "backward";
325328
stopReverseLoop();
326329
const adapter = getAdapter();
327330
if (!adapter) {
@@ -330,16 +333,27 @@ export function useTimelinePlayer() {
330333
}
331334
const duration = Math.max(0, adapter.getDuration());
332335
const nextTime = Math.max(0, duration > 0 ? Math.min(duration, time) : time);
333-
adapter.seek(nextTime);
336+
adapter.seek(nextTime, options);
334337
liveTime.notify(nextTime); // Direct DOM updates (playhead, timecode, progress) — no re-render
335338
setCurrentTime(nextTime); // sync store so Split/Delete have accurate time
336-
stopRAFLoop();
337-
if (usePlayerStore.getState().isPlaying) setIsPlaying(false);
338-
shuttleDirectionRef.current = null;
339-
shuttleSpeedIndexRef.current = 0;
339+
if (!options?.keepPlaying || wasReverseShuttle) {
340+
stopRAFLoop();
341+
if (usePlayerStore.getState().isPlaying) setIsPlaying(false);
342+
shuttleDirectionRef.current = null;
343+
shuttleSpeedIndexRef.current = 0;
344+
}
340345
return true;
341346
},
342-
[getAdapter, pendingSeekRef, setCurrentTime, setIsPlaying, stopRAFLoop, stopReverseLoop],
347+
[
348+
getAdapter,
349+
pendingSeekRef,
350+
setCurrentTime,
351+
setIsPlaying,
352+
stopRAFLoop,
353+
stopReverseLoop,
354+
shuttleDirectionRef,
355+
shuttleSpeedIndexRef,
356+
],
343357
);
344358

345359
// Handle seek requests from outside the player loop (e.g. LayersPanel).
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
import { wrapTimeline } from "./playbackAdapter";
3+
import type { TimelineLike } from "./playbackTypes";
4+
5+
describe("wrapTimeline seek keepPlaying option (#834)", () => {
6+
function mockTimeline(): TimelineLike & {
7+
play: ReturnType<typeof vi.fn>;
8+
pause: ReturnType<typeof vi.fn>;
9+
seek: ReturnType<typeof vi.fn>;
10+
} {
11+
return {
12+
play: vi.fn(),
13+
pause: vi.fn(),
14+
seek: vi.fn(),
15+
time: () => 0,
16+
duration: () => 10,
17+
isActive: () => false,
18+
};
19+
}
20+
21+
it("default seek pauses the GSAP timeline before seeking", () => {
22+
const tl = mockTimeline();
23+
const adapter = wrapTimeline(tl);
24+
25+
adapter.seek(5);
26+
27+
expect(tl.pause).toHaveBeenCalledTimes(1);
28+
expect(tl.seek).toHaveBeenCalledWith(5);
29+
});
30+
31+
it("seek with { keepPlaying: true } skips the implicit pause", () => {
32+
const tl = mockTimeline();
33+
const adapter = wrapTimeline(tl);
34+
35+
adapter.seek(5, { keepPlaying: true });
36+
37+
expect(tl.pause).not.toHaveBeenCalled();
38+
expect(tl.seek).toHaveBeenCalledWith(5);
39+
});
40+
41+
it("seek with { keepPlaying: false } still pauses (explicit default)", () => {
42+
const tl = mockTimeline();
43+
const adapter = wrapTimeline(tl);
44+
45+
adapter.seek(5, { keepPlaying: false });
46+
47+
expect(tl.pause).toHaveBeenCalledTimes(1);
48+
expect(tl.seek).toHaveBeenCalledWith(5);
49+
});
50+
});

packages/studio/src/player/lib/playbackAdapter.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ export function wrapTimeline(tl: TimelineLike): PlaybackAdapter {
134134
return {
135135
play: () => tl.play(),
136136
pause: () => tl.pause(),
137-
seek: (t) => {
138-
tl.pause();
137+
seek: (t, options) => {
138+
if (!options?.keepPlaying) tl.pause();
139139
tl.seek(t);
140140
},
141141
getTime: () => tl.time(),

packages/studio/src/player/lib/playbackTypes.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
export interface PlaybackAdapter {
88
play: () => void;
99
pause: () => void;
10-
seek: (time: number) => void;
10+
seek: (time: number, options?: { keepPlaying?: boolean }) => void;
1111
getTime: () => number;
1212
getDuration: () => number;
1313
isPlaying: () => boolean;

0 commit comments

Comments
 (0)