Skip to content

Fix scroll behavior when switching between chats #2643

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

Merged
wwwillchen merged 2 commits into from
Feb 12, 2026
Merged

Fix scroll behavior when switching between chats #2643

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f22fad0
Fix scroll behavior when switching between chats
wwwillchen Feb 11, 2026
8814a60
Fix scroll to bottom on new message send
wwwillchen Feb 12, 2026
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
32 changes: 29 additions & 3 deletions src/components/ChatPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,38 @@ export function ChatPanel({

// Scroll to bottom when a new stream starts (user sent a message)
const streamCount = chatId ? (streamCountById.get(chatId) ?? 0) : 0;
const messages = chatId ? (messagesById.get(chatId) ?? []) : [];

// Track previous chatId to detect chat switches
const prevChatIdRef = useRef<number | undefined>(undefined);

useEffect(() => {
const isChatSwitch = prevChatIdRef.current !== chatId;
prevChatIdRef.current = chatId;

isAtBottomRef.current = true;
setShowScrollButton(false);
Comment on lines 91 to 92

@github-actions github-actions Bot Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 HIGH: Unconditionally resetting isAtBottomRef defeats user scroll position during streaming

Flagged by 2/3 independent reviewers (HIGH, MEDIUM)

Lines 91-92 unconditionally set isAtBottomRef.current = true and setShowScrollButton(false) on every effect run. Since messages.length is now in the dependency array, this means whenever a new message arrives during streaming, the user's scroll position state is forcibly reset to "at bottom" and the scroll button is hidden — even if the user has scrolled up to read earlier messages.

The previous code only did this on chatId or streamCount changes, which was more intentional. Now with messages.length as a trigger, this reset happens too frequently and can break the user's ability to stay scrolled up during a stream.

Suggested change
isAtBottomRef.current = true;
setShowScrollButton(false);
if (isChatSwitch) {
isAtBottomRef.current = true;
setShowScrollButton(false);
}

Move the unconditional reset inside the if (isChatSwitch) block so it only fires on actual chat switches, not on every messages.length change during streaming.

Generated by Dyadbot multi-agent code review

scrollToBottom();
}, [chatId, streamCount, scrollToBottom]);

if (isChatSwitch && messages.length > 0) {
// When switching chats with existing messages, wait for Virtuoso to render
// then scroll to ensure we're at the bottom
requestAnimationFrame(() => {
requestAnimationFrame(() => {
scrollToBottom("instant");
});
});

@cursor cursor Bot Feb 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale RAF causes cross-chat scroll jumps

Medium Severity

The double requestAnimationFrame scheduled on chat switch has no cleanup, so pending callbacks from a previous switch can run after another chatId change and still call scrollToBottom("instant"). This can apply a stale scroll action to the newly active chat and cause unexpected jump-to-bottom behavior.

Fix in Cursor Fix in Web

} else if (!isChatSwitch) {
// For stream count changes (new message sent), wait for Virtuoso to render
// the placeholder message before scrolling
requestAnimationFrame(() => {
requestAnimationFrame(() => {
scrollToBottom();
});
});
Comment on lines +102 to +109

@github-actions github-actions Bot Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 MEDIUM: messages.length dependency triggers unnecessary scrolls in non-chat-switch branch

Flagged by 3/3 independent reviewers (MEDIUM, LOW, LOW)

The else if (!isChatSwitch) branch runs whenever any dependency changes and it is not a chat switch. Because messages.length is in the dependency array, this branch fires every time the message count changes during streaming (new message chunks, tool-use results, etc.), not just on streamCount changes.

The comment says "For stream count changes (new message sent)" but the code also triggers on message-count changes. This creates additional scroll-to-bottom calls during streaming that may conflict with Virtuoso's followOutput behavior or the test-mode auto-scroll effect, potentially causing janky scrolling.

Consider guarding this branch with a check that streamCount actually changed (e.g., track previous streamCount in a ref), or separate the messages.length-dependent logic (for chat-switch deferred scroll) from the streamCount-dependent logic (for new message scroll) into distinct effects.

Generated by Dyadbot multi-agent code review

}
// Note: if isChatSwitch && messages.length === 0, we don't scroll yet.
// The messages will be fetched and this effect will re-run with messages.length > 0.
}, [chatId, streamCount, messages.length, scrollToBottom]);
Comment on lines +82 to +113

@gemini-code-assist gemini-code-assist Bot Feb 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This logic has a subtle bug. When switching to a chat where messages aren't cached, they are fetched asynchronously. When they arrive, this useEffect runs again, but isChatSwitch is false. This leads to a 'smooth' scroll instead of the expected 'instant' scroll.

