Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/browser/components/ChatPane/ChatPane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ export const ChatPane: React.FC<ChatPaneProps> = (props) => {
jumpToBottom,
handleScroll,
markUserScrollIntent,
handleScrollContainerWheel,
handleScrollContainerMouseDown,
handleScrollContainerMouseMove,
handleScrollContainerMouseUp,
Expand Down Expand Up @@ -836,7 +837,7 @@ export const ChatPane: React.FC<ChatPaneProps> = (props) => {
<div className="mobile-header-spacer relative flex-1 overflow-hidden">
<div
ref={contentRef}
onWheel={markUserScrollIntent}
onWheel={handleScrollContainerWheel}
onMouseDown={handleScrollContainerMouseDown}
onMouseMove={handleScrollContainerMouseMove}
onMouseUp={handleScrollContainerMouseUp}
Expand Down
272 changes: 271 additions & 1 deletion src/browser/hooks/useAutoScroll.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test";
import { act, cleanup, renderHook } from "@testing-library/react";
import type { KeyboardEvent, MouseEvent, MutableRefObject, UIEvent } from "react";
import type { KeyboardEvent, MouseEvent, MutableRefObject, UIEvent, WheelEvent } from "react";

import { installDom } from "../../../tests/ui/dom";
import { mockScrollMetrics as attachScrollMetrics } from "../../../tests/ui/scrollMetrics";
Expand All @@ -22,6 +22,18 @@ function createMouseEvent(
} as unknown as MouseEvent<HTMLDivElement>;
}

function createWheelEvent(
element: HTMLDivElement,
options: { deltaX?: number; deltaY?: number } = {}
): WheelEvent<HTMLDivElement> {
return {
currentTarget: element,
target: element,
deltaX: options.deltaX ?? 0,
deltaY: options.deltaY ?? 0,
} as unknown as WheelEvent<HTMLDivElement>;
}

let scheduledFrames: Array<{ id: number; callback: FrameRequestCallback }> = [];
let nextFrameId = 1;

Expand Down Expand Up @@ -167,6 +179,264 @@ describe("useAutoScroll", () => {
}
});

test("small wheel-up tick releases the lock on the first event without snap-back", () => {
// Regression: a slow wheel-up gesture from the very bottom (~3-7 px per
// notch) used to keep the lock engaged because the user-intent branch
// treated "still ≀ USER_BOTTOM_RELOCK_THRESHOLD_PX from bottom" as
// "relock". The rAF settle tick then wrote scrollTop = max on the next
// frame, snapping the user back to the bottom mid-gesture.
const { result } = renderHook(() => useAutoScroll());
const element = document.createElement("div");
const metrics = attachScrollMetrics(element, {
scrollHeight: 1000,
clientHeight: 400,
initialScrollTop: 600,
});

const dateNowSpy = spyOn(Date, "now");
try {
let now = 1_000_000;
dateNowSpy.mockImplementation(() => now);

act(() => {
(result.current.contentRef as MutableRefObject<HTMLDivElement | null>).current = element;
});
expect(metrics.scrollTop).toBe(metrics.maxScrollTop);

// Single small wheel notch: scrollTop drops 5 px, well within the 8 px
// relock threshold. Lock must release on the first event.
metrics.setScrollTop(metrics.maxScrollTop - 5);
act(() => {
result.current.markUserScrollIntent();
now += 1;
result.current.handleScroll(createScrollEvent(element));
});
expect(result.current.autoScroll).toBe(false);

// Subsequent rAF ticks must not snap the user back to the bottom.
act(() => {
flushFrames(5);
});
expect(metrics.scrollTop).toBe(595);
} finally {
dateNowSpy.mockRestore();
}
});

test("continued slow wheel-up does not relock while still moving away from bottom", () => {
// Even after the lock releases, geometry alone (≀ 8 px from bottom) must
// not relock while the user is still scrolling upward in the same intent
// window. Direction matters: the second tick lands at 6 px from bottom
// but the user is moving away, so the lock must stay off.
const { result } = renderHook(() => useAutoScroll());
const element = document.createElement("div");
const metrics = attachScrollMetrics(element, {
scrollHeight: 1000,
clientHeight: 400,
initialScrollTop: 600,
});

const dateNowSpy = spyOn(Date, "now");
try {
let now = 1_000_000;
dateNowSpy.mockImplementation(() => now);

act(() => {
(result.current.contentRef as MutableRefObject<HTMLDivElement | null>).current = element;
});

// First tick: 3 px up. Releases the lock.
metrics.setScrollTop(metrics.maxScrollTop - 3);
act(() => {
result.current.markUserScrollIntent();
now += 1;
result.current.handleScroll(createScrollEvent(element));
});
expect(result.current.autoScroll).toBe(false);

// Second tick: 3 px further up (still within 8 px of bottom, but moving
// away). Lock must remain released.
metrics.setScrollTop(metrics.maxScrollTop - 6);
act(() => {
result.current.markUserScrollIntent();
now += 1;
result.current.handleScroll(createScrollEvent(element));
});
expect(result.current.autoScroll).toBe(false);

act(() => {
flushFrames(5);
});
expect(metrics.scrollTop).toBe(594);
} finally {
dateNowSpy.mockRestore();
}
});

