diff --git a/dashboard/src/pages/Chat/hooks/scrollFreeMode.test.ts b/dashboard/src/pages/Chat/hooks/scrollFreeMode.test.ts index 0e3bad0..63d28a4 100644 --- a/dashboard/src/pages/Chat/hooks/scrollFreeMode.test.ts +++ b/dashboard/src/pages/Chat/hooks/scrollFreeMode.test.ts @@ -3,26 +3,60 @@ import { shouldEnterFreeModeOnScrollUp } from "./scrollFreeMode"; describe("shouldEnterFreeModeOnScrollUp", () => { it("ignores tiny dips while still near the bottom (Safari reflow)", () => { - expect(shouldEnterFreeModeOnScrollUp({ upDelta: 2, atBottom: true })).toBe( - false, - ); + expect( + shouldEnterFreeModeOnScrollUp({ upDelta: 2, atBottom: true, gapToBottom: 2 }), + ).toBe(false); }); it("leaves follow on intentional upward scroll even near the bottom", () => { - expect(shouldEnterFreeModeOnScrollUp({ upDelta: 20, atBottom: true })).toBe( - true, - ); + expect( + shouldEnterFreeModeOnScrollUp({ + upDelta: 20, + atBottom: true, + gapToBottom: 20, + }), + ).toBe(true); }); it("always leaves follow once outside the bottom sticky zone", () => { - expect(shouldEnterFreeModeOnScrollUp({ upDelta: 3, atBottom: false })).toBe( - true, - ); + expect( + shouldEnterFreeModeOnScrollUp({ + upDelta: 3, + atBottom: false, + gapToBottom: 300, + }), + ).toBe(true); }); it("ignores non-upward movement", () => { - expect(shouldEnterFreeModeOnScrollUp({ upDelta: 0, atBottom: false })).toBe( - false, - ); + expect( + shouldEnterFreeModeOnScrollUp({ + upDelta: 0, + atBottom: false, + gapToBottom: 300, + }), + ).toBe(false); + }); + + it("stays in follow when a layout clamp lands exactly at the bottom", () => { + // Closing the file dock grows the viewport; the browser rewrites scrollTop + // to the new max. The *resulting* gap is ~0, so this is not user intent. + expect( + shouldEnterFreeModeOnScrollUp({ + upDelta: 200, + atBottom: true, + gapToBottom: 0, + }), + ).toBe(false); + }); + + it("leaves follow when the scroll-up genuinely moves away from the bottom", () => { + expect( + shouldEnterFreeModeOnScrollUp({ + upDelta: 30, + atBottom: false, + gapToBottom: 120, + }), + ).toBe(true); }); }); diff --git a/dashboard/src/pages/Chat/hooks/scrollFreeMode.ts b/dashboard/src/pages/Chat/hooks/scrollFreeMode.ts index 4f97361..7d3fecd 100644 --- a/dashboard/src/pages/Chat/hooks/scrollFreeMode.ts +++ b/dashboard/src/pages/Chat/hooks/scrollFreeMode.ts @@ -5,15 +5,27 @@ * must NOT leave follow mode. Clear upward movement — even inside that zone — * must enter free mode, otherwise ResizeObserver follow-pins yank the user * back to the bottom and "scroll up to load earlier" never works. + * + * Exception: a scroll-up whose *resulting* position is still within the bottom + * band (`gapToBottom <= atBottomBandPx`) is a layout clamp — the browser + * rewrote scrollTop because the viewport grew or content shrank (e.g. closing + * the file dock returns the chat to full height). That is not user intent and + * must not surface the ↓ control while the user is still pinned to the bottom. */ export function shouldEnterFreeModeOnScrollUp(opts: { upDelta: number; atBottom: boolean; + /** Distance from the bottom at the scroll's resulting position. */ + gapToBottom: number; /** Ignore sub-pixel / reflow noise. */ intentionalUpPx?: number; + /** Scroll-ups landing within this band count as "still at the bottom". */ + atBottomBandPx?: number; }): boolean { const intentionalUpPx = opts.intentionalUpPx ?? 8; + const atBottomBandPx = opts.atBottomBandPx ?? 12; if (opts.upDelta <= 1) return false; + if (opts.gapToBottom <= atBottomBandPx) return false; if (!opts.atBottom) return true; return opts.upDelta >= intentionalUpPx; } diff --git a/dashboard/src/pages/Chat/hooks/useAutoScroll.test.ts b/dashboard/src/pages/Chat/hooks/useAutoScroll.test.ts index 6328ef0..b585d3e 100644 --- a/dashboard/src/pages/Chat/hooks/useAutoScroll.test.ts +++ b/dashboard/src/pages/Chat/hooks/useAutoScroll.test.ts @@ -35,6 +35,30 @@ function makeScroller({ return el; } +/** + * Replace the no-op ResizeObserver stub (from src/test/setup.ts) with one we + * can fire manually, mirroring the real browser delivering RO callbacks after + * the dock resizes the scroller. + */ +function installCallableResizeObserver() { + const instances: Array<{ emit: () => void }> = []; + class MockResizeObserver { + private readonly cb: ResizeObserverCallback; + constructor(cb: ResizeObserverCallback) { + this.cb = cb; + instances.push(this); + } + observe() {} + unobserve() {} + disconnect() {} + emit() { + this.cb([], this as unknown as ResizeObserver); + } + } + vi.stubGlobal("ResizeObserver", MockResizeObserver); + return instances; +} + describe("useAutoScroll", () => { beforeEach(() => { vi.spyOn(window, "requestAnimationFrame").mockImplementation((cb) => { @@ -244,6 +268,154 @@ describe("useAutoScroll", () => { vi.useRealTimers(); }); + it("does not show the jump-to-bottom control after a layout clamp at the bottom", () => { + // Regression: closing the file dock grows the viewport, so the browser + // clamps scrollTop to the new max and fires a fake "scroll-up". The user + // never scrolled — the ↓ control must not appear and mode stays follow. + vi.useFakeTimers(); + const container = makeScroller({ + scrollHeight: 1000, + clientHeight: 200, + scrollTop: 800, + }); + const containerRef = { current: container }; + const endRef = { current: document.createElement("div") }; + endRef.current.scrollIntoView = vi.fn(); + + const { result } = renderHook(() => + useAutoScroll({ containerRef, endRef, deps: [] }), + ); + + act(() => { + vi.advanceTimersByTime(400); + }); + + // Dock closes → max scrollTop drops from 800 to 600; the browser rewrites + // scrollTop to 600 (the setter clamps it). + Object.defineProperty(container, "scrollHeight", { + value: 800, + configurable: true, + }); + act(() => { + container.scrollTop = 800; + container.dispatchEvent(new Event("scroll", { bubbles: true })); + }); + + expect(result.current.isFollowMode).toBe(true); + expect(result.current.showScrollBtn).toBe(false); + vi.useRealTimers(); + }); + + it("stays at the bottom with no button through a dock open/close resize cycle", () => { + // Regression (real browser, not just the stub): opening the file dock + // shrinks the chat scroller, closing it grows it back. The browser then + // clamps scrollTop and fires a fake "scroll-up" plus a ResizeObserver + // callback. Neither may surface the ↓ control while the user is at the + // bottom. + vi.useFakeTimers(); + const roInstances = installCallableResizeObserver(); + const container = makeScroller({ + scrollHeight: 1000, + clientHeight: 200, + scrollTop: 800, + }); + const containerRef = { current: container }; + const endRef = { current: document.createElement("div") }; + endRef.current.scrollIntoView = vi.fn(); + + const { result } = renderHook(() => + useAutoScroll({ containerRef, endRef, deps: [] }), + ); + + act(() => { + vi.advanceTimersByTime(400); + }); + expect(result.current.isFollowMode).toBe(true); + expect(result.current.showScrollBtn).toBe(false); + + // Dock opens → scroller shrinks 200 → 120; follow pins to new max 880. + Object.defineProperty(container, "clientHeight", { + value: 120, + configurable: true, + }); + act(() => { + roInstances.forEach((ro) => ro.emit()); + }); + expect(result.current.isFollowMode).toBe(true); + expect(result.current.showScrollBtn).toBe(false); + expect(container.scrollTop).toBe(880); + + act(() => { + vi.advanceTimersByTime(400); + }); + + // Dock closes → scroller grows 120 → 200; max drops 880 → 800, so the + // browser clamps scrollTop to 800 (the setter clamps it). + Object.defineProperty(container, "clientHeight", { + value: 200, + configurable: true, + }); + act(() => { + container.scrollTop = 880; + container.dispatchEvent(new Event("scroll", { bubbles: true })); + }); + act(() => { + roInstances.forEach((ro) => ro.emit()); + }); + + expect(result.current.isFollowMode).toBe(true); + expect(result.current.showScrollBtn).toBe(false); + vi.useRealTimers(); + }); + + it("drops a stale jump-to-bottom control when a layout clamp lands at the bottom in free mode", () => { + // The user was already in free mode (scrolled up earlier), then a layout + // change clamps them back to the bottom. The ↓ control must disappear even + // though no scroll-down event fires to trigger the normal resume path. + vi.useFakeTimers(); + const container = makeScroller({ + scrollHeight: 1000, + clientHeight: 200, + scrollTop: 800, + }); + const containerRef = { current: container }; + const endRef = { current: document.createElement("div") }; + endRef.current.scrollIntoView = vi.fn(); + + const { result } = renderHook(() => + useAutoScroll({ containerRef, endRef, deps: [] }), + ); + + act(() => { + vi.advanceTimersByTime(400); + }); + + // User scrolls up genuinely → free mode, button shown. + act(() => { + container.scrollTop = 750; + container.dispatchEvent(new Event("scroll", { bubbles: true })); + }); + expect(result.current.isFollowMode).toBe(false); + expect(result.current.showScrollBtn).toBe(true); + + // Content shrinks (scrollHeight 1000 → 850): the browser clamps scrollTop + // from 750 to the new max 650 — a scroll-up event whose resulting position + // is exactly the bottom. Follow must resume and the button must hide, even + // though no scroll-down event fires the normal resume path. + Object.defineProperty(container, "scrollHeight", { + value: 850, + configurable: true, + }); + act(() => { + container.scrollTop = 750; // setter clamps to 650 + container.dispatchEvent(new Event("scroll", { bubbles: true })); + }); + + expect(result.current.isFollowMode).toBe(true); + expect(result.current.showScrollBtn).toBe(false); + vi.useRealTimers(); + }); + it("keeps the jump-to-bottom control after scrolling up inside the sticky zone", () => { vi.useFakeTimers(); // Regression: resume-follow used the loose 80px threshold, so a settle diff --git a/dashboard/src/pages/Chat/hooks/useAutoScroll.ts b/dashboard/src/pages/Chat/hooks/useAutoScroll.ts index 5d3863f..98ccbf3 100644 --- a/dashboard/src/pages/Chat/hooks/useAutoScroll.ts +++ b/dashboard/src/pages/Chat/hooks/useAutoScroll.ts @@ -311,7 +311,7 @@ export function useAutoScroll({ } }); // eslint-disable-next-line react-hooks/exhaustive-deps -- caller supplies dynamic dependency list - }, [...deps, scrollToBottomInFollowMode, skipNextDepsScrollRef]); + }, [...deps, scrollToBottomInFollowMode, skipNextDepsScrollRef, getScroller]); useEffect(() => { const container = getScroller(); @@ -361,6 +361,11 @@ export function useAutoScroll({ const upDelta = prev - cur; const scrolledUp = upDelta > 1; + // Distance from the bottom at the *resulting* position. Layout clamps + // (dock open/close, content shrink) rewrite scrollTop without moving the + // user — they land exactly at the bottom, so this stays ~0. + const gapToBottom = + container.scrollHeight - container.scrollTop - container.clientHeight; // Rubber-band at the bottom while pulling past the end must not clear // overscroll intent or yank into free mode. @@ -377,6 +382,8 @@ export function useAutoScroll({ shouldEnterFreeModeOnScrollUp({ upDelta, atBottom: isAtBottom(), + gapToBottom, + atBottomBandPx: FOLLOW_RESUME_THRESHOLD, }) ) { resetOverscroll(); @@ -396,6 +403,13 @@ export function useAutoScroll({ if (scrolledUp) { if (!bottomPullNoise) resetOverscroll(); + // A layout clamp (dock close / content shrink) can rewrite scrollTop + // straight to the bottom while we were already in free mode from an + // earlier gesture. The resulting position is authoritative — inside the + // bottom band counts as at the bottom, so drop the stale ↓ control. + if (isAtBottom(FOLLOW_RESUME_THRESHOLD)) { + enterFollowMode(); + } return; } @@ -480,8 +494,15 @@ export function useAutoScroll({ scrollToBottomInFollowMode(true, true); return; } - // Free mode: never hide ↓ from resize/layout. Only enterFollowMode does. - setShowScrollBtn(true); + // Free mode after a layout change: a dock close / content shrink can + // clamp the user right back to the bottom without firing a scroll event. + // Re-check position — inside the bottom band counts as at the bottom, so + // resume follow; only keep the ↓ control when genuinely away. + if (isAtBottom(FOLLOW_RESUME_THRESHOLD)) { + enterFollowMode(); + } else { + setShowScrollBtn(true); + } }; const ro = new ResizeObserver(handleResize);