Skip to content

Commit 159ffeb

Browse files
committed
fix(flow-chat): Respect thinking scrollbar drags
- Pause tail following on upward scroll, including when scroll events arrive after animation frames - Ignore layout changes and scroll offset rounding noise - Reduce the follow recovery threshold from 80px to 20px while keeping the 700ms pause window - Add regression tests for scroll interruption and recovery boundaries
1 parent 21bc8c5 commit 159ffeb

2 files changed

Lines changed: 162 additions & 9 deletions

File tree

src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.test.tsx

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,3 +144,121 @@ describe('ModelThinkingDisplay reasoning summary', () => {
144144
?.getAttribute('data-expanded')).toBe('true');
145145
});
146146
});
147+
148+
describe('ModelThinkingDisplay scroll ownership', () => {
149+
let container: HTMLDivElement;
150+
let root: Root;
151+
let frames: Map<number, FrameRequestCallback>;
152+
let frameId: number;
153+
let height: number;
154+
let viewport: number;
155+
let clockMs: number;
156+
157+
beforeEach(() => {
158+
(globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean })
159+
.IS_REACT_ACT_ENVIRONMENT = true;
160+
frames = new Map();
161+
frameId = 0;
162+
height = 1000;
163+
viewport = 300;
164+
clockMs = 1000;
165+
vi.spyOn(performance, 'now').mockImplementation(() => clockMs);
166+
vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => {
167+
frames.set(++frameId, callback);
168+
return frameId;
169+
});
170+
vi.stubGlobal('cancelAnimationFrame', (id: number) => frames.delete(id));
171+
vi.stubGlobal('ResizeObserver', class {
172+
observe() {}
173+
disconnect() {}
174+
});
175+
container = document.createElement('div');
176+
document.body.appendChild(container);
177+
root = createRoot(container);
178+
});
179+
180+
afterEach(() => {
181+
act(() => root.unmount());
182+
container.remove();
183+
vi.restoreAllMocks();
184+
vi.unstubAllGlobals();
185+
});
186+
187+
function render(content = 'Thinking') {
188+
act(() => root.render(<ModelThinkingDisplay thinkingItem={{
189+
...summaryItem(content), reasoningKind: 'reasoning',
190+
}} />));
191+
}
192+
193+
function nextFrame() {
194+
const pending = [...frames.values()];
195+
frames.clear();
196+
act(() => pending.forEach((callback) => callback(clockMs)));
197+
}
198+
199+
function startFollow(initialTop = 640) {
200+
render();
201+
const el = container.querySelector('[data-testid="chat-thinking-content"]') as HTMLDivElement;
202+
Object.defineProperties(el, {
203+
scrollHeight: { get: () => height },
204+
clientHeight: { get: () => viewport },
205+
});
206+
el.scrollTop = initialTop;
207+
nextFrame();
208+
expect(el.scrollTop).toBeGreaterThan(initialTop);
209+
expect(frames.size).toBe(1);
210+
return el;
211+
}
212+
213+
it.each([true, false])('pauses a scrollbar drag with scroll event delivered: %s', (deliverScroll) => {
214+
const el = startFollow();
215+
el.scrollTop = 400;
216+
if (deliverScroll) act(() => el.dispatchEvent(new Event('scroll')));
217+
nextFrame();
218+
expect(el.scrollTop).toBe(400);
219+
expect(frames.size).toBe(0);
220+
clockMs += 1000;
221+
height += 20;
222+
render('More thinking');
223+
nextFrame();
224+
expect(el.scrollTop).toBe(400);
225+
});
226+
227+
it('continues following after its own scroll events', () => {
228+
const el = startFollow();
229+
const before = el.scrollTop;
230+
act(() => el.dispatchEvent(new Event('scroll')));
231+
nextFrame();
232+
expect(el.scrollTop).toBeGreaterThan(before);
233+
});
234+
235+
it.each(['shrink', 'resize', 'rounding'])('does not pause for %s', (change) => {
236+
const el = startFollow();
237+
if (change === 'shrink') height -= 20;
238+
if (change === 'resize') viewport += 20;
239+
el.scrollTop -= change === 'rounding' ? 0.5 : 20;
240+
const before = el.scrollTop;
241+
act(() => el.dispatchEvent(new Event('scroll')));
242+
nextFrame();
243+
expect(el.scrollTop).toBeGreaterThan(before);
244+
});
245+
246+
it.each([19, 20, 75])('only resumes within 20 px after the 700 ms pause (gap: %s)', (gap) => {
247+
const el = startFollow(690);
248+
const pausedTop = 700 - gap;
249+
el.scrollTop = pausedTop;
250+
act(() => el.dispatchEvent(new Event('scroll')));
251+
clockMs += 600;
252+
render('Still paused');
253+
nextFrame();
254+
expect(el.scrollTop).toBe(pausedTop);
255+
clockMs += 101;
256+
render('Resume near bottom');
257+
nextFrame();
258+
if (gap < 20) {
259+
expect(el.scrollTop).toBeGreaterThan(pausedTop);
260+
} else {
261+
expect(el.scrollTop).toBe(pausedTop);
262+
}
263+
});
264+
});

