diff --git a/.trail/learnings/2026-10-17-gantt-two-pane-scroll-sync-no-claude.md b/.trail/learnings/2026-10-17-gantt-two-pane-scroll-sync-no-claude.md new file mode 100644 index 00000000..4873c65c --- /dev/null +++ b/.trail/learnings/2026-10-17-gantt-two-pane-scroll-sync-no-claude.md @@ -0,0 +1,31 @@ +--- +title: "Gantt two-pane scroll sync: no echo, wheel-driven, measured equal scroll range" +author: Claude +date: 2026-10-17 +branch: fix/gantt-scroll-sync +cost: { minutes: 12, tokens: 78066 } +touches: + - CHANGELOG.md + - CLAUDE.md + - UI_GUIDELINES.md + - VERSION + - frontend/src/components/gantt/ProjectPanel/ProjectPanel.module.css + - frontend/src/components/views/ArchivedView/ArchivedView.module.css + - frontend/src/components/views/CrossSiteView/CrossSiteView.module.css + - frontend/src/components/views/EquipmentView/EquipmentView.module.css + - frontend/src/components/views/StaffView/StaffView.module.css + - frontend/src/hooks/__tests__/useScrollSync.test.ts +--- + +## Decided +- `useScrollSync` keeps the tree panel and the timeline body together with three behaviours: every programmatic `scrollTop` write is remembered and the follower's matching `scroll` event (within 1 px) is consumed, so the follower never snaps the source back; a non-passive `wheel` listener on both panes moves both in the same frame (Ctrl, Shift, any `deltaX` and zero `deltaY` are left to the browser so zoom and sideways trackpad panning keep working); and the hook measures both panes' natural `scrollHeight - clientHeight` and pads the shorter one so both reach the same scroll end. +- The re-measure after a `ResizeObserver` notification runs one `requestAnimationFrame` later (merged), otherwise changing padding inside the callback raises "ResizeObserver loop completed with undelivered notifications". jsdom has no `ResizeObserver`: the hook falls back to a `window` resize listener, tests stub the observer. + +## Tried and ruled out +- A fixed `padding-bottom: 8px` on the left pane "to match the horizontal scrollbar": the scrollbar takes 8 px of layout only with classic scrollbars; with overlay scrollbars (macOS) and `scrollbar-gutter: stable` it takes 0 and the left pane scrolls 8 px further, misaligning the last rows. +- A rAF-reset `isSyncing` flag against echo: the follower's `scroll` event arrives a frame later, after the reset, and copies its older position back, which is the stutter during fast scrolls. +- One shared vertical scroller with a sticky left panel would make alignment exact by construction, but the drag, resize and dependency layers locate rows by DOM position in the two-pane structure; too large for a fix. + +## Watch out +- Scroll events coalesce per frame; the echo guard assumes the follower's next `scroll` reports the last written value. Two writes to one pane in one frame are fine; a browser firing two separate events would cause one harmless snap. +- Mouse-wheel notches jump instantly on the wheel path (no smooth scrolling); accepted. diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a9fad20..4f15b4e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to Milestone Planner are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/). +## [1.10.1] - 2026-10-17 + +### Fixed +- The Gantt tree panel and the timeline now scroll together without the panel trailing by a frame or stuttering during fast scrolls, in the Gantt and in the Staff, Equipment, Cross-site and Archived views. +- Rows stay aligned at the very bottom of the list: the two panes always reach the same scroll end, whatever the horizontal scrollbar's height. + ## [1.10.0] - 2026-10-17 ### Changed diff --git a/CLAUDE.md b/CLAUDE.md index f4431044..a054c7b7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1047,7 +1047,7 @@ All backend↔frontend field mapping is centralized. The frontend model is mostl - `ProjectPanel/`: header actions, right-aligned across the name column + custom columns (new/import/manage columns/visibility/tag filter — `TagFilterDropdown` from `components/common`, also mounted in the Kanban toolbar — then `GroupByDropdown` and `ColorByDropdown`, both thin wrappers over the shared `TagGroupChoiceDropdown` shell; the header dropdowns hang from the positioned `.headerContent`, right-aligned and width-capped) + `SectionRow` (Group-by section header) + rows `ProjectRow → PhaseRow → SubphaseRow → AssignmentRow` + custom-column cells; wrapped in `ReorderProvider`. - `Timeline/`: `TimelineHeader` + `TimelineBody` — grid cells (weekend/holiday/company-event backgrounds; week separators when weekends are hidden), `TodayLine`, per-project bars (`ProjectBar`/`PhaseBar`), `DependencyLayer` (arrows), phantom overlays, `DragIndicator`, `ResourceDropOverlay`. Handles scroll persist/restore (200 ms debounce), zoom re-centering on the visible center date, view-mode changes preserving the left-edge date, Ctrl+wheel zoom, and scroll-to-today/date triggers. - **Group by tag group**: `GanttContainer` turns `useGanttGrouping()` + the tag-filtered projects into one flat `GanttRow[]` (`components/gantt/utils/projectSections.ts`: `buildProjectSections` → `buildGanttRows`; first tag in group order wins, empty sections omitted, "No " last) and hands the **same array** to `ProjectPanel` (`SectionRow`) and `TimelineBody` (`div.groupBand`, a direct child of `.rows`); both panes fall back to a memoised flat list when `rows` is omitted. Section rows are `--section-row-height` (36 px) + 1 px border, content-box, = `SECTION_ROW_TOTAL_HEIGHT` (37) in `calculateRowPositions(rows, …)`, keyed `section-`; collapsed sections' projects get no position (no arrows, no drop target). Section class names must not contain `row` and carry no `data-project-id`/`data-phase-id` (the drag/resize hooks and `DependencyLayer` find rows by `[class*="row"]` / data attributes). Project drag-reorder is off while grouped (`onProjectReorder` undefined). -- Scroll sync: `TimelineScrollContext` (horizontal, direct DOM with feedback-loop guard) across Gantt/Staff/Equipment timelines; `useScrollSync` (vertical) between tree panel and timeline body. +- Scroll sync: `TimelineScrollContext` (horizontal, direct DOM with feedback-loop guard) across Gantt/Staff/Equipment timelines; `useScrollSync` (vertical) between tree panel and timeline body — wheel-driven in the same frame, echo-free, and padding the shorter pane so both reach the same scroll end (never a fixed scrollbar-height padding in CSS). - **Drag pipeline** (`useDragAndDrop`): mousedown or touchstart on a bar (touches become a mouse-like event via `toMouseLikeEvent`) → live DOM movement with cell snapping (week/month) and a dependency lag indicator → on drop, `commitDragUpdate` computes new dates from pixels, awaits `confirmAction()` for whole-project moves (cancel snaps the bar back), snapshots undo state, clones and mutates the project tree (cascading children + assignments), updates the store optimistically, then persists via batched `PendingUpdate`s (`savePendingUpdates`). **Any persist error → reload all projects + clear undo.** The document `mousemove`/`mouseup` **and `touchmove`/`touchend`/`touchcancel`** listeners (here and in `useResize`) are attached at drag start and removed on mouse-up/touch-end and on unmount — never left on the document between drags. Touch: the bars hand the hooks a mouse-like event built by `utils/touchEvents.ts` (`toMouseLikeEvent`, an explicit object — never spread a SyntheticEvent, its methods are lost; its `preventDefault` is a no-op for the passive touchstart), and every touch start waits for a 5 px move first (`armTouchGesture` for project, assignment and resize-handle touches; the PhaseBar body has its own threshold) so a wobbly tap never commits a move; the non-passive `touchmove` listener calls `preventDefault()` only on interactive bars (React's root `touchstart` listener is passive, so only CSS `touch-action: none` on the *interactive* bars — the `interactive` class, set when a bar has drag/resize handlers, so read-only bars still scroll — plus that listener stop the timeline from scrolling), `touchend` is default-prevented so the browser sends no emulated click, and `touchcancel` restores the bar without saving (`cancelDrag`/`cancelResize`). - **Auto-calculation semantics** (`gantt/utils/autoCalculation.ts`): dependencies auto-align **only at creation time**; moving an item does *not* cascade to its dependents (users keep manual lead/lag control). Moves do (a) expand/contract parents to the union of their children and (b) auto-fit project dates to phases. Staff-assignment updates must include `allocation` (backend requirement). - **Critical path** (`components/gantt/utils/criticalPath.ts`): full CPM forward/backward pass, `totalFloat <= 0` = critical; loaded via dynamic import from `appStore.toggleCriticalPath` to avoid a circular dependency. @@ -1082,7 +1082,7 @@ All backend↔frontend field mapping is centralized. The frontend model is mostl | `useGanttColorBy` | Current site's Gantt "Colour by" (`{siteId, group, setGroup}`), read by every `ProjectTimeline` row and the `ColorByDropdown`; same rules as `useGanttGrouping` (primitive selectors, stale id → `null` without a write, no-op without a site). Import from `@/hooks/useGanttColorBy` | | `useEscapeKey` | Escape-to-close for standalone dialogs that can't use the shared `Modal` (e.g. the admin-portal modals). Stands down while a `confirmAction()` request is pending, so Escape on the confirm never also closes the dialog that asked | | `useCtrlScrollZoom` | Ctrl+wheel zoom keeping the date under the cursor fixed | -| `useScrollSync` | Bidirectional vertical scroll sync between two elements (used by GanttContainer and the Staff/Equipment/CrossSite/Archived views) | +| `useScrollSync` | Bidirectional vertical scroll sync between two elements (used by GanttContainer and the Staff/Equipment/CrossSite/Archived views). Three behaviours: no echo (a write remembers the value and ignores the follower's matching `scroll` event, so the follower never snaps the source back), a non-passive `wheel` handler that moves both panes in the same frame (Ctrl+wheel and horizontal deltas are left alone), and equalised scroll range — it measures both panes' natural `scrollHeight - clientHeight` and pads the shorter one (`ResizeObserver`, `window` resize fallback), which replaced the fixed 8 px bottom padding that only matched a classic horizontal scrollbar | | `useWorkloadCalculation` | Per-cell staff workload for the Staff heatmap (allocations, vacations incl. recurring, visual states). Only absences whose type `counts_as_unavailable` (or untyped ones) zero capacity. An optional closure lookup (`ClosureLookup`, usually `getClosureOn` bound to the person's sites) zeroes capacity on site-closure days → `vacation`/`conflict` state, `closureName`/`closureColor`, tooltip *Site closed: …* | | `useEquipmentOverlaps` / `useEquipmentTodayStatus` | Equipment double-booking detection / today-status (`blocked` > `booked` > available) — two hooks in one file | | `useTagFilter` | `useSiteTagFilter()` (the current site's `viewStore.tagFilters` entry, shared by the Gantt, the Kanban board and `TagFilterDropdown`; ids of tags deleted or re-scoped away from the site are ignored on read and pruned on the next write) + `useTagFilteredProjects(projects)` | diff --git a/UI_GUIDELINES.md b/UI_GUIDELINES.md index 7457bdce..6b25004e 100644 --- a/UI_GUIDELINES.md +++ b/UI_GUIDELINES.md @@ -619,6 +619,12 @@ from the inline `project.tags[].color`, which is stale during What-If. by those. - Bars take the item's saved colour (§2.6); milestones are diamonds; the critical path highlights bars with `totalFloat <= 0`. +- The tree panel and the timeline body are two vertical scroll containers kept together by + `useScrollSync` (also the Staff, Equipment, Cross-site and Archived views): wheel and + trackpad move both panes in the same frame, `scroll` events (scrollbar drag, keyboard) mirror + without echo, and the hook pads the pane with the shorter scroll range so both reach the same + end. Never give one pane a fixed bottom padding to "match the scrollbar" — the scrollbar's + height is 0 with overlay scrollbars, and the padding then misaligns the last rows. - A bar is draggable or resizable only when it has handlers; those bars get the `interactive` class, which sets `touch-action: none` and disables text selection. Read-only bars keep scrolling. diff --git a/VERSION b/VERSION index 81c871de..4dae2985 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.10.0 +1.10.1 diff --git a/frontend/src/components/gantt/ProjectPanel/ProjectPanel.module.css b/frontend/src/components/gantt/ProjectPanel/ProjectPanel.module.css index ff515855..d6d993a2 100644 --- a/frontend/src/components/gantt/ProjectPanel/ProjectPanel.module.css +++ b/frontend/src/components/gantt/ProjectPanel/ProjectPanel.module.css @@ -201,8 +201,6 @@ flex: 1; overflow-y: auto; overflow-x: hidden; - /* Add 8px padding at bottom to match horizontal scrollbar in timeline */ - padding-bottom: 8px; } .body::-webkit-scrollbar { diff --git a/frontend/src/components/views/ArchivedView/ArchivedView.module.css b/frontend/src/components/views/ArchivedView/ArchivedView.module.css index 0264a7cb..1b365a6c 100644 --- a/frontend/src/components/views/ArchivedView/ArchivedView.module.css +++ b/frontend/src/components/views/ArchivedView/ArchivedView.module.css @@ -35,7 +35,6 @@ flex: 1; overflow-y: auto; overflow-x: hidden; - padding-bottom: 8px; } .panelContent::-webkit-scrollbar { diff --git a/frontend/src/components/views/CrossSiteView/CrossSiteView.module.css b/frontend/src/components/views/CrossSiteView/CrossSiteView.module.css index 08a23034..7871a107 100644 --- a/frontend/src/components/views/CrossSiteView/CrossSiteView.module.css +++ b/frontend/src/components/views/CrossSiteView/CrossSiteView.module.css @@ -38,7 +38,6 @@ flex: 1; overflow-y: auto; overflow-x: hidden; - padding-bottom: 8px; } .panelBody::-webkit-scrollbar { diff --git a/frontend/src/components/views/EquipmentView/EquipmentView.module.css b/frontend/src/components/views/EquipmentView/EquipmentView.module.css index 6115d255..06a51752 100644 --- a/frontend/src/components/views/EquipmentView/EquipmentView.module.css +++ b/frontend/src/components/views/EquipmentView/EquipmentView.module.css @@ -423,7 +423,6 @@ flex: 1; overflow-y: auto; overflow-x: hidden; - padding-bottom: 8px; } .panelBody::-webkit-scrollbar { diff --git a/frontend/src/components/views/StaffView/StaffView.module.css b/frontend/src/components/views/StaffView/StaffView.module.css index eba1aab8..4d292596 100644 --- a/frontend/src/components/views/StaffView/StaffView.module.css +++ b/frontend/src/components/views/StaffView/StaffView.module.css @@ -82,7 +82,6 @@ flex: 1; overflow-y: auto; overflow-x: hidden; - padding-bottom: 8px; } .panelBody::-webkit-scrollbar { diff --git a/frontend/src/hooks/__tests__/useScrollSync.test.ts b/frontend/src/hooks/__tests__/useScrollSync.test.ts index 45a69cee..669df5c6 100644 --- a/frontend/src/hooks/__tests__/useScrollSync.test.ts +++ b/frontend/src/hooks/__tests__/useScrollSync.test.ts @@ -1,46 +1,309 @@ /** - * useScrollSync: vertical scroll position mirrored between the tree panel - * and the timeline body, without echo loops. + * useScrollSync: vertical scroll position mirrored between the tree panel and + * the timeline body without echo, wheel-driven in one frame, with a measured + * bottom padding that equalises the two scroll ranges. */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, afterEach, vi } from 'vitest'; import { renderHook } from '@testing-library/react'; import { useScrollSync } from '../useScrollSync'; const nextFrame = () => new Promise((r) => requestAnimationFrame(() => r(null))); -describe('useScrollSync', () => { - it('mirrors scrollTop both ways', async () => { - const a = document.createElement('div'); - const b = document.createElement('div'); - renderHook(() => useScrollSync({ current: a }, { current: b })); +function pair() { + const a = document.createElement('div'); + const b = document.createElement('div'); + a.appendChild(document.createElement('div')); + b.appendChild(document.createElement('div')); + return { a, b }; +} - a.scrollTop = 120; - a.dispatchEvent(new Event('scroll')); +/** scrollHeight = base() + the inline padding the hook applied; fixed clientHeight. */ +function setRange(el: HTMLElement, base: () => number, client: number) { + Object.defineProperty(el, 'scrollHeight', { + configurable: true, + get: () => base() + (parseFloat(el.style.paddingBottom) || 0), + }); + Object.defineProperty(el, 'clientHeight', { configurable: true, get: () => client }); +} + +function scrollTo(el: HTMLElement, top: number) { + el.scrollTop = top; + el.dispatchEvent(new Event('scroll')); +} + +function wheel(el: HTMLElement, init: WheelEventInit) { + const e = new WheelEvent('wheel', { cancelable: true, bubbles: true, ...init }); + el.dispatchEvent(e); + return e; +} + +const mount = (a: HTMLElement | null, b: HTMLElement | null) => + renderHook(() => useScrollSync({ current: a }, { current: b })); + +class FakeResizeObserver { + static instances: FakeResizeObserver[] = []; + observed: Element[] = []; + disconnected = false; + constructor(public callback: () => void) { + FakeResizeObserver.instances.push(this); + } + observe(el: Element) { + this.observed.push(el); + } + disconnect() { + this.disconnected = true; + } + unobserve() {} +} + +afterEach(() => { + FakeResizeObserver.instances = []; + vi.unstubAllGlobals(); +}); + +describe('useScrollSync — scroll path', () => { + it('mirrors scrollTop both ways', () => { + const { a, b } = pair(); + mount(a, b); + scrollTo(a, 120); expect(b.scrollTop).toBe(120); - // the echo scroll on b is swallowed while syncing - b.scrollTop = 5; - b.dispatchEvent(new Event('scroll')); - expect(a.scrollTop).toBe(120); + b.dispatchEvent(new Event('scroll')); // consume the echo + scrollTo(b, 40); + expect(a.scrollTop).toBe(40); + }); - await nextFrame(); - b.scrollTop = 40; + it('swallows its own echo but propagates genuine scrolls', () => { + const { a, b } = pair(); + mount(a, b); + scrollTo(a, 120); + expect(b.scrollTop).toBe(120); + + // echo on b: a was moved meanwhile, and must not be written back to 120 + a.scrollTop = 130; b.dispatchEvent(new Event('scroll')); + expect(a.scrollTop).toBe(130); + + // a genuine b scroll afterwards moves a + scrollTo(b, 40); expect(a.scrollTop).toBe(40); }); - it('does nothing without both elements and detaches on unmount', () => { - const a = document.createElement('div'); - const { unmount } = renderHook(() => useScrollSync({ current: a }, { current: null })); - a.scrollTop = 10; + it('propagates a different value while an expectation is pending', () => { + const { a, b } = pair(); + mount(a, b); + scrollTo(a, 200); // b expects 200 + scrollTo(b, 90); // the user scrolled b before the echo arrived + expect(a.scrollTop).toBe(90); + }); + + it('treats a fractional echo (< 1 px) as its own', () => { + const { a, b } = pair(); + mount(a, b); + scrollTo(a, 120); + a.scrollTop = 300; + scrollTo(b, 120.4); + expect(a.scrollTop).toBe(300); + }); + + it('does not write when the panes differ by less than 1 px', () => { + const { a, b } = pair(); + mount(a, b); + b.scrollTop = 50.5; + const setter = vi.spyOn(b, 'scrollTop', 'set'); + scrollTo(a, 50); + expect(setter).not.toHaveBeenCalled(); + }); +}); + +describe('useScrollSync — wheel path', () => { + function laidOut() { + const { a, b } = pair(); + setRange(a, () => 1000, 400); + setRange(b, () => 1000, 400); + mount(a, b); + return { a, b }; + } + + it('moves both panes synchronously and cancels the native scroll', () => { + const { a, b } = laidOut(); + const e = wheel(a, { deltaY: 100 }); + expect(a.scrollTop).toBe(100); + expect(b.scrollTop).toBe(100); + expect(e.defaultPrevented).toBe(true); + // the follow-up scroll events are our own echoes + b.scrollTop = 100; a.dispatchEvent(new Event('scroll')); - unmount(); + b.dispatchEvent(new Event('scroll')); + expect(a.scrollTop).toBe(100); + }); + + it('works from the other pane too', () => { + const { a, b } = laidOut(); + wheel(b, { deltaY: 30 }); + expect(a.scrollTop).toBe(30); + expect(b.scrollTop).toBe(30); + }); + + it('clamps at 0 and at the maximum, and does not cancel at the end', () => { + const { a, b } = laidOut(); + wheel(a, { deltaY: 50 }); + wheel(a, { deltaY: -500 }); + expect(a.scrollTop).toBe(0); + expect(b.scrollTop).toBe(0); + + wheel(a, { deltaY: 5000 }); + expect(a.scrollTop).toBe(600); + expect(b.scrollTop).toBe(600); + + const atEnd = wheel(a, { deltaY: 100 }); + expect(atEnd.defaultPrevented).toBe(false); + expect(a.scrollTop).toBe(600); + + const atTop = (() => { + a.scrollTop = 0; + return wheel(a, { deltaY: -10 }); + })(); + expect(atTop.defaultPrevented).toBe(false); + }); + + it.each([ + ['ctrl (zoom)', { deltaY: 100, ctrlKey: true }], + ['shift (horizontal)', { deltaY: 100, shiftKey: true }], + ['horizontal only', { deltaX: 100 }], + ['diagonal', { deltaX: 3, deltaY: 100 }], + ['zero delta', { deltaY: 0 }], + ])('leaves %s wheel events to the browser', (_label, init) => { + const { a, b } = laidOut(); + const e = wheel(a, init); + expect(e.defaultPrevented).toBe(false); + expect(a.scrollTop).toBe(0); + expect(b.scrollTop).toBe(0); + }); + it('normalises line and page delta modes', () => { + const { a, b } = laidOut(); + wheel(a, { deltaY: 3, deltaMode: 1 }); + expect(a.scrollTop).toBe(48); + expect(b.scrollTop).toBe(48); + wheel(a, { deltaY: 1, deltaMode: 2 }); + expect(a.scrollTop).toBe(448); + expect(b.scrollTop).toBe(448); + }); +}); + +describe('useScrollSync — equal scroll range', () => { + it('pads the pane with the smaller range, and clears it when the ranges match', async () => { + vi.stubGlobal('ResizeObserver', FakeResizeObserver); + const { a, b } = pair(); + let bBase = 992; + setRange(a, () => 1000, 400); // natural range 600 + setRange(b, () => bBase, 400); // natural range 592 + mount(a, b); + + expect(b.style.paddingBottom).toBe('8px'); + expect(a.style.paddingBottom).toBe('0px'); + + const [observer] = FakeResizeObserver.instances; + expect(observer.observed).toEqual([a, b, a.firstElementChild, b.firstElementChild]); + + // remeasure excludes the 8 px it applied: still 8 px + observer.callback(); + await nextFrame(); + expect(b.style.paddingBottom).toBe('8px'); + + bBase = 1000; + observer.callback(); + observer.callback(); // coalesced into one frame + await nextFrame(); + expect(b.style.paddingBottom).toBe('0px'); + expect(a.style.paddingBottom).toBe('0px'); + + // the other way round + bBase = 1010; + observer.callback(); + await nextFrame(); + expect(a.style.paddingBottom).toBe('10px'); + expect(b.style.paddingBottom).toBe('0px'); + }); + + it('observes only the panes when they have no content child', () => { + vi.stubGlobal('ResizeObserver', FakeResizeObserver); + const a = document.createElement('div'); const b = document.createElement('div'); - const c = document.createElement('div'); - const second = renderHook(() => useScrollSync({ current: b }, { current: c })); - second.unmount(); - b.scrollTop = 33; - b.dispatchEvent(new Event('scroll')); - expect(c.scrollTop).toBe(0); + mount(a, b); + expect(FakeResizeObserver.instances[0].observed).toEqual([a, b]); + }); + + it('falls back to window resize without ResizeObserver', async () => { + vi.stubGlobal('ResizeObserver', undefined); + const { a, b } = pair(); + let aBase = 1000; + setRange(a, () => aBase, 400); + setRange(b, () => 1000, 400); + const removeSpy = vi.spyOn(window, 'removeEventListener'); + const { unmount } = mount(a, b); + expect(a.style.paddingBottom).toBe('0px'); + + aBase = 995; + window.dispatchEvent(new Event('resize')); + await nextFrame(); + expect(a.style.paddingBottom).toBe('5px'); + + unmount(); + expect(removeSpy).toHaveBeenCalledWith('resize', expect.any(Function)); + expect(a.style.paddingBottom).toBe(''); + removeSpy.mockRestore(); + }); + + it('leaves panes that are not laid out alone', () => { + const { a, b } = pair(); + mount(a, b); + expect(a.style.paddingBottom).toBe(''); + expect(b.style.paddingBottom).toBe(''); + }); + + it('cancels a pending re-measure and resets paddings on unmount', async () => { + vi.stubGlobal('ResizeObserver', FakeResizeObserver); + const { a, b } = pair(); + let bBase = 992; + setRange(a, () => 1000, 400); + setRange(b, () => bBase, 400); + const { unmount } = mount(a, b); + expect(b.style.paddingBottom).toBe('8px'); + + const [observer] = FakeResizeObserver.instances; + bBase = 900; + observer.callback(); + unmount(); + await nextFrame(); + + expect(observer.disconnected).toBe(true); + expect(a.style.paddingBottom).toBe(''); + expect(b.style.paddingBottom).toBe(''); + }); +}); + +describe('useScrollSync — lifecycle', () => { + it('does nothing without both elements', () => { + const a = document.createElement('div'); + const { unmount } = mount(a, null); + scrollTo(a, 10); + expect(a.scrollTop).toBe(10); + unmount(); + }); + + it('detaches scroll and wheel listeners on unmount', () => { + const { a, b } = pair(); + setRange(a, () => 1000, 400); + setRange(b, () => 1000, 400); + const { unmount } = mount(a, b); + unmount(); + + scrollTo(a, 33); + expect(b.scrollTop).toBe(0); + const e = wheel(b, { deltaY: 100 }); + expect(e.defaultPrevented).toBe(false); + expect(a.scrollTop).toBe(33); + expect(a.style.paddingBottom).toBe(''); }); }); diff --git a/frontend/src/hooks/useScrollSync.ts b/frontend/src/hooks/useScrollSync.ts index 201fa603..cf65ca9d 100644 --- a/frontend/src/hooks/useScrollSync.ts +++ b/frontend/src/hooks/useScrollSync.ts @@ -1,12 +1,35 @@ /** * useScrollSync Hook - * Synchronizes vertical scroll position (scrollTop) between two elements, - * bidirectionally. Both containers should have the same content height for - * proper row alignment. + * + * Keeps the vertical scroll position (scrollTop) of two panes — the left tree + * panel and the timeline body — identical, in both directions. + * + * No echo. Every scrollTop the hook writes is remembered as the value expected + * for that element; the element's next `scroll` event reporting that value + * (within 1 px, for fractional scrolls) is our own write coming back and is + * consumed. Any other value is a genuine scroll and is mirrored. (A rAF flag + * used to do this, but the follower's scroll event arrives a frame later, after + * the flag reset, and copied its older position back onto the leading pane.) + * + * Wheel. A vertical wheel over either pane moves both panes synchronously in + * the same frame and cancels the native scroll, so the follower never lags a + * frame behind. Ctrl+wheel (zoom, `useCtrlScrollZoom`), Shift+wheel and any + * event with a horizontal component are left to the browser; they still sync + * through the scroll path. At the end of the range the event is not cancelled, + * so the page can scroll. + * + * Equal scroll range. The timeline's horizontal scrollbar takes 8 px of layout + * with classic scrollbars and 0 with overlay ones, so a fixed CSS padding can't + * match the two ranges. The hook measures each pane's natural range (excluding + * the padding it applied itself) and gives the pane with the smaller range a + * bottom padding equal to the difference — on mount, then one frame after any + * resize of the panes or their content. */ import { useEffect, RefObject } from 'react'; +const LINE_HEIGHT_PX = 16; + export function useScrollSync( ref1: RefObject, ref2: RefObject @@ -17,32 +40,88 @@ export function useScrollSync( if (!el1 || !el2) return; - let isSyncing = false; + const expected = new Map(); + const pads = new Map(); + let frame: number | null = null; + + const write = (el: HTMLElement, value: number) => { + const before = el.scrollTop; + el.scrollTop = value; + if (el.scrollTop !== before) expected.set(el, el.scrollTop); + }; + + const onScroll = (source: HTMLElement, target: HTMLElement) => { + const pending = expected.get(source); + expected.delete(source); + if (pending !== undefined && Math.abs(source.scrollTop - pending) < 1) return; + if (Math.abs(target.scrollTop - source.scrollTop) >= 1) { + write(target, source.scrollTop); + } + }; + + const onWheel = (source: HTMLElement, target: HTMLElement, e: WheelEvent) => { + if (e.ctrlKey || e.shiftKey || e.deltaY === 0 || e.deltaX !== 0) return; + const unit = + e.deltaMode === 1 ? LINE_HEIGHT_PX : e.deltaMode === 2 ? source.clientHeight : 1; + const max = Math.max(0, source.scrollHeight - source.clientHeight); + const next = Math.min(max, Math.max(0, source.scrollTop + e.deltaY * unit)); + if (next === source.scrollTop) return; + write(source, next); + write(target, next); + e.preventDefault(); + }; + + const applyPad = (el: HTMLElement, px: number) => { + if (pads.get(el) === px) return; + pads.set(el, px); + el.style.paddingBottom = `${px}px`; + }; - const handleScroll1 = () => { - if (isSyncing) return; - isSyncing = true; - el2.scrollTop = el1.scrollTop; - requestAnimationFrame(() => { - isSyncing = false; - }); + const equalise = () => { + frame = null; + if (el1.scrollHeight === 0 && el2.scrollHeight === 0) return; + const range = (el: HTMLElement) => + Math.max(0, el.scrollHeight - el.clientHeight - (pads.get(el) ?? 0)); + const diff = range(el1) - range(el2); + applyPad(el1, diff < 0 ? -diff : 0); + applyPad(el2, diff > 0 ? diff : 0); }; - const handleScroll2 = () => { - if (isSyncing) return; - isSyncing = true; - el1.scrollTop = el2.scrollTop; - requestAnimationFrame(() => { - isSyncing = false; - }); + const schedule = () => { + if (frame === null) frame = requestAnimationFrame(equalise); }; - el1.addEventListener('scroll', handleScroll1, { passive: true }); - el2.addEventListener('scroll', handleScroll2, { passive: true }); + const scroll1 = () => onScroll(el1, el2); + const scroll2 = () => onScroll(el2, el1); + const wheel1 = (e: WheelEvent) => onWheel(el1, el2, e); + const wheel2 = (e: WheelEvent) => onWheel(el2, el1, e); + + el1.addEventListener('scroll', scroll1, { passive: true }); + el2.addEventListener('scroll', scroll2, { passive: true }); + el1.addEventListener('wheel', wheel1, { passive: false }); + el2.addEventListener('wheel', wheel2, { passive: false }); + + let observer: ResizeObserver | null = null; + if (typeof ResizeObserver === 'undefined') { + window.addEventListener('resize', schedule); + } else { + observer = new ResizeObserver(schedule); + for (const el of [el1, el2, el1.firstElementChild, el2.firstElementChild]) { + if (el) observer.observe(el); + } + } + + equalise(); return () => { - el1.removeEventListener('scroll', handleScroll1); - el2.removeEventListener('scroll', handleScroll2); + el1.removeEventListener('scroll', scroll1); + el2.removeEventListener('scroll', scroll2); + el1.removeEventListener('wheel', wheel1); + el2.removeEventListener('wheel', wheel2); + if (observer) observer.disconnect(); + else window.removeEventListener('resize', schedule); + if (frame !== null) cancelAnimationFrame(frame); + for (const el of pads.keys()) el.style.paddingBottom = ''; }; }, [ref1, ref2]); }