Skip to content

feat(ui): implement sidebar scroll position preservation - #8517

Merged
ovflowd merged 13 commits into
nodejs:mainfrom
malav2110:fix-selected-article-sidebar-scroll-fix
Jan 11, 2026
Merged

ovflowd merged 13 commits into
nodejs:mainfrom
malav2110:fix-selected-article-sidebar-scroll-fix

Conversation

@malav2110

@malav2110 malav2110 commented Jan 6, 2026

Copy link
Copy Markdown
Contributor

Description

This PR adds functionality to preserve sidebar scroll location on Learn Page

Validation

Before:
https://github.com/user-attachments/assets/41ca31eb-72c2-447a-a6a7-2d451171775e

After:
https://github.com/user-attachments/assets/656713ee-5d03-430f-beda-8ae491f415a8

Related Issues

Fixes #8336

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run pnpm format to ensure the code follows the style guide.
  • I have run pnpm test to check if all tests are passing.
  • I have run pnpm build to check if the website builds without errors.
  • I've covered new added functionality with unit tests if necessary.

Copilot AI review requested due to automatic review settings January 6, 2026 01:41
@malav2110
malav2110 requested a review from a team as a code owner January 6, 2026 01:41
@vercel

vercel Bot commented Jan 6, 2026

Copy link
Copy Markdown

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

Project Deployment Review Updated (UTC)
nodejs-org Ready Ready Preview Jan 9, 2026 0:47am

@github-actions

github-actions Bot commented Jan 6, 2026

Copy link
Copy Markdown
Contributor

👋 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 apps/site/hooks/client/useScroll.ts

Copilot AI left a comment

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.

Pull request overview

This PR implements scroll position preservation for the sidebar on the Learn Page, addressing issue #8336 where the sidebar would reset to the top when navigating between articles. The implementation uses React's forwardRef pattern and a custom hook to track and restore scroll positions across navigation.

Key Changes:

  • Created a new useNavigationState hook that debounces scroll events and stores scroll positions in a context-based state store
  • Modified the Sidebar component to support ref forwarding, allowing parent components to access the underlying DOM element
  • Integrated the scroll preservation functionality into the withSidebar component

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
packages/ui-components/src/Containers/Sidebar/index.tsx Converted from functional component to forwardRef to expose the sidebar element for scroll position tracking
apps/site/hooks/client/useNavigationState.ts New custom hook that manages scroll position state using context, debouncing, and DOM manipulation
apps/site/hooks/client/index.ts Added export for the new useNavigationState hook
apps/site/components/withSidebar.tsx Integrated scroll position preservation by using useNavigationState hook with a ref to the sidebar element

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/ui-components/src/Containers/Sidebar/index.tsx
Comment thread apps/site/hooks/client/useNavigationState.ts Outdated
Comment thread apps/site/hooks/client/useNavigationState.ts Outdated
Comment thread apps/site/hooks/client/useNavigationState.ts Outdated
Comment thread apps/site/components/withSidebar.tsx Outdated
Comment thread apps/site/components/withSidebar.tsx Outdated
Comment thread packages/ui-components/src/Containers/Sidebar/index.tsx
…and fixed potential memory leak issue in useNavigationState
Comment thread apps/site/components/withSidebar.tsx Outdated
Comment thread apps/site/components/withSidebar.tsx Outdated
Comment thread packages/ui-components/src/Containers/Sidebar/index.tsx Outdated
Comment thread apps/site/hooks/client/useScroll.ts
Comment thread apps/site/hooks/client/useScroll.ts
Comment thread apps/site/hooks/client/useNavigationState.ts Outdated
Comment thread apps/site/hooks/client/useNavigationState.ts Outdated
Comment thread apps/site/hooks/client/useNavigationState.ts Outdated
Comment thread apps/site/hooks/client/useNavigationState.ts Outdated
Comment thread apps/site/hooks/client/useNavigationState.ts Outdated
Comment thread apps/site/hooks/client/useNavigationState.ts Outdated
Comment thread packages/ui-components/src/Containers/Sidebar/index.tsx

@ovflowd ovflowd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I feel we're missing tests that verify the hook works as expected and scrolls to an element that was outside of the view.

Comment thread apps/site/hooks/client/useScrollToElement.ts Outdated
Comment thread apps/site/hooks/client/useScrollToElement.ts
Comment thread apps/site/hooks/client/index.ts

@ovflowd ovflowd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left a few nits, but SGTM! And thanks for the work and effort here 🙇

@avivkeller avivkeller left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you pnpm version patch in packages/ui-components?

Signed-off-by: Malav Shah <shahmalav1995@gmail.com>
@malav2110

malav2110 commented Jan 8, 2026

Copy link
Copy Markdown
Contributor Author

Addressed nitpicks and ran pnpm version patch command in ui-packages directory. :)

@ovflowd ovflowd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

SGTM, but left final two nits!

Comment thread apps/site/components/withSidebar.tsx
Comment thread apps/site/hooks/client/useScrollToElement.ts Outdated
@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Jan 10, 2026
@github-actions github-actions Bot removed the github_actions:pull-request Trigger Pull Request Checks label Jan 10, 2026
@ovflowd
ovflowd added this pull request to the merge queue Jan 10, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jan 10, 2026
@ovflowd
ovflowd added this pull request to the merge queue Jan 10, 2026
Merged via the queue into nodejs:main with commit 0743e54 Jan 11, 2026
20 checks passed

This branch was successfully deployed

2 active deployments
Storybook ae02e186 Deployed Jan 10, 2026 by ovflowd via Chromatic #1344
Preview ae02e186 Deployed Jan 9, 2026 by vercel[bot]
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.

Selected article in sidebar doesn’t scroll into view

4 participants