Skip to content

feat(timeline): scroll position lives in url#201

Open
chiptus wants to merge 5 commits into
mainfrom
claude/188-192-scrollto-url-state
Open

feat(timeline): scroll position lives in url#201
chiptus wants to merge 5 commits into
mainfrom
claude/188-192-scrollto-url-state

Conversation

@chiptus

@chiptus chiptus commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Stacked on the #191 branch (claude/188-191-timeline-geometry) / PR. Adds a scrollTo search param that captures the moment centered in the Timeline viewport, with a pure precedence function deciding the mount position and a dedicated hook owning one-way scroll<->URL sync.

Closes #192

Verification

  • Load the timeline with no scrollTo in the URL; scroll the horizontal timeline and confirm scrollTo appears in the URL (~300ms after scrolling stops), rounded to 5 minutes, via history replace (back doesn't land on an intermediate scroll state).
  • Load a timeline URL with scrollTo set; confirm the viewport centers on that moment on mount.
  • Change the day filter with no scrollTo present; confirm the viewport centers on that day's start.
  • Scroll the timeline and confirm the day/time/stage filters and rendered schedule do not recompute (no flicker/reflow) purely from the scrollTo write.
  • Run pnpm vitest run — includes new unit tests for resolveTimelineMountMoment and roundToNearestMinutes in src/lib/timelineMountMoment.test.ts.
  • New Playwright spec tests/e2e/timeline-scroll.spec.ts covers scroll-to-URL writing, mount centering, and back/reload restoring the viewport (runs in CI, not run locally in this environment).

Generated by Claude Code

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@vercel

vercel Bot commented Jul 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
upline Ready Ready Preview, Comment Jul 16, 2026 2:20pm

time: search.time,
stages: search.stages,
}),
structuralSharing: true,

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Check: what is structuralSharing

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

structuralSharing is a TanStack Router option on useSearch's select: when the selected result is deep-equal to the previous one, it reuses the previous object/array references (via replaceEqualDeep) instead of returning fresh ones. It matters here because select builds a new object (with a new stages array) on every URL change — without structural sharing, a scrollTo-only write would hand the Timeline a new stages reference, re-triggering the filtering useMemo on every debounced scroll write. With it, stages keeps its identity as long as its contents are unchanged, so scroll writes never recompute the filtered schedule (the "scroll writes must not re-run filtering" requirement from #192).


Generated by Claude Code

Comment thread src/lib/searchSchemas.ts Outdated
Comment thread tests/e2e/timeline-scroll.spec.ts Outdated
Base automatically changed from claude/188-191-timeline-geometry to main July 15, 2026 19:31
Copilot AI review requested due to automatic review settings July 15, 2026 19:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds URL-backed scroll position for the horizontal schedule timeline by introducing a scrollTo search param, a pure mount-moment precedence resolver, and a dedicated hook to own one-way scroll ↔ URL synchronization. This builds on the timeline geometry work by centralizing time↔pixel mapping helpers and adding unit/e2e coverage around the new behavior.

Changes:

  • Introduces scrollTo URL state with debounced history-replace writes and mount-time centering via useTimelineScrollSync.
  • Adds pure utilities (resolveTimelineMountMoment, roundToNearestMinutes, timeToOffset, offsetToTime) with new unit tests.
  • Updates timeline rendering to consume shared geometry functions and adds Playwright coverage for scrollTo behavior.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/e2e/timeline-scroll.spec.ts New e2e coverage for debounced URL writes, mount centering, and back/reload restoration.
src/pages/EditionView/tabs/ScheduleTab/horizontal/TimeScale.tsx Switches time axis positioning to shared timeToOffset mapping.
src/pages/EditionView/tabs/ScheduleTab/horizontal/TimelineContainer.tsx Wires in useTimelineScrollSync and adds a test id for the scroll container.
src/lib/timelineMountMoment.ts Adds pure mount precedence resolver + rounding helper for coarse scrollTo values.
src/lib/timelineMountMoment.test.ts Unit tests for mount precedence and rounding behavior.
src/lib/timelineCalculator.ts Adds canonical time↔pixel conversion helpers and applies them to set positioning/width and total width.
src/lib/timelineCalculator.test.ts Unit tests for geometry mappings and key timeline-boundary behaviors.
src/lib/searchSchemas.ts Extends timeline search schema to accept optional scrollTo.
src/hooks/useTimelineUrlState.ts Uses narrowed select + structural sharing to prevent scrollTo writes from invalidating filter state.
src/hooks/useTimelineScrollSync.ts New hook implementing one-way mount centering and debounced scroll→URL sync.

Comment on lines +68 to +76
const targetScrollLeft = Math.max(
0,
timeToOffset(moment, festivalStart) - container.clientWidth / 2,
);

if (targetScrollLeft !== container.scrollLeft) {
programmaticScrollLeftRef.current = targetScrollLeft;
container.scrollLeft = targetScrollLeft;
}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Partially taken. The clamp point was real: the ref now records the read-back scrollLeft after assignment (5fcab7c), so a browser-clamped mount scroll is still recognized as programmatic. The 20px point isn't: the hour-marker axis renders at the raw geometry offset — only set blocks and date labels carry a pre-existing +20 — so the conversion here is consistent with the canonical axis mapping, and the round-trip (write moment → restore scrollLeft) is exact regardless. Aligning set blocks with the axis is a separate visual cleanup, out of scope for this PR.


Generated by Claude Code

Comment on lines +103 to +105
const centerOffset = el.scrollLeft + el.clientWidth / 2;
const centerMoment = offsetToTime(centerOffset, festivalStart);
const rounded = roundToNearestMinutes(

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Not taken — same reasoning as the thread above: the hour-marker axis (the canonical time→pixel mapping) has no 20px offset; only set blocks/date labels carry a pre-existing +20. The conversion matches the axis, and the scrollTo round-trip restores the exact scroll position either way.


Generated by Claude Code

Comment on lines +81 to +82

useEffect(() => {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Intentional, per the spec (issue #192 / PRD #188): mount precedence applies at mount only, and "filtering narrows, it never scrolls" — changing the day filter must never move the viewport. The day-filter rule exists only to pick the initial position when a page loads with ?day= and no scrollTo.


Generated by Claude Code

claude added 4 commits July 16, 2026 14:17
Add a scrollTo search param that captures the moment centered in the
Timeline viewport. A pure resolveTimelineMountMoment precedence
function (scrollTo -> day filter -> festival start) decides where to
center on mount; useTimelineScrollSync owns that one-time centering
plus debounced (~300ms), 5-minute-rounded, history-replace writes on
user scroll. useTimelineUrlState now selects its filter params with
structural sharing so scrollTo writes don't recompute the filtered
schedule.

Closes #192

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AziTqr3f12fxALYD6jhrW8
Position-based suppression of programmatic scroll events (a browser may
fire more than one per scrollLeft write), and the back-restore e2e half
now skips explicitly instead of silently passing when no set link is
rendered.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AziTqr3f12fxALYD6jhrW8
Also drops a redundant schema comment, per review.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AziTqr3f12fxALYD6jhrW8
The browser clamps scrollLeft to the scrollable range, so the
suppression ref must record the read-back value, not the requested
target, for the scroll handler to recognize the mount event.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AziTqr3f12fxALYD6jhrW8
The rebase onto main brought in #199's removal of the view search
param; the select still read it, failing typecheck.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AziTqr3f12fxALYD6jhrW8
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.

feat: timeline scroll position lives in the URL (scrollTo)

3 participants