Skip to content

fix(studio): the timeline follows a paused playhead and zoom keeps it in place - #4712

Open
miguel-heygen wants to merge 7 commits into
mainfrom
fix/timeline-follow-paused-playhead
Open

miguel-heygen wants to merge 7 commits into
mainfrom
fix/timeline-follow-paused-playhead

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

What

The Studio timeline now keeps the playhead in view while the film is paused:

  • A paused seek follows. Moving the preview slider, stepping with the keys or any other seek that puts the playhead off screen scrolls the timeline so the playhead is visible again. A seek that lands on screen does not scroll.
  • Toolbar and slider zoom keep the playhead in place. The playhead stays where it was on screen while the scale changes; if it was off screen, the zoom brings it into view. Before, zoom kept the middle of the view fixed, and did nothing at all when the view was at 00:00.

Unchanged: no scroll while the playhead is being dragged, none in Fit, and a person's own timeline scroll while paused is never undone. The view follows only when the playhead time changes. A window resize still keeps the view's middle time, or 00:00 when the view is at the start (#4411).

Why

Paused, the timeline only followed during playback, so a seek from the preview or the keyboard could leave the playhead off screen with no way to see where it went. Zooming in to look closely at the playhead moved it away instead.

Related work

Keeps #4411's rule for window resizes. A zoom set with the playhead at 00:00, such as a zoom restored when a project opens, still stays at 00:00.

How

useTimelinePlayhead.ts:

  • The live-time subscriber, which already scrolled during playback, now also runs while paused, when the time changed. Paused, it scrolls only when the playhead is outside the view, reusing the playback follow position.
  • A person's zoom is told apart from every other scale change by a counter: useTimelineZoom's setManualZoomPercent (the toolbar buttons and slider) bumps userZoomCount in the store, while the zoom pin on a first edit and the re-pin after a length change write the percent directly. The anchor effect treats a scale change as a person's zoom only when the counter moved in the same commit. A person's zoom anchors on the playhead when it is on screen, then brings it into view if needed; a window resize, a pin or a re-pin keep the old centre anchor and the start-of-view rule.

Test plan

  • Unit tests added/updated

  • Manual testing performed

  • Documentation updated (if applicable)

  • Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug

  • useTimelinePlayhead.test.tsx covers: a paused seek off screen scrolls it into view; a paused seek on screen (even past the playback follow line) does not; a republished paused time does not undo a manual scroll; no scroll while dragging or in Fit; a toolbar zoom keeps the playhead's screen position, brings an off-screen playhead into view, and still anchors when it lands on the stored percent; a zoom with the playhead at 0 stays at 00:00; a resize keeps 00:00 (also after an edit pinned the zoom) and keeps the centre when scrolled; a length re-pin does not jump to an off-screen playhead. TimelineToolbar.test.tsx: a Zoom in click counts as a person's zoom. Four of the new tests fail on main's code.

  • Nine targeted breaks (time-moved check, drag check, Fit check, paused reveal rule, playhead anchor, the 00:00 skip for zooms, resize treated as zoom, the counter bump, the resize 00:00 rule) each fail a test.

  • Touched test files pass together and one by one (149 tests); tsc, oxlint, oxfmt, the comment checks and the dead-code audit pass.

A repeat seek to the time the playhead already has (Home when it is at 0) does not scroll, because the view follows only a playhead change.

Before

Studio on main (2195db5), paused, on the pr-to-video film from the public hyperframes-launches repo: zoom in three times, step the playhead past the right edge with Shift+Right, then zoom in once more. The timeline stays at 00:00 while the playhead leaves the view, and the last zoom keeps 00:00 with the playhead far off screen.

follow-main-2195db5e1.mp4

After

The same steps on this branch (bcaa431): the timeline scrolls to keep the stepped playhead in view, and the last zoom keeps the playhead where it was on screen.

follow-fix-bcaa4311d.mp4

@miguel-heygen
miguel-heygen marked this pull request as ready for review September 29, 2026 02:20

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at bcaa4311d587721901dee7f89922f95a9ebce53d.

Verdict: REQUEST_CHANGES

The zoom half is solid and the paused follow is the right shape. One path breaks the promise that a person's own scroll while paused is never undone.

Findings

Blocking

1. The first preview reload after a keyboard pause scrolls the timeline back to the playhead.

useTimelinePlayhead.ts decides "the playhead moved" by comparing each liveTime value with lastLiveTimeRef, and that ref is only written by the liveTime subscriber. pause() in useTimelinePlayer.ts (the path Space and togglePlay take) stops the rAF ticks, then calls setCurrentTime(adapter.getTime()) and setIsPlaying(false). It never calls liveTime.notify. So after a pause the ref holds the last rAF tick time, while the store and the adapter hold a slightly later time.

The next reload (an edit that refreshes the preview) saves adapter.getTime() into pendingSeekRef, and the reload commit publishes it with liveTime.notify(startTime) (useTimelineSyncCallbacks.ts:249). That value differs from the ref, so moved is true. If the person scrolled away from the playhead meanwhile, revealPlayheadScrollLeft jumps the view back to it. The geometry hook's scroll restore then keeps the jumped position, because the scroll event updates lastScrollLeftRef first.

Repro at the hook level (drop into useTimelinePlayhead.test.tsx, it uses the existing harness):

it("keeps a person's scroll when a reload follows a keyboard pause", () => {
  const scroll = scrollBox(0);
  mount({ pps: 100, scroll });
  act(() => usePlayerStore.setState({ isPlaying: true }));
  act(() => liveTime.notify(30)); // last rAF tick
  act(() => {
    // pause(): store gets adapter.getTime(), no liveTime notify
    usePlayerStore.getState().setCurrentTime(30.012);
    usePlayerStore.setState({ isPlaying: false });
  });
  scroll.scrollLeft = 0; // the person scrolls back to the start
  act(() => liveTime.notify(30.012)); // reload commit republishes the restored time
  expect(scroll.scrollLeft).toBe(0);
});

At this head it fails with expected 2425.2 to be +0.

A fix that passes this test and the 13 existing tests in the file: sync the ref when playback stops, in the existing unsubPlaying subscriber.

if (prev.isPlaying && !state.isPlaying) {
  lastLiveTimeRef.current = state.currentTime;
  place(lastLiveTimeRef.current, true);
}

pause() sets currentTime before isPlaying, so state.currentTime is the adapter time there. The preview-click pause and the end-of-playback stop both publish through liveTime anyway. The same drift also feeds the zoom anchor's time, so this fixes that too. The gap there is sub-frame, so it does not matter on its own.

Non-blocking

2. userZoomCount in the anchor effect's dependency list is not covered by a test. If I drop it from the list, all 26 tests still pass. It matters when a toolbar zoom hits the zoom clamp. The counter moves but pps does not, so only that dependency re-runs the effect and moves previousZoomCountRef forward. Without it, the next window resize would look like a person's zoom and anchor on the playhead. A test for "zoom in at the max clamp, then resize a scrolled view, centre is kept" would pin it.

Claims checked

  • "A person's zoom is told apart ... by a counter." True. useTimelineZoom's setManualZoomPercent bumps userZoomCount before the store write, and both land in one React commit from the toolbar click and slider onChange handlers. The edit pin (pinTimelineZoom) and the length re-pin in useTimelineGeometry.ts write manualZoomPercent directly, so they keep the centre rule. Pinch also goes through the wrapper, but skipCenterAnchorRef still returns early first, so it keeps its cursor anchor.
  • "No scroll while the playhead is being dragged." True. beginScrub sets isDragging, and finishScrub publishes its last seek before it clears the flag, so a release past the edge does not reveal.
  • "A person's own timeline scroll while paused is never undone." Not true in the case in finding 1. It does hold when the same time is published again.
  • "Four of the new tests fail on main's code." Checked by putting the three source files from 2195db5e back in place: 4 tests in useTimelinePlayhead.test.tsx fail, plus the new toolbar test, since the field does not exist there.
  • The follow while playing is unchanged. The playing branch still calls getTimelinePlaybackFollowScrollLeft with the same inputs, and the drag and Fit guards are the same.
  • Scroll clamps: the anchor result is clamped to [0, scrollWidth - clientWidth], and the reveal goes through the follow helper, which clamps to the same range.
  • Cleanup: the mount effect still unsubscribes both listeners. No new rAF or timer was added.
  • New store field: userZoomCount defaults to 0. Its only readers are the playhead hook and the tests.

Tests run

  • Mutation test, one change at a time, on the two touched test files. These changes each failed at least one test: removing the time-moved check, turning the paused follow off, using the playback follow line while paused, always anchoring at the centre, applying the 00:00 skip to zooms, dropping the reveal after the anchor, treating every scale change as a zoom, and removing the counter bump. Dropping userZoomCount from the effect dependencies failed nothing (finding 2). Every change was reverted.
  • Repro test above: fails at this head. Passes with the suggested fix. The file was removed afterwards.
  • useTimelinePlayhead.test.tsx and TimelineToolbar.test.tsx: 26 passed.
  • Full Studio suite: 557 files passed, 1 skipped; 6013 tests passed, 16 todo.
  • tsc --noEmit in packages/studio: clean.
  • oxlint and oxfmt --check on the 5 changed files: clean.

Checks

38 pass, 17 skipping, none failing or pending at this head.

Gate

reviewDecision REVIEW_REQUIRED, mergeStateStatus BLOCKED. No reviews exist on this PR yet.

— Rames

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants