feat(chat): add scroll navigation for chat messages (decoupled, compiles clean)#855
feat(chat): add scroll navigation for chat messages (decoupled, compiles clean)#855bourgois wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (26)
✅ Files skipped from review due to trivial changes (9)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughRefactors chat pagination to explicit "load more" controls, adds a floating ScrollNavigation rail with jump/dot controls, updates useChatSessionState and ChatMessagesPane to support the new flow, and adds preferences + i18n strings for the navigation UI. ChangesChat Scroll Navigation Feature
Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
Hey @bourgois, thanks for the PR! Can you explain what issue this PR actually solves (with detailed reproduction step of the issue). In addition, it appears that the PR contains code for many independent issues. Can you send them in small chunks as separate PRs? This would be very useful for reviewing it. |
bourgois
force-pushed
the
feat/chat-scroll-navigation
branch
from
da5b44e to
1a19993
Compare
|
Thanks @blackmammoth, both points are fair. What it does. This is a UX feature, not a bug fix. In long sessions the only way to move through the conversation is free-scrolling — there's no way to jump between your own messages or hop to the top/bottom. This adds a slim navigation rail to the chat pane: a marker per user message (the active one tracks the viewport as you scroll), plus jump-to-top/bottom, previous/next-message, and a "load all" control for paginated history. How to try it:
On splitting — done. I've force-pushed this branch down to scroll-navigation only. It now touches just The two unrelated changes that previously rode along are now their own small PRs:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/chat/hooks/useChatSessionState.ts (1)
756-768: Remove dead assignment totopLoadLockRef.currentinloadMoreMessagesIn
src/components/chat/hooks/useChatSessionState.ts,topLoadLockRefis initialized viauseRef(false)and is only ever assignedfalse(including at lines 194, 370, and 757). No other reads/uses oftopLoadLockRefwere found, sotopLoadLockRef.current = false;insideloadMoreMessagesappears to be leftover dead code and can be removed (or properly wired if it’s meant to be a lock).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/chat/hooks/useChatSessionState.ts` around lines 756 - 768, The assignment topLoadLockRef.current = false; inside the loadMoreMessages callback is dead code—remove that line from the loadMoreMessages function (located alongside scrollContainerRef, loadOlderMessages and setIsLoadingMoreMessages) and, if topLoadLockRef (created via useRef) is not used anywhere else, remove the topLoadLockRef declaration entirely to avoid unused refs; otherwise, if it was intended as a lock, wire reads/writes consistently where loadOlderMessages and other related logic use it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/chat/hooks/useChatSessionState.ts`:
- Around line 756-768: The assignment topLoadLockRef.current = false; inside the
loadMoreMessages callback is dead code—remove that line from the
loadMoreMessages function (located alongside scrollContainerRef,
loadOlderMessages and setIsLoadingMoreMessages) and, if topLoadLockRef (created
via useRef) is not used anywhere else, remove the topLoadLockRef declaration
entirely to avoid unused refs; otherwise, if it was intended as a lock, wire
reads/writes consistently where loadOlderMessages and other related logic use
it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c25d9988-b647-420d-8c89-7623c0bfebe5
📒 Files selected for processing (12)
src/components/chat/hooks/useChatSessionState.tssrc/components/chat/view/ChatInterface.tsxsrc/components/chat/view/subcomponents/ChatMessagesPane.tsxsrc/components/chat/view/subcomponents/ScrollNavigation.tsxsrc/i18n/locales/de/chat.jsonsrc/i18n/locales/en/chat.jsonsrc/i18n/locales/it/chat.jsonsrc/i18n/locales/ja/chat.jsonsrc/i18n/locales/ko/chat.jsonsrc/i18n/locales/ru/chat.jsonsrc/i18n/locales/tr/chat.jsonsrc/i18n/locales/zh-CN/chat.json
✅ Files skipped from review due to trivial changes (5)
- src/i18n/locales/tr/chat.json
- src/i18n/locales/it/chat.json
- src/i18n/locales/ko/chat.json
- src/i18n/locales/en/chat.json
- src/i18n/locales/zh-CN/chat.json
🚧 Files skipped from review as they are similar to previous changes (5)
- src/i18n/locales/ru/chat.json
- src/i18n/locales/de/chat.json
- src/i18n/locales/ja/chat.json
- src/components/chat/view/subcomponents/ScrollNavigation.tsx
- src/components/chat/view/subcomponents/ChatMessagesPane.tsx
|
Hey @bourgois, thanks for the explanation. This looks very useful. However, clicking the scroll wheel is a bit difficult with the new setup since it's covering a portion of it. Can you address this? 
|
|
Hi @blackmammoth |
|
I'm using Chrome on Windows. The UI/UX is a bit clunky and difficult to understand. So that should be addressed. In addition, it would be better if this was an optional in settings that is off by default.
|
Adds a slim navigation rail to the chat message pane: a marker per user message (the active marker tracks the viewport as you scroll), plus jump-to-top/bottom, previous/next-message, and a load-all control for paginated history. Helps move through long conversations without free-scrolling. Scoped to the scroll-navigation feature only. Two unrelated fixes that previously rode along on siteboon#811 are split into separate PRs (the task-notification regex fix and the pending-message render race fix). Co-authored-by: WenhuaXia <wenhua_xia@163.com>
bourgois
force-pushed
the
feat/chat-scroll-navigation
branch
from
1a19993 to
4ed42d4
Compare
Addresses review feedback on the scroll-navigation feature: - Add a "Show scroll navigation" toggle to Quick Settings (View Options), defaulting to OFF. Wired through useUiPreferences (persisted), the quick-settings panel, MainContent, and ChatInterface, which now only renders the rail when the preference is enabled. i18n added for all eight locales. - Offset the rail from the right edge (right-0 -> right-3) so it no longer covers the message pane's native scrollbar. Flush at the edge it sat on top of a classic Windows/Chrome scrollbar, making the bar hard to grab (not visible with macOS overlay scrollbars). Co-authored-by: WenhuaXia <wenhua_xia@163.com>
|
Thanks for the feedback @blackmammoth — both points addressed in the latest push. 1. Opt-in, off by default. Added a "Show scroll navigation" toggle under Quick Settings → View Options, defaulting to off. It's wired through 2. Scrollbar overlap. The rail was positioned flush at
|
2 participants
Revives #811 (by @WenhuaXia) with the compilation blocker removed. Their commits are preserved; this branch adds one cleanup commit on top.
Why #811 was closed, and what changed
#811 was closed for a compile failure in
useChatRealtimeHandlers. The cause was a dependency on the unmerged completion-sound PR (#809):utils/notification-sounddoesn't exist onmain, so the import fails to resolve. ThoseuseChatRealtimeHandlersedits (the sound calls plus an unrelated session-resumption tweak) are out of scope for scroll navigation, so this branch reverts that file tomain. The feature now stands alone.What this adds
ScrollNavigation.tsx— a floating rail of user-message markers with jump-to-top/bottom, previous/next, and load-all controls; the active marker tracks the viewport via live DOM offsets.ChatInterface,ChatMessagesPane,useChatSessionState,useChatMessages, and i18n strings for all 8 locales.Verification
npm run typecheckpasses (this was the original blocker).eslintreports 0 errors on the touched files.useChatRealtimeHandlers.tshas no diff vsmain— no dependency on feat: add completion sound notification / 添加完成提示音 #809.Credit
Feature commits are @WenhuaXia's from #811. The only addition is the decoupling/cleanup commit. Happy to squash or adjust attribution however you prefer.
Summary by CodeRabbit
New Features
Pagination
Preferences
Internationalization