Skip to content

Show floating scroll-to-bottom button in chat pane - #186

Closed
ctate wants to merge 3 commits into
mainfrom
feature/scroll-to-bottom-button-chat-iQHRUc
Closed

ctate wants to merge 3 commits into
mainfrom
feature/scroll-to-bottom-button-chat-iQHRUc

Conversation

@ctate

@ctate ctate commented Oct 25, 2025

Copy link
Copy Markdown
Collaborator

No description provided.

@vercel

vercel Bot commented Oct 25, 2025

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
coding-agent-platform Ready Ready Preview Comment Oct 25, 2025 5:04pm

Comment thread components/task-chat.tsx
Comment on lines +369 to +375
setTimeout(() => {
const container = scrollContainerRef.current
if (container) {
const nearBottom = isNearBottom()
setShowScrollButton(!nearBottom)
}
}, 100)

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.

Suggested change
setTimeout(() => {
const container = scrollContainerRef.current
if (container) {
const nearBottom = isNearBottom()
setShowScrollButton(!nearBottom)
}
}, 100)
const timeout = setTimeout(() => {
const container = scrollContainerRef.current
if (container) {
const nearBottom = isNearBottom()
setShowScrollButton(!nearBottom)
}
}, 100)
return () => clearTimeout(timeout)

The second useEffect schedules a timeout to update scroll button visibility but doesn't provide a cleanup function to cancel it, which can cause state update warnings on component unmount and potential memory issues if messages change frequently.

View Details

Analysis

Missing cleanup function in useEffect with setTimeout schedules duplicate scroll button updates

What fails: The useEffect hook starting at line 366 in components/task-chat.tsx schedules a setTimeout without returning a cleanup function to cancel it. When dependencies (messages or activeTab) change, previous timeouts remain pending and can accumulate, causing redundant setState calls and violating React's official guidance on synchronizing with effects.

How to reproduce:

  1. Open a task in the chat interface
  2. Switch to another tab and back to 'chat' (triggers activeTab dependency change)
  3. Receive new messages frequently (triggers messages dependency change)
  4. Without cleanup, multiple setTimeout callbacks accumulate and each fires its own setShowScrollButton call within a short timeframe

Result: Multiple timeouts execute independently, each calling setShowScrollButton redundantly. According to React documentation, the cleanup function should mirror the Effect's setup—when a timeout is scheduled, it must be cancelled by the cleanup function.

Expected: Each useEffect invocation should return a cleanup function that calls clearTimeout() on the scheduled timeout, matching the pattern shown in React's recommended approach and the existing pattern in the same codebase at repo-selector.tsx line 203.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant