feat(web): replace manual auto-scroll with useStickToBottom in chat thread#1031
Conversation
…hread Replaces the custom scroll tracking implementation (manual scroll listeners, scrollIntoView calls, isAutoScrollEnabled state) with the useStickToBottom library already used in detailsCard. Fixes layout regressions for sticky answer headers and chat box visibility. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughReplaced Radix ScrollArea and manual scroll refs in the chat thread with the useStickToBottom hook; updated scroll state and invocations, restructured message list markup, adjusted bounce button logic, and added a changelog entry documenting improved auto-scroll behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web/src/features/chat/components/chatThread/chatThread.tsx (1)
279-296:⚠️ Potential issue | 🟠 MajorSkip the initial restore when no scroll offset was captured.
This effect always falls back to
scrollOffset ?? 0. On fresh entries that auto-submit on mount (inputMessage/ pending-message restore), that timeout runs afterscrollToBottom()and snaps already-scrollable threads back to the top becausehistory.state.scrollOffsethas not been recorded yet. Only run the restore when a numeric offset exists.💡 Suggested guard for the initial restore
useEffect(() => { const scrollElement = scrollRef.current; if (!scrollElement) { return; } + + const { scrollOffset } = (history.state ?? {}) as ChatHistoryState; + if (typeof scrollOffset !== 'number') { + return; + } // `@hack`: without this setTimeout, the scroll position would not be restored // at the correct position (it was slightly too high). The theory is that the // content hasn't fully rendered yet, so restoring the scroll position too // early results in weirdness. Waiting 10ms seems to fix the issue. - setTimeout(() => { - const { scrollOffset } = (history.state ?? {}) as ChatHistoryState; + const timeout = window.setTimeout(() => { scrollElement.scrollTo({ - top: scrollOffset ?? 0, + top: scrollOffset, behavior: 'instant', }); }, 10); - }, [scrollRef]); + + return () => window.clearTimeout(timeout); + }, [scrollRef]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/features/chat/components/chatThread/chatThread.tsx` around lines 279 - 296, The effect that restores scroll position in the useEffect (using scrollRef and ChatHistoryState) should skip restoring when no numeric scrollOffset exists to avoid snapping fresh threads to top; change the logic inside the setTimeout to read const { scrollOffset } = (history.state ?? {}) as ChatHistoryState and only call scrollElement.scrollTo(...) when typeof scrollOffset === 'number' (otherwise do nothing), so initial mounts with undefined offsets won't override the current scroll (e.g., after scrollToBottom()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/features/chat/components/chatThread/chatThread.tsx`:
- Around line 363-368: The empty-state wrapper using contentRef lost its
full-height behavior after removing ScrollArea.Viewport, so update the wrapper
div rendered when messagePairs.length === 0 (the div with className "flex
items-center justify-center text-center h-full" inside the ChatThread component)
to preserve height by adding min-h-full to its classes (e.g., "min-h-full") so
it stays vertically centered; adjust the className string on that div
accordingly.
---
Outside diff comments:
In `@packages/web/src/features/chat/components/chatThread/chatThread.tsx`:
- Around line 279-296: The effect that restores scroll position in the useEffect
(using scrollRef and ChatHistoryState) should skip restoring when no numeric
scrollOffset exists to avoid snapping fresh threads to top; change the logic
inside the setTimeout to read const { scrollOffset } = (history.state ?? {}) as
ChatHistoryState and only call scrollElement.scrollTo(...) when typeof
scrollOffset === 'number' (otherwise do nothing), so initial mounts with
undefined offsets won't override the current scroll (e.g., after
scrollToBottom()).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: be1fbf22-6a5f-4288-8e45-ccd0bf3b9a74
📒 Files selected for processing (2)
CHANGELOG.mdpackages/web/src/features/chat/components/chatThread/chatThread.tsx
Summary
scrollAreaRef,latestMessagePairRef,isAutoScrollEnabledstate, manual scroll event listeners,scrollIntoViewcalls) withuseStickToBottom, which is already used indetailsCardFixes SOU-455
🤖 Generated with Claude Code
Summary by CodeRabbit