From 5f0ea16c68ac5018b70c5d723166866cea0f836d Mon Sep 17 00:00:00 2001 From: ciregenz Date: Sun, 6 Sep 2026 17:58:24 -0700 Subject: [PATCH] [eric] canvas: a composer past its cap scrolls; the wheel walk caches only the overflow style and measures capacity live, the cached verdict outlived the box it judged (ENG-485) Co-Authored-By: Claude Fable 5.1 --- .../hooks/interaction/scrollContainer.test.ts | 31 +++++++++++++++ .../hooks/interaction/scrollContainer.ts | 38 +++++++++++++++++++ .../hooks/interaction/useCanvasControls.ts | 25 ++---------- 3 files changed, 73 insertions(+), 21 deletions(-) create mode 100644 frontend/src/app/pages/Dashboard/hooks/interaction/scrollContainer.test.ts create mode 100644 frontend/src/app/pages/Dashboard/hooks/interaction/scrollContainer.ts diff --git a/frontend/src/app/pages/Dashboard/hooks/interaction/scrollContainer.test.ts b/frontend/src/app/pages/Dashboard/hooks/interaction/scrollContainer.test.ts new file mode 100644 index 00000000..6482a798 --- /dev/null +++ b/frontend/src/app/pages/Dashboard/hooks/interaction/scrollContainer.test.ts @@ -0,0 +1,31 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { isScrollContainer, OverflowVerdict } from './scrollContainer'; + +function box(scrollHeight: number, overflowY = 'auto') { + const el = { scrollHeight, clientHeight: 220, scrollWidth: 300, clientWidth: 300 } as unknown as HTMLElement; + let styleReads = 0; + const computeStyle = () => { styleReads++; return { overflowY, overflowX: 'visible' }; }; + return { el, computeStyle, reads: () => styleReads }; +} + +test('a composer that was short at the first wheel scrolls once it has grown past its cap', () => { + const cache = new WeakMap(); + const b = box(200); + assert.equal(isScrollContainer(b.el, cache, b.computeStyle), false); + (b.el as unknown as { scrollHeight: number }).scrollHeight = 900; + assert.equal(isScrollContainer(b.el, cache, b.computeStyle), true); +}); + +test('the overflow style is read once per node, the capacity every time', () => { + const cache = new WeakMap(); + const b = box(900); + for (let i = 0; i < 5; i++) assert.equal(isScrollContainer(b.el, cache, b.computeStyle), true); + assert.equal(b.reads(), 1); +}); + +test('a clipped box never takes the wheel, however tall its content', () => { + const cache = new WeakMap(); + const b = box(5000, 'hidden'); + assert.equal(isScrollContainer(b.el, cache, b.computeStyle), false); +}); diff --git a/frontend/src/app/pages/Dashboard/hooks/interaction/scrollContainer.ts b/frontend/src/app/pages/Dashboard/hooks/interaction/scrollContainer.ts new file mode 100644 index 00000000..37af276d --- /dev/null +++ b/frontend/src/app/pages/Dashboard/hooks/interaction/scrollContainer.ts @@ -0,0 +1,38 @@ +// Whether a wheel over this element should scroll IT instead of moving the canvas. The overflow +// style is cached per node (getComputedStyle walks were the dominant cost of trackpad nav); the +// capacity is read live every time, because a composer or transcript that was short when first +// wheeled grows later, and a cached "cannot scroll" from that first wheel stuck for the life of the +// canvas (Haik, exp.8: a composer past its cap would not scroll, the canvas panned instead). +export type OverflowVerdict = 'scrolls' | 'clips'; + +export interface ScrollBox { + scrollHeight: number; + clientHeight: number; + scrollWidth: number; + clientWidth: number; +} + +type OverflowStyle = Pick; + +export function overflowVerdict(style: OverflowStyle): OverflowVerdict { + const scrolls = (v: string) => v === 'auto' || v === 'scroll'; + return scrolls(style.overflowY) || scrolls(style.overflowX) ? 'scrolls' : 'clips'; +} + +export function hasScrollCapacity(el: ScrollBox): boolean { + return el.scrollHeight > el.clientHeight || el.scrollWidth > el.clientWidth; +} + +export function isScrollContainer( + el: HTMLElement, + cache: WeakMap, + computeStyle: (el: HTMLElement) => OverflowStyle = (e) => getComputedStyle(e), +): boolean { + if (!hasScrollCapacity(el)) return false; + let verdict = cache.get(el); + if (verdict === undefined) { + verdict = overflowVerdict(computeStyle(el)); + cache.set(el, verdict); + } + return verdict === 'scrolls'; +} diff --git a/frontend/src/app/pages/Dashboard/hooks/interaction/useCanvasControls.ts b/frontend/src/app/pages/Dashboard/hooks/interaction/useCanvasControls.ts index ee2ffaed..8ffd4bbe 100644 --- a/frontend/src/app/pages/Dashboard/hooks/interaction/useCanvasControls.ts +++ b/frontend/src/app/pages/Dashboard/hooks/interaction/useCanvasControls.ts @@ -11,6 +11,7 @@ import { applyBrowserZoom } from '@/shared/browserZoom'; import { syncTiledGeometry } from '../../canvas/tiledGeometry'; import { revealZoom, REVEAL_MIN_ZOOM } from '../../canvas/revealZoom'; import { classifyWheelDevice } from './classifyWheelDevice'; +import { isScrollContainer, OverflowVerdict } from './scrollContainer'; // Surfaces that are WINDOWS, not canvas cards: they behave like an OS window, so a wheel inside one // belongs to it whether or not you clicked in first. Canvas cards (agent, browser, view) keep the @@ -346,8 +347,8 @@ export function useCanvasControls( wheelRafId = requestAnimationFrame(flushWheel); }; - // Cache "is this element a scrollable child" decision per node. The Cache getComputedStyle ancestor walks; uncached was the dominant cost of trackpad two-finger nav. ResizeObserver below invalidates on scroll-capacity change. - const scrollableCache: WeakMap = new WeakMap(); + // The overflow style is cached per node, the capacity is read live; scrollContainer.ts says why. + const overflowCache: WeakMap = new WeakMap(); const gesture: WheelGesture = { owner: null, at: 0 }; const onWheel = (e: WheelEvent) => { @@ -399,25 +400,7 @@ export function useCanvasControls( const isTrackpadScroll = classifyTrackpad(e, e.deltaX, e.deltaY); let target = e.target as HTMLElement | null; while (target && target !== el) { - let cls = scrollableCache.get(target); - if (cls === undefined) { - const couldScroll = - target.scrollHeight > target.clientHeight || - target.scrollWidth > target.clientWidth; - if (couldScroll) { - const style = getComputedStyle(target); - const oy = style.overflowY; - const ox = style.overflowX; - const isOverflowScrollable = - oy === 'auto' || oy === 'scroll' || ox === 'auto' || ox === 'scroll'; - cls = isOverflowScrollable ? 'scrollable' : 'not'; - } else { - cls = 'not'; - } - scrollableCache.set(target, cls); - } - - if (cls === 'scrollable' && !isModifierWheel && !canvasOwnsGesture) { + if (isScrollContainer(target, overflowCache) && !isModifierWheel && !canvasOwnsGesture) { // Google Maps model: plain scroll acts on the CANVAS over a CARD (chat, scheduled task) UNLESS you've clicked INTO it. Only a card that isn't scroll-focused diverts to the canvas gesture; non-card scrollable UI (dropdowns, menus, nested panels) always scrolls natively, and a focused card scrolls its content. const cardEl = target.closest('[data-select-id]'); const cardId = cardEl?.getAttribute('data-select-id') ?? null;