Skip to content

Commit e061bde

Browse files
Sun-sunshine06Sun-sunshine06
authored
Merge pull request #295 from Sun-sunshine06/fix/issue-267-session-scroll-position
fix(chat): preserve per-session scroll position
2 parents 5d1ba4d + 75c4fb2 commit e061bde

5 files changed

Lines changed: 220 additions & 20 deletions

File tree

‎src/renderer/components/ChatView.tsx‎

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useState, useRef, useEffect, useMemo, useCallback } from 'react';
1+
import { useState, useRef, useEffect, useLayoutEffect, useMemo, useCallback } from 'react';
22
import { useTranslation } from 'react-i18next';
33
import {
44
useActiveSessionId,
@@ -17,6 +17,7 @@ import { SubagentTracker } from './SubagentTracker';
1717
import { ContextUsageBar } from './ContextUsageBar';
1818
import type { Message, ContentBlock } from '../types';
1919
import { Send, Square, Plus, Loader2, Plug, X, Clock } from 'lucide-react';
20+
import { isScrollNearBottom, resolveSessionScrollTop } from '../utils/chat-scroll-position';
2021

2122
type AttachedFile = {
2223
name: string;
@@ -63,6 +64,8 @@ export function ChatView() {
6364
const prevPartialLengthRef = useRef(0);
6465
const scrollTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
6566
const scrollRequestRef = useRef<number | null>(null);
67+
const scrollStateRequestRef = useRef<number | null>(null);
68+
const pendingScrollStateRef = useRef<{ sessionId: string; scrollTop: number } | null>(null);
6669
const isScrollingRef = useRef(false);
6770

6871
const hasActiveTurn = Boolean(activeTurn);
@@ -133,6 +136,30 @@ export function ChatView() {
133136
: Math.max(0, (executionClock.endAt ?? clockNow) - executionClock.startAt);
134137
const timerActive = Boolean(executionClock?.startAt && executionClock.endAt === null);
135138

139+
useLayoutEffect(() => {
140+
const container = scrollContainerRef.current;
141+
if (!container || !activeSessionId) return;
142+
143+
const savedScrollTop = useAppStore.getState().sessionScrollPositions[activeSessionId];
144+
const restoredScrollTop = resolveSessionScrollTop(
145+
savedScrollTop,
146+
container.scrollHeight,
147+
container.clientHeight
148+
);
149+
container.scrollTop = restoredScrollTop;
150+
isUserAtBottomRef.current = isScrollNearBottom(
151+
restoredScrollTop,
152+
container.scrollHeight,
153+
container.clientHeight
154+
);
155+
156+
// Prevent the generic new-message effect from overriding the session restore.
157+
const sessionState = useAppStore.getState().sessionStates[activeSessionId];
158+
prevMessageCountRef.current = sessionState?.messages.length ?? 0;
159+
prevPartialLengthRef.current =
160+
(sessionState?.partialMessage.length ?? 0) + (sessionState?.partialThinking.length ?? 0);
161+
}, [activeSessionId]);
162+
136163
// Debounced scroll function to prevent scroll conflicts
137164
const scrollToBottom = useRef((behavior: ScrollBehavior = 'auto', immediate: boolean = false) => {
138165
// Cancel any pending scroll requests
@@ -175,17 +202,42 @@ export function ChatView() {
175202
useEffect(() => {
176203
const container = scrollContainerRef.current;
177204
if (!container) return;
205+
206+
const flushScrollPosition = () => {
207+
scrollStateRequestRef.current = null;
208+
const pending = pendingScrollStateRef.current;
209+
pendingScrollStateRef.current = null;
210+
if (pending) {
211+
useAppStore.getState().setSessionScrollPosition(pending.sessionId, pending.scrollTop);
212+
}
213+
};
214+
178215
const updateScrollState = () => {
179216
const distanceToBottom =
180217
container.scrollHeight - container.scrollTop - container.clientHeight;
181218
isUserAtBottomRef.current = distanceToBottom <= 80;
219+
if (activeSessionId) {
220+
pendingScrollStateRef.current = {
221+
sessionId: activeSessionId,
222+
scrollTop: container.scrollTop,
223+
};
224+
if (scrollStateRequestRef.current === null) {
225+
scrollStateRequestRef.current = requestAnimationFrame(flushScrollPosition);
226+
}
227+
}
182228
};
183229
updateScrollState();
184-
// 用户阅读旧消息时，阻止新消息自动滚动打断视线
230+
// Keep new messages from interrupting the user while they read older content.
185231
const onScroll = () => updateScrollState();
186232
container.addEventListener('scroll', onScroll, { passive: true });
187-
return () => container.removeEventListener('scroll', onScroll);
188-
}, []);
233+
return () => {
234+
container.removeEventListener('scroll', onScroll);
235+
if (scrollStateRequestRef.current !== null) {
236+
cancelAnimationFrame(scrollStateRequestRef.current);
237+
}
238+
flushScrollPosition();
239+
};
240+
}, [activeSessionId]);
189241

190242
useEffect(() => {
191243
const messageCount = messages.length;

‎src/renderer/store/index.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,9 @@ interface AppState {
9393
// Per-session state (messages, partials, turns, traces, etc.)
9494
sessionStates: Record<string, SessionState>;
9595

96+
// Ephemeral viewport state, kept separate so scrolling does not rerender message consumers.
97+
sessionScrollPositions: Record<string, number>;
98+
9699
// UI state
97100
isLoading: boolean;
98101
sidebarCollapsed: boolean;
@@ -138,6 +141,7 @@ interface AppState {
138141
removeSession: (sessionId: string) => void;
139142
removeSessions: (sessionIds: string[]) => void;
140143
setActiveSession: (sessionId: string | null) => void;
144+
setSessionScrollPosition: (sessionId: string, scrollTop: number) => void;
141145

142146
addMessage: (sessionId: string, message: Message) => void;
143147
updateMessage: (sessionId: string, messageId: string, updates: Partial<Message>) => void;
@@ -238,6 +242,7 @@ export const useAppStore = create<AppState>((set) => ({
238242
sessions: [],
239243
activeSessionId: null,
240244
sessionStates: {},
245+
sessionScrollPositions: {},
241246
isLoading: false,
242247
sidebarCollapsed: false,
243248
contextPanelCollapsed: false,
@@ -279,9 +284,13 @@ export const useAppStore = create<AppState>((set) => ({
279284
removeSession: (sessionId) =>
280285
set((state) => {
281286
const { [sessionId]: _, ...restSessionStates } = state.sessionStates;
287+
const restScrollPositions = Object.fromEntries(
288+
Object.entries(state.sessionScrollPositions).filter(([id]) => id !== sessionId)
289+
);
282290
return {
283291
sessions: state.sessions.filter((s) => s.id !== sessionId),
284292
sessionStates: restSessionStates,
293+
sessionScrollPositions: restScrollPositions,
285294
activeSessionId: state.activeSessionId === sessionId ? null : state.activeSessionId,
286295
};
287296
}),
@@ -290,20 +299,33 @@ export const useAppStore = create<AppState>((set) => ({
290299
set((state) => {
291300
const idSet = new Set(sessionIds);
292301
const newSessionStates: Record<string, SessionState> = {};
302+
const newScrollPositions: Record<string, number> = {};
293303
for (const key of Object.keys(state.sessionStates)) {
294304
if (!idSet.has(key)) newSessionStates[key] = state.sessionStates[key];
295305
}
306+
for (const key of Object.keys(state.sessionScrollPositions)) {
307+
if (!idSet.has(key)) newScrollPositions[key] = state.sessionScrollPositions[key];
308+
}
296309

297310
return {
298311
sessions: state.sessions.filter((s) => !idSet.has(s.id)),
299312
sessionStates: newSessionStates,
313+
sessionScrollPositions: newScrollPositions,
300314
activeSessionId:
301315
state.activeSessionId && idSet.has(state.activeSessionId) ? null : state.activeSessionId,
302316
};
303317
}),
304318

305319
setActiveSession: (sessionId) => set({ activeSessionId: sessionId }),
306320

321+
setSessionScrollPosition: (sessionId, scrollTop) =>
322+
set((state) => ({
323+
sessionScrollPositions: {
324+
...state.sessionScrollPositions,
325+
[sessionId]: scrollTop,
326+
},
327+
})),
328+
307329
// Message actions
308330
addMessage: (sessionId, message) =>
309331
set((state) => {

‎src/renderer/utils/chat-scroll-position.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
export function resolveSessionScrollTop(
2+
savedScrollTop: number | undefined,
3+
scrollHeight: number,
4+
clientHeight: number
5+
): number {
6+
const maximumScrollTop = Math.max(0, scrollHeight - clientHeight);
7+
if (savedScrollTop === undefined) return maximumScrollTop;
8+
return Math.min(Math.max(0, savedScrollTop), maximumScrollTop);
9+
}
10+
11+
export function isScrollNearBottom(
12+
scrollTop: number,
13+
scrollHeight: number,
14+
clientHeight: number,
15+
threshold = 80
16+
): boolean {
17+
return scrollHeight - scrollTop - clientHeight <= threshold;
18+
}

‎src/tests/store/session-state.test.ts‎

Lines changed: 100 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ describe('SessionState unified store', () => {
3636
expect(state.sessionStates['s1'].executionClock).toEqual({ startAt: null, endAt: null });
3737
expect(state.sessionStates['s1'].traceSteps).toEqual([]);
3838
expect(state.sessionStates['s1'].contextWindow).toBe(0);
39+
expect(state.sessionScrollPositions['s1']).toBeUndefined();
3940
});
4041
});
4142

@@ -48,6 +49,7 @@ describe('SessionState unified store', () => {
4849
useAppStore.getState().removeSession('s1');
4950
expect(useAppStore.getState().sessionStates['s1']).toBeUndefined();
5051
expect(useAppStore.getState().sessions).toHaveLength(0);
52+
expect(useAppStore.getState().sessionScrollPositions['s1']).toBeUndefined();
5153
});
5254

5355
it('should clear activeSessionId when removing active session', () => {
@@ -59,6 +61,24 @@ describe('SessionState unified store', () => {
5961
});
6062
});
6163

64+
describe('scroll positions', () => {
65+
it('stores viewport positions independently for each session', () => {
66+
useAppStore.getState().setSessionScrollPosition('s1', 120);
67+
useAppStore.getState().setSessionScrollPosition('s2', 480);
68+
69+
expect(useAppStore.getState().sessionScrollPositions).toEqual({ s1: 120, s2: 480 });
70+
});
71+
72+
it('removes viewport positions with their sessions', () => {
73+
useAppStore.getState().addSession(makeSession('s1'));
74+
useAppStore.getState().setSessionScrollPosition('s1', 120);
75+
76+
useAppStore.getState().removeSession('s1');
77+
78+
expect(useAppStore.getState().sessionScrollPositions['s1']).toBeUndefined();
79+
});
80+
});
81+
6282
describe('removeSessions (batch)', () => {
6383
it('should remove multiple sessions at once', () => {
6484
useAppStore.getState().addSession(makeSession('s1'));
@@ -116,8 +136,20 @@ describe('SessionState unified store', () => {
116136
it('should set messages (bulk replace)', () => {
117137
useAppStore.getState().addSession(makeSession('s1'));
118138
const msgs = [
119-
{ id: 'a', sessionId: 's1', role: 'user' as const, content: [{ type: 'text' as const, text: 'hi' }], timestamp: 1 },
120-
{ id: 'b', sessionId: 's1', role: 'assistant' as const, content: [{ type: 'text' as const, text: 'hello' }], timestamp: 2 },
139+
{
140+
id: 'a',
141+
sessionId: 's1',
142+
role: 'user' as const,
143+
content: [{ type: 'text' as const, text: 'hi' }],
144+
timestamp: 1,
145+
},
146+
{
147+
id: 'b',
148+
sessionId: 's1',
149+
role: 'assistant' as const,
150+
content: [{ type: 'text' as const, text: 'hello' }],
151+
timestamp: 2,
152+
},
121153
];
122154
useAppStore.getState().setMessages('s1', msgs);
123155
expect(useAppStore.getState().sessionStates['s1'].messages).toHaveLength(2);
@@ -219,8 +251,11 @@ describe('SessionState unified store', () => {
219251
useAppStore.getState().addSession(makeSession('s1'));
220252
// Setup an active turn first
221253
useAppStore.getState().addMessage('s1', {
222-
id: 'msg1', sessionId: 's1', role: 'user',
223-
content: [{ type: 'text', text: 'test' }], timestamp: Date.now(),
254+
id: 'msg1',
255+
sessionId: 's1',
256+
role: 'user',
257+
content: [{ type: 'text', text: 'test' }],
258+
timestamp: Date.now(),
224259
});
225260
useAppStore.getState().activateNextTurn('s1', 'step1');
226261
useAppStore.getState().updateActiveTurnStep('s1', 'step2');
@@ -233,8 +268,11 @@ describe('SessionState unified store', () => {
233268
it('should clear active turn', () => {
234269
useAppStore.getState().addSession(makeSession('s1'));
235270
useAppStore.getState().addMessage('s1', {
236-
id: 'msg1', sessionId: 's1', role: 'user',
237-
content: [{ type: 'text', text: 'test' }], timestamp: Date.now(),
271+
id: 'msg1',
272+
sessionId: 's1',
273+
role: 'user',
274+
content: [{ type: 'text', text: 'test' }],
275+
timestamp: Date.now(),
238276
});
239277
useAppStore.getState().activateNextTurn('s1', 'step1');
240278
useAppStore.getState().clearActiveTurn('s1');
@@ -244,8 +282,11 @@ describe('SessionState unified store', () => {
244282
it('should only clear active turn when stepId matches', () => {
245283
useAppStore.getState().addSession(makeSession('s1'));
246284
useAppStore.getState().addMessage('s1', {
247-
id: 'msg1', sessionId: 's1', role: 'user',
248-
content: [{ type: 'text', text: 'test' }], timestamp: Date.now(),
285+
id: 'msg1',
286+
sessionId: 's1',
287+
role: 'user',
288+
content: [{ type: 'text', text: 'test' }],
289+
timestamp: Date.now(),
249290
});
250291
useAppStore.getState().activateNextTurn('s1', 'step1');
251292
// Try clearing with wrong stepId - should not clear
@@ -259,8 +300,11 @@ describe('SessionState unified store', () => {
259300
it('should clear pending turns', () => {
260301
useAppStore.getState().addSession(makeSession('s1'));
261302
useAppStore.getState().addMessage('s1', {
262-
id: 'msg1', sessionId: 's1', role: 'user',
263-
content: [{ type: 'text', text: 'test' }], timestamp: Date.now(),
303+
id: 'msg1',
304+
sessionId: 's1',
305+
role: 'user',
306+
content: [{ type: 'text', text: 'test' }],
307+
timestamp: Date.now(),
264308
});
265309
expect(useAppStore.getState().sessionStates['s1'].pendingTurns).toHaveLength(1);
266310
useAppStore.getState().clearPendingTurns('s1');
@@ -273,8 +317,21 @@ describe('SessionState unified store', () => {
273317
useAppStore.getState().addSession(makeSession('s1'));
274318
// Manually set messages with queued status
275319
useAppStore.getState().setMessages('s1', [
276-
{ id: 'msg1', sessionId: 's1', role: 'user', content: [{ type: 'text', text: 'a' }], timestamp: 1, localStatus: 'queued' },
277-
{ id: 'msg2', sessionId: 's1', role: 'user', content: [{ type: 'text', text: 'b' }], timestamp: 2 },
320+
{
321+
id: 'msg1',
322+
sessionId: 's1',
323+
role: 'user',
324+
content: [{ type: 'text', text: 'a' }],
325+
timestamp: 1,
326+
localStatus: 'queued',
327+
},
328+
{
329+
id: 'msg2',
330+
sessionId: 's1',
331+
role: 'user',
332+
content: [{ type: 'text', text: 'b' }],
333+
timestamp: 2,
334+
},
278335
]);
279336
useAppStore.getState().clearQueuedMessages('s1');
280337
const msgs = useAppStore.getState().sessionStates['s1'].messages;
@@ -285,7 +342,14 @@ describe('SessionState unified store', () => {
285342
it('should cancel queued messages', () => {
286343
useAppStore.getState().addSession(makeSession('s1'));
287344
useAppStore.getState().setMessages('s1', [
288-
{ id: 'msg1', sessionId: 's1', role: 'user', content: [{ type: 'text', text: 'a' }], timestamp: 1, localStatus: 'queued' },
345+
{
346+
id: 'msg1',
347+
sessionId: 's1',
348+
role: 'user',
349+
content: [{ type: 'text', text: 'a' }],
350+
timestamp: 1,
351+
localStatus: 'queued',
352+
},
289353
]);
290354
useAppStore.getState().cancelQueuedMessages('s1');
291355
expect(useAppStore.getState().sessionStates['s1'].messages[0].localStatus).toBe('cancelled');
@@ -295,7 +359,14 @@ describe('SessionState unified store', () => {
295359
describe('trace steps', () => {
296360
it('should add and update trace steps', () => {
297361
useAppStore.getState().addSession(makeSession('s1'));
298-
const step = { id: 'ts1', type: 'tool_call' as const, status: 'running' as const, title: 'read', toolName: 'read', timestamp: Date.now() };
362+
const step = {
363+
id: 'ts1',
364+
type: 'tool_call' as const,
365+
status: 'running' as const,
366+
title: 'read',
367+
toolName: 'read',
368+
timestamp: Date.now(),
369+
};
299370
useAppStore.getState().addTraceStep('s1', step);
300371
expect(useAppStore.getState().sessionStates['s1'].traceSteps).toHaveLength(1);
301372

@@ -306,8 +377,21 @@ describe('SessionState unified store', () => {
306377
it('should set trace steps (bulk replace)', () => {
307378
useAppStore.getState().addSession(makeSession('s1'));
308379
const steps = [
309-
{ id: 'ts1', type: 'tool_call' as const, status: 'completed' as const, title: 'read', toolName: 'read', timestamp: 1 },
310-
{ id: 'ts2', type: 'thinking' as const, status: 'completed' as const, title: 'thinking', timestamp: 2 },
380+
{
381+
id: 'ts1',
382+
type: 'tool_call' as const,
383+
status: 'completed' as const,
384+
title: 'read',
385+
toolName: 'read',
386+
timestamp: 1,
387+
},
388+
{
389+
id: 'ts2',
390+
type: 'thinking' as const,
391+
status: 'completed' as const,
392+
title: 'thinking',
393+
timestamp: 2,
394+
},
311395
];
312396
useAppStore.getState().setTraceSteps('s1', steps);
313397
expect(useAppStore.getState().sessionStates['s1'].traceSteps).toHaveLength(2);

0 commit comments

Comments
 (0)