To fix this and ensure an instant scroll in all chat-switch scenarios, we can use an additional useRef to track the chat switch state across renders until the messages are loaded. This provides a better user experience.

const messages = chatId ? (messagesById.get(chatId) ?? []) : [];

// Track previous chatId to detect chat switches
const prevChatIdRef = useRef<number | undefined>(undefined);
const justSwitchedChat = useRef(false);

useEffect(() => {
  const isChatSwitch = prevChatIdRef.current !== chatId;
  prevChatIdRef.current = chatId;
  if (isChatSwitch) {
    justSwitchedChat.current = true; // A chat switch occurred
  }

  isAtBottomRef.current = true;
  setShowScrollButton(false);

  // If we just switched chat and messages are available (either immediately or after fetch)
  if (justSwitchedChat.current && messages.length > 0) {
    // Wait for Virtuoso to render, then scroll instantly
    requestAnimationFrame(() => {
      requestAnimationFrame(() => {
        scrollToBottom("instant");
        justSwitchedChat.current = false; // Reset for this chat
      });
    });
  } else if (!isChatSwitch) {
    // For stream count changes (new message sent in the same chat), scroll smoothly
    scrollToBottom();
  }
}, [chatId, streamCount, messages.length, scrollToBottom]);

Comment on lines 87 to +113

@devin-ai-integration devin-ai-integration Bot Feb 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Chat switch scroll logic fails when messages are fetched asynchronously (uncached chats)

When switching to a chat whose messages are not yet in the messagesById atom (i.e., not cached), the double-RAF instant scroll intended for chat switches is never executed.

Root Cause

The effect at lines 87-108 tracks chat switches via prevChatIdRef. On a chat switch to an uncached chat:

  1. Effect runs: isChatSwitch = true, messages.length === 0 → no scroll is performed, but prevChatIdRef.current is already updated to the new chatId (line 89).
  2. fetchChatMessages (line 110-121) asynchronously loads messages into the atom.
  3. When messages arrive, messages.length changes from 0 to >0, triggering the effect again.
  4. On this re-run, prevChatIdRef.current === chatId so isChatSwitch = false.
  5. The code enters the else if (!isChatSwitch) branch (line 100) and calls scrollToBottom() with default smooth behavior.

This means:

  • The double requestAnimationFrame pattern (lines 95-101) that waits for Virtuoso to render is bypassed for uncached chats.
  • The scroll uses "smooth" animation instead of "instant" jump.
  • The scroll may execute before Virtuoso has finished rendering the newly-fetched messages, resulting in incomplete or incorrect scroll positioning.

The comment at lines 104-105 acknowledges this re-run scenario but incorrectly assumes it will be handled properly. In practice, the isChatSwitch flag is already consumed/lost by the first run.

Impact: When switching to a chat that hasn't been visited yet in the current session (messages not cached), the user may see a brief flash of content at the wrong scroll position, or a smooth scroll animation instead of an instant jump to the bottom.

Prompt for agents 
The core issue is that prevChatIdRef.current is updated on the first run of the effect (when messages.length === 0 for uncached chats), so when the effect re-runs after messages are fetched, isChatSwitch is false.

A fix would be to use a separate ref to track whether we're still waiting for messages after a chat switch, rather than relying solely on prevChatIdRef for the isChatSwitch check. For example:

1. Add a new ref: const pendingChatSwitchRef = useRef(false);
2. In the effect, when isChatSwitch && messages.length === 0, set pendingChatSwitchRef.current = true (instead of doing nothing).
3. When !isChatSwitch && pendingChatSwitchRef.current && messages.length > 0, treat it as a deferred chat switch: use the double-RAF instant scroll pattern, then set pendingChatSwitchRef.current = false.
4. The existing else if (!isChatSwitch && !pendingChatSwitchRef.current) branch handles the normal stream count change case with scrollToBottom().

This ensures that the chat-switch scroll behavior (double-RAF + instant) is applied even when messages are fetched asynchronously after the chat switch.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


const fetchChatMessages = useCallback(async () => {
if (!chatId) {
Expand All @@ -102,7 +129,6 @@ export function ChatPanel({
fetchChatMessages();
}, [fetchChatMessages]);

const messages = chatId ? (messagesById.get(chatId) ?? []) : [];
const isStreaming = chatId ? (isStreamingById.get(chatId) ?? false) : false;

// Scroll to bottom when streaming completes to ensure footer content is visible,
Expand Down
Loading