fix(web): maintain sidebar scroll position when navigating between chats#1411
Conversation
Generated with [Linear](https://linear.app/sourcebot/issue/SOU-1483/maintain-sidebar-scroll-position-when-opening-a-e6d7#agent-session-05e85428) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis change adds sidebar scroll position persistence across remounts by restoring the saved scroll offset on mount and updating it during scrolling. A changelog entry records the behavior change. ChangesSidebar Scroll Persistence
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
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/app/(app)/@sidebar/components/sidebarBase.tsx (1)
51-91: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winScroll position bleeds across sidebar variants.
lastScrollTopis a single module-level value shared by every mount ofSidebarBase.DefaultSidebarandSettingsSidebar(seepackages/web/src/app/(app)/@sidebar/components/defaultSidebar/index.tsxand.../settingsSidebar/index.tsx) both renderSidebarBasewith very different content. Scrolling the chat sidebar, then navigating to settings (or back), restores an unrelated offset from the other variant instead of resetting/restoring correctly for the current one.Consider keying the stored value by a stable identifier for the sidebar variant (e.g. pathname prefix or a prop) so restoration only applies within the same sidebar type.
💡 Possible approach
-let lastScrollTop = 0; +const lastScrollTopByKey: Record<string, number> = {};Then read/write
lastScrollTopByKey[variantKey]wherevariantKeyis derived from the sidebar's identity (e.g. a prop passed byDefaultSidebar/SettingsSidebar).🤖 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 `@packages/web/src/app/`(app)/@sidebar/components/sidebarBase.tsx around lines 51 - 91, The scroll position is stored in a single module-level `lastScrollTop`, so `SidebarBase` shares one offset across both `DefaultSidebar` and `SettingsSidebar`. Update `SidebarBase` to scope the saved scroll position by a stable sidebar identity passed in from its callers (for example a variant key prop from `defaultSidebar/index.tsx` and `settingsSidebar/index.tsx`), and read/write that keyed value in the `useLayoutEffect` and scroll listener instead of the global value. Ensure restoration only happens for the matching sidebar variant and resets independently for others.
🤖 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.
Outside diff comments:
In `@packages/web/src/app/`(app)/@sidebar/components/sidebarBase.tsx:
- Around line 51-91: The scroll position is stored in a single module-level
`lastScrollTop`, so `SidebarBase` shares one offset across both `DefaultSidebar`
and `SettingsSidebar`. Update `SidebarBase` to scope the saved scroll position
by a stable sidebar identity passed in from its callers (for example a variant
key prop from `defaultSidebar/index.tsx` and `settingsSidebar/index.tsx`), and
read/write that keyed value in the `useLayoutEffect` and scroll listener instead
of the global value. Ensure restoration only happens for the matching sidebar
variant and resets independently for others.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 469f9424-dcb3-41fc-9b79-f4cdc3be7213
📒 Files selected for processing (2)
CHANGELOG.mdpackages/web/src/app/(app)/@sidebar/components/sidebarBase.tsx
…ition-when-opening-a-e6d7
…ition-when-opening-a-e6d7
Fixes SOU-1483
The sidebar lives in the
@sidebarparallel route slot, whose catch-all segment remounts when navigating between chats (/chat/A→/chat/B). That remount reset theSidebarContentscroll container back to the top each time.This persists the scroll position in a module-level variable and restores it (via
useLayoutEffect, to avoid flicker) wheneverSidebarBaseremounts, so the sidebar stays where you left it. The value resets on a full page reload.Summary by CodeRabbit