feat(timeline): scroll position lives in url#201
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
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
scrollToURL state with debounced history-replace writes and mount-time centering viauseTimelineScrollSync. - 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. |
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
5fcab7c to
acf2265
Compare
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
Extracts the repeated clock-install/goto/skip-if-hidden block into openTimeline(), and turns the set-detail-link skip in the back-navigation test into an assertion so missing seed data fails loudly instead of silently dropping that acceptance criterion's coverage.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AziTqr3f12fxALYD6jhrW8
Addresses review feedback — the invariant is already explained inline by the structuralSharing thread on this PR.
Turns on defaultStructuralSharing router-wide instead of opting in per-hook, so any future select-based hook gets stable identities by default rather than needing to remember structuralSharing: true.
Stacked on the #191 branch (
claude/188-191-timeline-geometry) / PR. Adds ascrollTosearch 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
scrollToin the URL; scroll the horizontal timeline and confirmscrollToappears in the URL (~300ms after scrolling stops), rounded to 5 minutes, via history replace (back doesn't land on an intermediate scroll state).scrollToset; confirm the viewport centers on that moment on mount.scrollTopresent; confirm the viewport centers on that day's start.scrollTowrite.pnpm vitest run— includes new unit tests forresolveTimelineMountMomentandroundToNearestMinutesinsrc/lib/timelineMountMoment.test.ts.tests/e2e/timeline-scroll.spec.tscovers 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