nodejs / nodejs/nodejs.org

Sidebar scroll position resets on navigation, active item can be off-screen

オープン 初心者向け
#8,828 コメント 4 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

主要言語
TypeScript
スター
6.9k
フォーク
6.5k
平均マージ
2日 8時間
マージ済み PR(30日)
29

説明

I couldn't push my own branch to make a PR so sharing this as issue instead. The fix belongs in components/Sidebar/index.jsx.

Problem
Clicking an item in the left sidebar causes a full page reload because navigation uses window.location.href in components/Sidebar/index.jsx. After the reload, the sidebar’s internal scroll position resets to the top.

As a result, when a user clicks a page located lower in the sidebar list, such as an item under TypeScript or Diagnostics, the selected item may still be active but no longer visible. The user then has to scroll through the sidebar again to find their current location.

Reproduction steps

  • Open https://nodejs.org/learn
  • Scroll the left sidebar to a lower item, for example Diagnostics → Memory
  • Click the item
  • After the page reloads, the sidebar scroll position has returned to the top, and the active item is off-screen

Expected behavior

The active sidebar item should remain visible after navigation.

Proposed fix

When the sidebar mounts, locate the active link inside the sidebar <aside>.
If that link is outside the visible portion of the sidebar’s own scroll container, adjust aside.scrollTop so the active item is centered in view. This only affects the sidebar’s internal scroll position and does not change the main window scroll.

Example approach

useEffect(() => {
  const aside = asideRef.current;
  const active = aside?.querySelector(`a[href="${CSS.escape(pathname)}"]`);
  if (!aside || !active) return;

  const offsetTop =
    active.getBoundingClientRect().top -
    aside.getBoundingClientRect().top +
    aside.scrollTop;

  const viewTop = aside.scrollTop;
  const viewBottom = viewTop + aside.clientHeight;

  if (offsetTop >= viewTop && offsetTop + active.offsetHeight <= viewBottom) {
    return;
  }

  aside.scrollTop = Math.max(
    0,
    offsetTop - aside.clientHeight / 2 + active.offsetHeight / 2
  );
}, [pathname]);

Notes

This approach uses the existing forwardRef exposed by @node-core/ui-components/Containers/Sidebar, so no upstream or vendor changes are required.

Alternative considered

Persisting the sidebar’s scrollTop in sessionStorage across reloads was considered, but rejected. That approach adds more complexity, such as saving on unload, restoring at the right time during hydration, and handling stale scroll state when the sidebar structure changes.

Ensuring the active item is visible is simpler and aligns with expected user behavior in most cases.

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

components/Sidebar/index.jsx から始めて、既存の forwardRef とナビゲーションの動作を確認します。サイドバーの下部にある項目でリロードを再現し、ナビゲーション後にアクティブなリンクが表示されることを確認します。その際、スクロール位置が変わるのはサイドバー内部だけで、メインウィンドウのスクロール位置は変わらないことを確認してください。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
javascript, react
領域
frontend, web-dev
issue の種類
バグ
難易度
2/5
見積もり時間
1〜3時間
活発さ
活発
明瞭さ
明確に書かれている
初心者へのやさしさ
86/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。