-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix/restore thread scroll position #5552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5aeeae8
c31d010
6705a75
a7cd406
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1152,6 +1152,53 @@ type LocalThreadErrorEntry = { | |
| readonly at: number; | ||
| }; | ||
|
|
||
| // Module state survives route changes but resets when the client reloads. | ||
| type TimelineScrollPosition = | ||
| | { readonly kind: "automatic"; readonly offset: number } | ||
| | { | ||
| readonly kind: "manual"; | ||
| readonly offset: number; | ||
| readonly turnId: TurnId | null; | ||
| }; | ||
|
|
||
| type TimelineEntryScrollMode = | ||
| | { readonly kind: "follow-end" } | ||
| | { readonly kind: "restore-position"; readonly offset: number } | ||
| | { readonly kind: "anchor-response"; readonly turnId: TurnId }; | ||
|
|
||
| const timelineScrollPositionByThreadKey = new Map<string, TimelineScrollPosition>(); | ||
| const TIMELINE_SCROLL_NAVIGATION_KEYS = new Set([ | ||
| "ArrowDown", | ||
| "ArrowUp", | ||
| "End", | ||
| "Home", | ||
| "PageDown", | ||
| "PageUp", | ||
| " ", | ||
| ]); | ||
|
|
||
| function resolveTimelineEntryScrollMode(input: { | ||
| readonly latestTurnId: TurnId | null; | ||
| readonly runningTurnId: TurnId | null; | ||
| readonly savedPosition: TimelineScrollPosition | undefined; | ||
| }): TimelineEntryScrollMode { | ||
| if (input.runningTurnId !== null) { | ||
| if ( | ||
| input.savedPosition?.kind === "manual" && | ||
| input.savedPosition.turnId === input.runningTurnId | ||
| ) { | ||
| return { kind: "restore-position", offset: input.savedPosition.offset }; | ||
| } | ||
| return { kind: "anchor-response", turnId: input.runningTurnId }; | ||
| } | ||
| if (input.savedPosition) { | ||
| return { kind: "restore-position", offset: input.savedPosition.offset }; | ||
| } | ||
| return input.latestTurnId | ||
| ? { kind: "anchor-response", turnId: input.latestTurnId } | ||
| : { kind: "follow-end" }; | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| function chatActionErrorMessage(error: unknown): string { | ||
| return error instanceof Error ? error.message : "An error occurred."; | ||
| } | ||
|
|
@@ -1174,6 +1221,10 @@ function ChatViewContent(props: ChatViewProps) { | |
| [environmentId, threadId], | ||
| ); | ||
| const routeThreadKey = useMemo(() => scopedThreadKey(routeThreadRef), [routeThreadRef]); | ||
| const restoredTimelineScrollPosition = useMemo( | ||
| () => timelineScrollPositionByThreadKey.get(routeThreadKey), | ||
| [routeThreadKey], | ||
| ); | ||
| const updateProject = useAtomCommand(projectEnvironment.update, { reportFailure: false }); | ||
| const upsertKeybinding = useAtomCommand(serverEnvironment.upsertKeybinding, { | ||
| reportFailure: false, | ||
|
|
@@ -1511,6 +1562,47 @@ function ChatViewContent(props: ChatViewProps) { | |
| [activeThread], | ||
| ); | ||
| const activeThreadKey = activeThreadRef ? scopedThreadKey(activeThreadRef) : null; | ||
| const activeRunningTurnId = | ||
| activeThread?.session?.status === "running" ? activeThread.session.activeTurnId : null; | ||
| const runningTurnOnEntryRef = useRef<{ | ||
| readonly captured: boolean; | ||
| readonly threadKey: string | null; | ||
| readonly turnId: TurnId | null; | ||
| }>({ | ||
| captured: !threadDetailLoading, | ||
| threadKey: activeThreadKey, | ||
| turnId: threadDetailLoading ? null : activeRunningTurnId, | ||
| }); | ||
| if ( | ||
| runningTurnOnEntryRef.current.threadKey !== activeThreadKey || | ||
| (!runningTurnOnEntryRef.current.captured && !threadDetailLoading) | ||
| ) { | ||
| runningTurnOnEntryRef.current = { | ||
| captured: !threadDetailLoading, | ||
| threadKey: activeThreadKey, | ||
| turnId: threadDetailLoading ? null : activeRunningTurnId, | ||
| }; | ||
| } | ||
| const runningTurnOnEntry = runningTurnOnEntryRef.current.turnId; | ||
| const timelineEntryScrollMode = useMemo( | ||
| () => | ||
| resolveTimelineEntryScrollMode({ | ||
| latestTurnId: activeThread?.latestTurn?.turnId ?? null, | ||
| runningTurnId: runningTurnOnEntry, | ||
| savedPosition: restoredTimelineScrollPosition, | ||
| }), | ||
| [activeThread?.latestTurn?.turnId, restoredTimelineScrollPosition, runningTurnOnEntry], | ||
| ); | ||
| const [manuallyNavigatedTimelineEntry, setManuallyNavigatedTimelineEntry] = useState<{ | ||
| readonly threadKey: string; | ||
| readonly turnId: TurnId | null; | ||
| } | null>(null); | ||
| const entryResponseAnchorActive = | ||
| timelineEntryScrollMode.kind === "anchor-response" && | ||
| !( | ||
| manuallyNavigatedTimelineEntry?.threadKey === routeThreadKey && | ||
| manuallyNavigatedTimelineEntry.turnId === timelineEntryScrollMode.turnId | ||
| ); | ||
| const [timelineAnchor, setTimelineAnchor] = useState<{ | ||
| readonly threadKey: string | null; | ||
| readonly messageId: MessageId | null; | ||
|
|
@@ -2399,6 +2491,18 @@ function ChatViewContent(props: ChatViewProps) { | |
| deriveTimelineEntries(timelineMessages, activeThread?.proposedPlans ?? [], workLogEntries), | ||
| [activeThread?.proposedPlans, timelineMessages, workLogEntries], | ||
| ); | ||
| const entryResponseAnchorMessageId = useMemo(() => { | ||
| if (!entryResponseAnchorActive) { | ||
| return null; | ||
| } | ||
| for (let index = timelineEntries.length - 1; index >= 0; index -= 1) { | ||
| const entry = timelineEntries[index]; | ||
| if (entry?.kind === "message" && entry.message.role === "user") { | ||
| return entry.message.id; | ||
| } | ||
| } | ||
| return null; | ||
| }, [entryResponseAnchorActive, timelineEntries]); | ||
| const [dockedDraftHeroThreadKey, setDockedDraftHeroThreadKey] = useState<string | null>(null); | ||
| const draftHeroDockRequested = | ||
| activeThreadKey !== null && dockedDraftHeroThreadKey === activeThreadKey; | ||
|
|
@@ -3529,6 +3633,7 @@ function ChatViewContent(props: ChatViewProps) { | |
| new Debouncer(() => setShowScrollToBottom(true), { wait: 150 }), | ||
| ); | ||
| const timelineScrollModeRef = useRef<TimelineScrollMode>("following-end"); | ||
| const preserveInitialResponsePositionRef = useRef(false); | ||
| const pendingTimelineAnchorRef = useRef<MessageId | null>(null); | ||
| const positionedTimelineAnchorRef = useRef<MessageId | null>(null); | ||
| const settledTimelineAnchorRef = useRef<MessageId | null>(null); | ||
|
|
@@ -3540,10 +3645,16 @@ function ChatViewContent(props: ChatViewProps) { | |
| readonly offset: number; | ||
| readonly userScrollGeneration: number; | ||
| } | null>(null); | ||
| const pendingTimelineManualScrollRef = useRef<{ | ||
| readonly initialOffset: number; | ||
| readonly threadKey: string; | ||
| readonly turnId: TurnId | null; | ||
| } | null>(null); | ||
| const anchorScrollRestoreFrameRef = useRef<number | null>(null); | ||
| const cancelTimelineLiveFollowForUserNavigation = useCallback(() => { | ||
| anchorUserScrollGenerationRef.current += 1; | ||
| timelineScrollModeRef.current = "free-scrolling"; | ||
| preserveInitialResponsePositionRef.current = false; | ||
| liveFollowUserScrollGenerationRef.current = null; | ||
| pendingTimelineAnchorRef.current = null; | ||
| positionedTimelineAnchorRef.current = null; | ||
|
|
@@ -3555,13 +3666,26 @@ function ChatViewContent(props: ChatViewProps) { | |
| anchorScrollRestoreFrameRef.current = null; | ||
| } | ||
| }, []); | ||
| const cancelTimelineLiveFollowForUserNavigationRef = useRef( | ||
| const beginTimelineManualNavigation = useCallback(() => { | ||
| const currentOffset = legendListRef.current?.getState().scroll; | ||
| if (typeof currentOffset === "number" && Number.isFinite(currentOffset)) { | ||
| pendingTimelineManualScrollRef.current = { | ||
| initialOffset: currentOffset, | ||
| threadKey: routeThreadKey, | ||
| turnId: | ||
| activeRunningTurnId ?? | ||
| (timelineEntryScrollMode.kind === "anchor-response" | ||
| ? timelineEntryScrollMode.turnId | ||
| : null), | ||
| }; | ||
| } | ||
| cancelTimelineLiveFollowForUserNavigation(); | ||
| }, [ | ||
| activeRunningTurnId, | ||
| cancelTimelineLiveFollowForUserNavigation, | ||
| ); | ||
| useEffect(() => { | ||
| cancelTimelineLiveFollowForUserNavigationRef.current = | ||
| cancelTimelineLiveFollowForUserNavigation; | ||
| }, [cancelTimelineLiveFollowForUserNavigation]); | ||
| routeThreadKey, | ||
| timelineEntryScrollMode, | ||
| ]); | ||
| const getActiveTimelineTurnMetrics = useCallback( | ||
| (list?: LegendListRef | null) => { | ||
| const resolvedList = list ?? legendListRef.current; | ||
|
|
@@ -3615,6 +3739,7 @@ function ChatViewContent(props: ChatViewProps) { | |
| const scrollToEnd = useCallback((animated = false) => { | ||
| isAtEndRef.current = true; | ||
| timelineScrollModeRef.current = "following-end"; | ||
| preserveInitialResponsePositionRef.current = false; | ||
| liveFollowUserScrollGenerationRef.current = anchorUserScrollGenerationRef.current; | ||
| pendingTimelineAnchorRef.current = null; | ||
| activeTimelineAnchorIndexRef.current = null; | ||
|
|
@@ -3629,30 +3754,46 @@ function ChatViewContent(props: ChatViewProps) { | |
| if (!scrollNode) { | ||
| return; | ||
| } | ||
| const handleManualNavigation = () => { | ||
| cancelTimelineLiveFollowForUserNavigationRef.current(); | ||
| const handlePointerDown = (event: PointerEvent) => { | ||
| if (event.target === scrollNode) { | ||
| beginTimelineManualNavigation(); | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| }; | ||
| const clearPendingManualScroll = () => { | ||
| pendingTimelineManualScrollRef.current = null; | ||
| }; | ||
| scrollNode.addEventListener("wheel", handleManualNavigation, { | ||
| const handleKeyDown = (event: KeyboardEvent) => { | ||
| if (TIMELINE_SCROLL_NAVIGATION_KEYS.has(event.key)) { | ||
| beginTimelineManualNavigation(); | ||
| } | ||
| }; | ||
| scrollNode.addEventListener("wheel", beginTimelineManualNavigation, { | ||
| passive: true, | ||
| }); | ||
| scrollNode.addEventListener("touchmove", handleManualNavigation, { | ||
| scrollNode.addEventListener("touchmove", beginTimelineManualNavigation, { | ||
| passive: true, | ||
| }); | ||
| scrollNode.addEventListener("pointerdown", handleManualNavigation, { | ||
| scrollNode.addEventListener("pointerdown", handlePointerDown, { | ||
| passive: true, | ||
| }); | ||
| scrollNode.addEventListener("pointerup", clearPendingManualScroll, { passive: true }); | ||
| scrollNode.addEventListener("pointercancel", clearPendingManualScroll, { passive: true }); | ||
| scrollNode.addEventListener("keydown", handleKeyDown); | ||
| removeListeners = () => { | ||
| scrollNode.removeEventListener("wheel", handleManualNavigation); | ||
| scrollNode.removeEventListener("touchmove", handleManualNavigation); | ||
| scrollNode.removeEventListener("pointerdown", handleManualNavigation); | ||
| scrollNode.removeEventListener("wheel", beginTimelineManualNavigation); | ||
| scrollNode.removeEventListener("touchmove", beginTimelineManualNavigation); | ||
| scrollNode.removeEventListener("pointerdown", handlePointerDown); | ||
| scrollNode.removeEventListener("pointerup", clearPendingManualScroll); | ||
| scrollNode.removeEventListener("pointercancel", clearPendingManualScroll); | ||
| scrollNode.removeEventListener("keydown", handleKeyDown); | ||
| }; | ||
| }); | ||
|
|
||
| return () => { | ||
| cancelAnimationFrame(frame); | ||
| removeListeners?.(); | ||
| }; | ||
| }, [activeThread?.id]); | ||
| }, [activeThread?.id, beginTimelineManualNavigation]); | ||
|
|
||
| const onTimelineAnchorReady = useCallback((messageId: MessageId, anchorIndex: number) => { | ||
| if (pendingTimelineAnchorRef.current === messageId) { | ||
|
|
@@ -3758,6 +3899,13 @@ function ChatViewContent(props: ChatViewProps) { | |
| if (isAtEndRef.current === isAtEnd) return; | ||
| isAtEndRef.current = isAtEnd; | ||
| if (isAtEnd) { | ||
| if (preserveInitialResponsePositionRef.current) { | ||
| timelineScrollModeRef.current = "free-scrolling"; | ||
| liveFollowUserScrollGenerationRef.current = null; | ||
| showScrollDebouncer.current.cancel(); | ||
| setShowScrollToBottom(false); | ||
| return; | ||
| } | ||
| timelineScrollModeRef.current = "following-end"; | ||
| liveFollowUserScrollGenerationRef.current = anchorUserScrollGenerationRef.current; | ||
| showScrollDebouncer.current.cancel(); | ||
|
|
@@ -3769,6 +3917,43 @@ function ChatViewContent(props: ChatViewProps) { | |
| } | ||
| }, []); | ||
|
|
||
| const onTimelineScrollOffsetChange = useCallback( | ||
| (offset: number) => { | ||
| const existing = timelineScrollPositionByThreadKey.get(routeThreadKey); | ||
| const pendingManualScroll = pendingTimelineManualScrollRef.current; | ||
| const manualScrollLanded = | ||
| pendingManualScroll?.threadKey === routeThreadKey && | ||
| Math.abs(offset - pendingManualScroll.initialOffset) > 0.5; | ||
|
|
||
| if (manualScrollLanded) { | ||
| const manualPosition: TimelineScrollPosition = { | ||
| kind: "manual", | ||
| offset, | ||
| turnId: pendingManualScroll.turnId, | ||
| }; | ||
| timelineScrollPositionByThreadKey.set(routeThreadKey, manualPosition); | ||
| pendingTimelineManualScrollRef.current = null; | ||
| setTimelineAnchor((current) => | ||
| current.threadKey === routeThreadKey && current.messageId !== null | ||
| ? { threadKey: routeThreadKey, messageId: null } | ||
| : current, | ||
| ); | ||
| setManuallyNavigatedTimelineEntry((current) => | ||
| current?.threadKey === routeThreadKey && current.turnId === manualPosition.turnId | ||
| ? current | ||
| : { threadKey: routeThreadKey, turnId: manualPosition.turnId }, | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| timelineScrollPositionByThreadKey.set( | ||
| routeThreadKey, | ||
| existing?.kind === "manual" ? { ...existing, offset } : { kind: "automatic", offset }, | ||
| ); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Early scroll overwrites saved offsetHigh Severity
Additional Locations (2)Reviewed by Cursor Bugbot for commit 6705a75. Configure here. |
||
| }, | ||
| [routeThreadKey], | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| if (!activeThread?.id) { | ||
| return; | ||
|
|
@@ -3838,9 +4023,13 @@ function ChatViewContent(props: ChatViewProps) { | |
| useEffect(() => { | ||
| setPullRequestDialogState(null); | ||
| isAtEndRef.current = true; | ||
| timelineScrollModeRef.current = "following-end"; | ||
| liveFollowUserScrollGenerationRef.current = anchorUserScrollGenerationRef.current; | ||
| preserveInitialResponsePositionRef.current = timelineEntryScrollMode.kind === "anchor-response"; | ||
| timelineScrollModeRef.current = | ||
| timelineEntryScrollMode.kind === "follow-end" ? "following-end" : "free-scrolling"; | ||
| liveFollowUserScrollGenerationRef.current = | ||
| timelineEntryScrollMode.kind === "follow-end" ? anchorUserScrollGenerationRef.current : null; | ||
| pendingTimelineAnchorRef.current = null; | ||
| pendingTimelineManualScrollRef.current = null; | ||
| positionedTimelineAnchorRef.current = null; | ||
| settledTimelineAnchorRef.current = null; | ||
| activeTimelineAnchorIndexRef.current = null; | ||
|
|
@@ -3854,7 +4043,7 @@ function ChatViewContent(props: ChatViewProps) { | |
| } | ||
| } | ||
| // activeThreadRef resets transitively with the active thread. | ||
| }, [activeThread?.id]); | ||
| }, [routeThreadKey, timelineEntryScrollMode.kind]); | ||
|
|
||
| // Auto-open the plan sidebar when plan/todo steps arrive for the current turn. | ||
| // Don't auto-open for plans carried over from a previous turn (the user can open manually). | ||
|
|
@@ -4897,6 +5086,7 @@ function ChatViewContent(props: ChatViewProps) { | |
| // streams into the reserved space below it. | ||
| isAtEndRef.current = true; | ||
| timelineScrollModeRef.current = "anchoring-new-turn"; | ||
| preserveInitialResponsePositionRef.current = false; | ||
| liveFollowUserScrollGenerationRef.current = anchorUserScrollGenerationRef.current; | ||
| pendingTimelineAnchorRef.current = messageIdForSend; | ||
| activeTimelineAnchorIndexRef.current = null; | ||
|
|
@@ -5341,6 +5531,7 @@ function ChatViewContent(props: ChatViewProps) { | |
| // Position this sent row once LegendList has measured the anchored tail. | ||
| isAtEndRef.current = true; | ||
| timelineScrollModeRef.current = "anchoring-new-turn"; | ||
| preserveInitialResponsePositionRef.current = false; | ||
| liveFollowUserScrollGenerationRef.current = anchorUserScrollGenerationRef.current; | ||
| pendingTimelineAnchorRef.current = messageIdForSend; | ||
| activeTimelineAnchorIndexRef.current = null; | ||
|
|
@@ -5979,7 +6170,7 @@ function ChatViewContent(props: ChatViewProps) { | |
| <MessagesTimeline | ||
| agentPanelModel={agentPanelModel} | ||
| onOpenAgents={addAgentsSurface} | ||
| key={activeThread.id} | ||
| key={routeThreadKey} | ||
| isWorking={isWorking} | ||
| activeTurnInProgress={isWorking || !latestTurnSettled} | ||
| activeTurnStartedAt={activeWorkStartedAt} | ||
|
|
@@ -6004,12 +6195,18 @@ function ChatViewContent(props: ChatViewProps) { | |
| timestampFormat={timestampFormat} | ||
| workspaceRoot={activeWorkspaceRoot} | ||
| skills={activeProviderStatus?.skills ?? EMPTY_PROVIDER_SKILLS} | ||
| anchorMessageId={timelineAnchorMessageId} | ||
| anchorMessageId={timelineAnchorMessageId ?? entryResponseAnchorMessageId} | ||
| onAnchorReady={onTimelineAnchorReady} | ||
| onAnchorSizeChanged={onTimelineAnchorSizeChanged} | ||
| contentInsetEndAdjustment={composerOverlayHeight} | ||
| initialScrollOffset={ | ||
| timelineEntryScrollMode.kind === "restore-position" | ||
| ? timelineEntryScrollMode.offset | ||
| : undefined | ||
| } | ||
| onIsAtEndChange={onIsAtEndChange} | ||
| onManualNavigation={cancelTimelineLiveFollowForUserNavigation} | ||
| onManualNavigation={beginTimelineManualNavigation} | ||
| onScrollOffsetChange={onTimelineScrollOffsetChange} | ||
| hideEmptyPlaceholder={isDraftHeroState || threadDetailLoading} | ||
| topFadeEnabled={!hasTimelineTopBanner} | ||
| /> | ||
|
|
@@ -6024,7 +6221,10 @@ function ChatViewContent(props: ChatViewProps) { | |
| type="button" | ||
| aria-label="Scroll to end" | ||
| title="Scroll to end" | ||
| onClick={() => scrollToEnd(true)} | ||
| onClick={() => { | ||
| beginTimelineManualNavigation(); | ||
| scrollToEnd(true); | ||
| }} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Scroll pill saves partial offsetMedium Severity The “Scroll to end” control calls Reviewed by Cursor Bugbot for commit 6705a75. Configure here. |
||
| className="chat-composer-glass pointer-events-auto flex items-center gap-1.5 rounded-full border border-border/60 px-3 py-1 text-muted-foreground text-xs shadow-sm transition-colors hover:border-border hover:text-foreground hover:cursor-pointer" | ||
| > | ||
| <ChevronDownIcon className="size-3.5" /> | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Automatic offset blocks response anchor
Medium Severity
The new
anchor-responsepath forlatestTurnIdis skipped whenever any saved timeline position exists, includingkind: "automatic"offsets written during initial list layout. After a brief first visit that only auto-settled at the bottom, returning to the same thread restores that offset instead of anchoring at the latest response top described in the PR.Additional Locations (1)
apps/web/src/components/ChatView.tsx#L3948-L3952Reviewed by Cursor Bugbot for commit a7cd406. Configure here.