Skip to content

fix: improve auto-scroll behavior during API activities and streaming #6989

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.

Sign up for GitHub

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

Closed
roomote wants to merge 1 commit into from
Closed

fix: improve auto-scroll behavior during API activities and streaming #6989

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
71 changes: 41 additions & 30 deletions webview-ui/src/components/chat/ChatView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,26 +465,6 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
}
}, [])

useEffect(() => {
const prev = prevExpandedRowsRef.current
let wasAnyRowExpandedByUser = false
if (prev) {
// Check if any row transitioned from false/undefined to true
for (const [tsKey, isExpanded] of Object.entries(expandedRows)) {
const ts = Number(tsKey)
if (isExpanded && !(prev[ts] ?? false)) {
wasAnyRowExpandedByUser = true
break
}
}
}

if (wasAnyRowExpandedByUser) {
disableAutoScrollRef.current = true
}
prevExpandedRowsRef.current = expandedRows // Store current state for next comparison
}, [expandedRows])

const isStreaming = useMemo(() => {
// Checking clineAsk isn't enough since messages effect may be called
// again for a tool for example, set clineAsk to its value, and if the
Expand Down Expand Up @@ -529,6 +509,27 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
return false
}, [modifiedMessages, clineAsk, enableButtons, primaryButtonText])

useEffect(() => {
const prev = prevExpandedRowsRef.current
let wasAnyRowExpandedByUser = false
if (prev) {
// Check if any row transitioned from false/undefined to true
for (const [tsKey, isExpanded] of Object.entries(expandedRows)) {
const ts = Number(tsKey)
if (isExpanded && !(prev[ts] ?? false)) {
wasAnyRowExpandedByUser = true
break
}
}
}

// Don't disable auto-scroll during streaming even if rows are expanded
if (wasAnyRowExpandedByUser && !isStreaming) {
Copy link
Contributor Author

@roomote roomote bot Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future maintainability, consider extracting the auto-scroll logic into a custom hook like useAutoScroll. This would centralize the complex conditions and make the behavior easier to test and understand.

disableAutoScrollRef.current = true
}
prevExpandedRowsRef.current = expandedRows // Store current state for next comparison
}, [expandedRows, isStreaming])

const markFollowUpAsAnswered = useCallback(() => {
const lastFollowUpMessage = messagesRef.current.findLast((msg: ClineMessage) => msg.ask === "followup")
if (lastFollowUpMessage) {
Expand Down Expand Up @@ -1390,26 +1391,32 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro

useEffect(() => {
let timer: ReturnType<typeof setTimeout> | undefined
if (!disableAutoScrollRef.current) {
// Always scroll to bottom during streaming unless user explicitly scrolled up
if (!disableAutoScrollRef.current || isStreaming) {
Copy link
Contributor Author

@roomote roomote bot Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes to auto-scroll logic during streaming are significant. Have we considered adding test coverage for this new behavior? It would help ensure the fix works correctly across different scenarios.

timer = setTimeout(() => scrollToBottomSmooth(), 50)
}
return () => {
if (timer) {
clearTimeout(timer)
}
}
}, [groupedMessages.length, scrollToBottomSmooth])
}, [groupedMessages.length, scrollToBottomSmooth, isStreaming])

const handleWheel = useCallback((event: Event) => {
const wheelEvent = event as WheelEvent
const handleWheel = useCallback(
(event: Event) => {
const wheelEvent = event as WheelEvent

if (wheelEvent.deltaY && wheelEvent.deltaY < 0) {
if (scrollContainerRef.current?.contains(wheelEvent.target as Node)) {
// User scrolled up
disableAutoScrollRef.current = true
if (wheelEvent.deltaY && wheelEvent.deltaY < 0) {
if (scrollContainerRef.current?.contains(wheelEvent.target as Node)) {
// User scrolled up - but don't disable during streaming if we're near bottom
if (!isStreaming || !isAtBottom) {
Copy link
Contributor Author

@roomote roomote bot Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentional? The isAtBottom state might not be immediately updated when checking in the wheel handler, potentially causing a race condition. Consider using a ref or checking the scroll position directly to avoid stale state issues.

disableAutoScrollRef.current = true
}
}
}
}
}, [])
},
[isStreaming, isAtBottom],
)

useEvent("wheel", handleWheel, window, { passive: true }) // passive improves scrolling performance

Expand Down Expand Up @@ -1878,6 +1885,10 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
if (isAtBottom) {
disableAutoScrollRef.current = false
}
// During streaming, always keep auto-scroll enabled unless user explicitly scrolled
if (isStreaming && isAtBottom) {
Copy link
Contributor Author

@roomote roomote bot Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This second condition checking isStreaming && isAtBottom appears redundant since if isAtBottom is true, we just set disableAutoScrollRef.current = false on line 1886. Could we simplify this logic?

Copy link
Contributor Author

@roomote roomote bot Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a comment here explaining why auto-scroll behaves differently during streaming. Something like:

disableAutoScrollRef.current = false
}
setShowScrollToBottom(disableAutoScrollRef.current && !isAtBottom)
}}
atBottomThreshold={10}
Expand Down
Loading