src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.tsx

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ export const ModelThinkingDisplay: React.FC<ModelThinkingDisplayProps> = ({
5252
/** Frame the follow has booked, and the sign that it is still travelling. */
5353
const tailFollowFrameRef = useRef<number | null>(null);
5454
const touchScrollStartYRef = useRef<number | null>(null);
55+
const lastScrollPositionRef = useRef<{
56+
top: number;
57+
height: number;
58+
viewport: number;
59+
} | null>(null);
5560

5661
const isActive = isStreaming || status === 'streaming';
5762
const { displayText: displayContent, isRevealing } = useTypewriter(
@@ -98,6 +103,38 @@ export const ModelThinkingDisplay: React.FC<ModelThinkingDisplayProps> = ({
98103
tailFollowFrameRef.current = null;
99104
}, []);
100105

106+
const pauseTailFollowForUserScroll = useCallback(() => {
107+
shouldFollowTailRef.current = false;
108+
tailFollowPauseVersionRef.current += 1;
109+
tailFollowUserPauseUntilMsRef.current = performance.now() + 700;
110+
stopTailFollow();
111+
}, [stopTailFollow]);
112+
113+
const recordScrollPosition = useCallback((el: HTMLElement) => {
114+
lastScrollPositionRef.current = {
115+
top: el.scrollTop,
116+
height: el.scrollHeight,
117+
viewport: el.clientHeight,
118+
};
119+
}, []);
120+
121+
const detectUpwardScroll = useCallback((el: HTMLElement) => {
122+
const previous = lastScrollPositionRef.current;
123+
// Scrollbar drags have no wheel/key event. Compare with our last actual
124+
// offset, allowing rounding noise and excluding layout-driven movement.
125+
const movedUp = isExpanded && previous !== null &&
126+
el.scrollHeight >= previous.height &&
127+
el.clientHeight === previous.viewport &&
128+
el.scrollTop < previous.top - 1;
129+
recordScrollPosition(el);
130+
if (movedUp) pauseTailFollowForUserScroll();
131+
return movedUp;
132+
}, [isExpanded, pauseTailFollowForUserScroll, recordScrollPosition]);
133+
134+
useLayoutEffect(() => {
135+
lastScrollPositionRef.current = null;
136+
}, [isExpanded]);
137+
101138
/**
102139
* Follow the tail across the frames it is given, rather than in one write.
103140
*
@@ -119,6 +156,8 @@ export const ModelThinkingDisplay: React.FC<ModelThinkingDisplayProps> = ({
119156
tailFollowFrameRef.current = null;
120157
const el = contentRef.current;
121158
if (!el) return;
159+
// The browser may update the offset before delivering its scroll event.
160+
if (detectUpwardScroll(el)) return;
122161
if (expectedPauseVersion !== tailFollowPauseVersionRef.current) return;
123162
if (!shouldFollowTailRef.current) return;
124163

@@ -132,6 +171,7 @@ export const ModelThinkingDisplay: React.FC<ModelThinkingDisplayProps> = ({
132171
: { offsetPx: targetPx, outcome: 'snapped' as const };
133172

134173
el.scrollTop = step.offsetPx;
174+
recordScrollPosition(el);
135175
// Read back rather than taken from the step: the browser clamps to the
136176
// scrollable range, and a platform without fractional scroll offsets
137177
// rounds the last part of an ease away entirely. Believing the step there
@@ -155,23 +195,17 @@ export const ModelThinkingDisplay: React.FC<ModelThinkingDisplayProps> = ({
155195
};
156196

157197
tailFollowFrameRef.current = requestAnimationFrame(runFrame);
158-
}, []);
198+
}, [detectUpwardScroll, recordScrollPosition]);
159199

160200
/** A follow in flight outlives neither the card nor its collapse. */
161201
useEffect(() => stopTailFollow, [isExpanded, stopTailFollow]);
162202

163-
const pauseTailFollowForUserScroll = useCallback(() => {
164-
shouldFollowTailRef.current = false;
165-
tailFollowPauseVersionRef.current += 1;
166-
tailFollowUserPauseUntilMsRef.current = performance.now() + 700;
167-
}, []);
168-
169203
// Auto-scroll to bottom while content grows.
170204
useEffect(() => {
171205
if (isExpanded && contentRef.current) {
172206
const el = contentRef.current;
173207
const gap = getThinkingScrollGap(el);
174-
const wasNearBottom = gap < 80;
208+
const wasNearBottom = gap < 20;
175209
const userPauseActive = performance.now() <= tailFollowUserPauseUntilMsRef.current;
176210
if (wasNearBottom && !userPauseActive) {
177211
shouldFollowTailRef.current = true;
@@ -215,6 +249,7 @@ export const ModelThinkingDisplay: React.FC<ModelThinkingDisplayProps> = ({
215249
const checkScrollState = useCallback(() => {
216250
const el = contentRef.current;
217251
if (!el) return;
252+
detectUpwardScroll(el);
218253
const gap = getThinkingScrollGap(el);
219254
const nextScrollState = {
220255
hasScroll: el.scrollHeight > el.clientHeight,
@@ -249,7 +284,7 @@ export const ModelThinkingDisplay: React.FC<ModelThinkingDisplayProps> = ({
249284
atBottom: nextScrollState.atBottom,
250285
}
251286
));
252-
}, [getThinkingScrollGap]);
287+
}, [detectUpwardScroll, getThinkingScrollGap]);
253288

254289
useEffect(() => {
255290
if (isExpanded) {

0 commit comments

Comments
 (0)