test("scrolling back toward bottom within user intent re-engages the lock", () => {
// The release path must not regress the "scroll back to bottom and the
// lock re-engages immediately" behavior. After the user scrolls up, a
// direction reversal toward the bottom that lands within 8 px must relock
// without waiting for the intent window to expire.
const { result } = renderHook(() => useAutoScroll());
const element = document.createElement("div");
const metrics = attachScrollMetrics(element, {
scrollHeight: 1000,
clientHeight: 400,
initialScrollTop: 600,
});

const dateNowSpy = spyOn(Date, "now");
try {
let now = 1_000_000;
dateNowSpy.mockImplementation(() => now);

act(() => {
(result.current.contentRef as MutableRefObject<HTMLDivElement | null>).current = element;
});

// User scrolls up far enough to release.
metrics.setScrollTop(400);
act(() => {
result.current.markUserScrollIntent();
now += 1;
result.current.handleScroll(createScrollEvent(element));
});
expect(result.current.autoScroll).toBe(false);

// User reverses direction and scrolls back within 8 px of the bottom
// while the intent window is still open.
metrics.setScrollTop(metrics.maxScrollTop - 4);
act(() => {
result.current.markUserScrollIntent();
now += 1;
result.current.handleScroll(createScrollEvent(element));
});
expect(result.current.autoScroll).toBe(true);
} finally {
dateNowSpy.mockRestore();
}
});

test("disableAutoScroll seeds direction baseline so a later wheel-up does not relock", () => {
// Regression: without seeding lastScrollTopRef inside disableAutoScroll,
// the released-branch user-intent direction check compared the next
// user-driven scroll event against a stale value (often 0 from cold-start
// or the previous workspace's residue). A small wheel-up notch (~5 px)
// landing within 8 px of the new bottom would then look like "moving
// toward bottom" (e.g. 895 > 0) and spuriously relock the lock that the
// explicit programmatic disable just turned off.
const { result } = renderHook(() => useAutoScroll());
const element = document.createElement("div");
const metrics = attachScrollMetrics(element, {
scrollHeight: 1000,
clientHeight: 400,
initialScrollTop: 600,
});

const dateNowSpy = spyOn(Date, "now");
try {
let now = 1_000_000;
dateNowSpy.mockImplementation(() => now);

act(() => {
(result.current.contentRef as MutableRefObject<HTMLDivElement | null>).current = element;
});
// Pin lastScrollTopRef to 0 by skipping any prior handleScroll calls,
// mimicking the pre-mount or workspace-switch cold-start path. Then
// disableAutoScroll is invoked from a hypothetical "edit last user
// message" or "navigate to message" flow.
act(() => {
result.current.disableAutoScroll();
});
expect(result.current.autoScroll).toBe(false);

// User then bumps the trackpad: wheel handler primes intent (which
// also clears programmaticDisableRef), and a small wheel-up notch
// brings scrollTop to 595 β€” still within 8 px of the new bottom but
// strictly moving away from it.
metrics.setScrollTop(metrics.maxScrollTop - 5);
act(() => {
result.current.markUserScrollIntent();
now += 1;
result.current.handleScroll(createScrollEvent(element));
});
// The released branch must read previousScrollTop = 600 (the seeded
// value), see currentScrollTop = 595, and conclude the user is moving
// away from bottom β€” no relock.
expect(result.current.autoScroll).toBe(false);
expect(metrics.scrollTop).toBe(595);
} finally {
dateNowSpy.mockRestore();
}
});

test("delta-0 wheel events do not open a user-intent window", () => {
// Modifier-key wheel (Cmd-wheel zoom on macOS), Shift-wheel for horizontal
// scroll, and Bluetooth-mouse jitter all dispatch wheel events with
// deltaY === 0 (and often deltaX === 0). These must not clear
// programmaticDisableRef or refresh the 750 ms intent window β€” otherwise
// any layout drift that follows takes the wrong branch in handleScroll.
const { result } = renderHook(() => useAutoScroll());
const element = document.createElement("div");
const metrics = attachScrollMetrics(element, {
scrollHeight: 1000,
clientHeight: 400,
});

act(() => {
(result.current.contentRef as MutableRefObject<HTMLDivElement | null>).current = element;
// Phantom wheel: zero deltas. Must be a no-op for the intent window.
result.current.handleScrollContainerWheel(createWheelEvent(element, { deltaY: 0 }));
});

// Now produce a layout drift. With no intent window open, handleScroll
// takes the no-intent locked branch and snaps back to bottom β€” proving
// the wheel did not open an intent window (otherwise we'd see the
// user-intent locked branch release the lock instead).
metrics.setScrollTop(metrics.maxScrollTop - 5);
act(() => {
result.current.handleScroll(createScrollEvent(element));
});

expect(metrics.scrollTop).toBe(metrics.maxScrollTop);
expect(result.current.autoScroll).toBe(true);
});

test("non-zero wheel events do open a user-intent window", () => {
// Sanity-check that the wheel filter doesn't over-block. A real wheel
// event with deltaY != 0 must still mark intent and let the next scroll
// event release the lock.
const { result } = renderHook(() => useAutoScroll());
const element = document.createElement("div");
const metrics = attachScrollMetrics(element, {
scrollHeight: 1000,
clientHeight: 400,
});

const dateNowSpy = spyOn(Date, "now");
try {
let now = 1_000_000;
dateNowSpy.mockImplementation(() => now);

act(() => {
(result.current.contentRef as MutableRefObject<HTMLDivElement | null>).current = element;
result.current.handleScrollContainerWheel(createWheelEvent(element, { deltaY: -100 }));
});

metrics.setScrollTop(metrics.maxScrollTop - 5);
act(() => {
now += 1;
result.current.handleScroll(createScrollEvent(element));
});

expect(result.current.autoScroll).toBe(false);
expect(metrics.scrollTop).toBe(metrics.maxScrollTop - 5);
} finally {
dateNowSpy.mockRestore();
}
});

test("returning to bottom geometry re-acquires the lock and rAF resumes pinning", () => {
const { result } = renderHook(() => useAutoScroll());
const element = document.createElement("div");
Expand Down
Loading
Loading