Skip to content

feat: add sidebar scroll position persistence#75

Open
ubay1 wants to merge 4 commits into
nodejs:mainfrom
ubay1:fix/persist-sidebar-scroll-position
Open

feat: add sidebar scroll position persistence#75
ubay1 wants to merge 4 commits into
nodejs:mainfrom
ubay1:fix/persist-sidebar-scroll-position

Conversation

@ubay1
Copy link
Copy Markdown

@ubay1 ubay1 commented May 21, 2026

  • Add useScroll hook with debounced scroll event handling
  • Add useScrollToElement hook to save/restore scroll position via localStorage and in-memory NavigationStateContext
  • Add NavigationStateProvider context for session-level state
  • Update Sidebar component to use useScrollToElement with a useLayoutEffect workaround (SideBar lacks forwardRef support)

#74 fix sidebar scroll position persistence

- Add `useScroll` hook with debounced scroll event handling
- Add `useScrollToElement` hook to save/restore scroll position
  via localStorage and in-memory NavigationStateContext
- Add `NavigationStateProvider` context for session-level state
- Update `Sidebar` component to use `useScrollToElement` with
  a `useLayoutEffect` workaround (SideBar lacks forwardRef support)
@ubay1 ubay1 requested a review from a team as a code owner May 21, 2026 01:13
@vercel
Copy link
Copy Markdown

vercel Bot commented May 21, 2026

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

Project Deployment Actions Updated (UTC)
nodejs-learn Ready Ready Preview May 21, 2026 2:06am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 21, 2026

PR Summary

Medium Risk
Moderate risk because it introduces new client-side context/provider wrapping the layout plus DOM querying and localStorage reads/writes that can impact hydration and runtime behavior in the sidebar.

Overview
Adds sidebar scroll-position persistence by introducing NavigationStateProvider (session in-memory state) and new hooks useScroll (debounced scroll listener) and useScrollToElement (save/restore via context + localStorage).

Updates Layout to wrap the app in the provider and updates Sidebar to restore/save its <aside> scroll position, including a useLayoutEffect workaround to obtain the real DOM element because the upstream SideBar component doesn’t forward refs.

Reviewed by Cursor Bugbot for commit 93020dd. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions
Copy link
Copy Markdown

👋 Codeowner Review Request

The following codeowners have been identified for the changed files:

Team reviewers: @nodejs/nodejs-website

Please review the changes when you have a chance. Thank you! 🙏

Comment thread hooks/useScrollToElement.js Outdated
Comment thread hooks/useScrollToElement.js Outdated
Comment thread providers/navigationStateProvider.jsx
Comment thread hooks/useScrollToElement.js Outdated
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 305c7d0. Configure here.

Comment thread hooks/useScrollToElement.js
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