From bc1941cffb545663b7b219769e2056f0a8113551 Mon Sep 17 00:00:00 2001 From: ulofiai Date: Fri, 7 Aug 2026 02:51:13 +0800 Subject: [PATCH] fix: keep active sidebar item visible --- apps/site/components/withSidebar.tsx | 4 +- .../__tests__/useScrollToElement.test.jsx | 56 +++++++++++++++++++ apps/site/hooks/useScrollToElement.ts | 52 +++++++++++++++-- 3 files changed, 105 insertions(+), 7 deletions(-) diff --git a/apps/site/components/withSidebar.tsx b/apps/site/components/withSidebar.tsx index fb9f04b2c27da..ab7b053496ff1 100644 --- a/apps/site/components/withSidebar.tsx +++ b/apps/site/components/withSidebar.tsx @@ -49,8 +49,8 @@ const WithSidebar: FC = ({ navKeys, context, ...props }) => { const sidebarRef = useRef(null); const sideNavigation = getSideNavigation(navKeys, context); - // Preserve sidebar scroll position across navigations - useScrollToElement('sidebar', sidebarRef); + // Preserve sidebar scroll position and keep the active item visible + useScrollToElement('sidebar', sidebarRef, pathname); const mappedSidebarItems = // If there's only a single navigation key, use its sub-items diff --git a/apps/site/hooks/__tests__/useScrollToElement.test.jsx b/apps/site/hooks/__tests__/useScrollToElement.test.jsx index a26dfe75545f2..2e04c8842ea2a 100644 --- a/apps/site/hooks/__tests__/useScrollToElement.test.jsx +++ b/apps/site/hooks/__tests__/useScrollToElement.test.jsx @@ -14,11 +14,14 @@ describe('useScrollToElement', () => { navigationState = {}; mockElement = { + clientHeight: 400, scrollTop: 0, scrollLeft: 0, scroll: mock.fn(), addEventListener: mock.fn(), removeEventListener: mock.fn(), + getBoundingClientRect: mock.fn(() => ({ top: 0 })), + querySelectorAll: mock.fn(() => []), }; mockRef = { current: mockElement }; @@ -87,6 +90,59 @@ describe('useScrollToElement', () => { ]); }); + it('should center the active item when it is outside the viewport', () => { + const wrapper = ({ children }) => ( + + {children} + + ); + + const activeElement = { + origin: window.location.origin, + pathname: '/learn/diagnostics/memory', + offsetHeight: 40, + getBoundingClientRect: mock.fn(() => ({ top: 900 })), + }; + + mockElement.querySelectorAll = mock.fn(() => [activeElement]); + + renderHook( + () => + useScrollToElement('sidebar', mockRef, '/learn/diagnostics/memory'), + { wrapper } + ); + + assert.equal(mockElement.scroll.mock.callCount(), 1); + assert.deepEqual(mockElement.scroll.mock.calls[0].arguments, [ + { top: 720, behavior: 'auto' }, + ]); + }); + + it('should not scroll when the active item is already visible', () => { + const wrapper = ({ children }) => ( + + {children} + + ); + + const activeElement = { + origin: window.location.origin, + pathname: window.location.pathname, + offsetHeight: 40, + getBoundingClientRect: mock.fn(() => ({ top: 100 })), + }; + + mockElement.querySelectorAll = mock.fn(() => [activeElement]); + + renderHook( + () => + useScrollToElement('sidebar', mockRef, '/learn/diagnostics/memory'), + { wrapper } + ); + + assert.equal(mockElement.scroll.mock.callCount(), 0); + }); + it('should persist and restore scroll position across navigation', async () => { const wrapper = ({ children }) => ( diff --git a/apps/site/hooks/useScrollToElement.ts b/apps/site/hooks/useScrollToElement.ts index c634eb8756c7f..2a9a283171e3f 100644 --- a/apps/site/hooks/useScrollToElement.ts +++ b/apps/site/hooks/useScrollToElement.ts @@ -11,13 +11,16 @@ import useScroll from './useScroll'; const useScrollToElement = ( id: string, ref: RefObject, + pathname?: string, debounceTime = 300 ) => { const navigationState = use(NavigationStateContext); - // Restore scroll position on mount + // Restore scroll position and keep the active link visible useEffect(() => { - if (!ref.current) { + const element = ref.current; + + if (!element) { return; } @@ -25,13 +28,52 @@ const useScrollToElement = ( const savedState = navigationState[id]; // Scroll only if the saved position differs from current - if (savedState && savedState.y !== ref.current.scrollTop) { - ref.current.scroll({ top: savedState.y, behavior: 'auto' }); + if (savedState && savedState.y !== element.scrollTop) { + element.scroll({ top: savedState.y, behavior: 'auto' }); + } + + if (!pathname) { + return; } + + // usePathname can omit the locale prefix, so compare resolved anchor paths + // against both the app pathname and the browser location. + const activeElement = Array.from( + element.querySelectorAll('a[href]') + ).find( + ({ origin, pathname: linkPathname }) => + origin === window.location.origin && + (linkPathname === window.location.pathname || linkPathname === pathname) + ); + + if (!activeElement) { + return; + } + + const activeRect = activeElement.getBoundingClientRect(); + const containerRect = element.getBoundingClientRect(); + const offsetTop = activeRect.top - containerRect.top + element.scrollTop; + const viewTop = element.scrollTop; + const viewBottom = viewTop + element.clientHeight; + + if ( + offsetTop >= viewTop && + offsetTop + activeElement.offsetHeight <= viewBottom + ) { + return; + } + + element.scroll({ + top: Math.max( + 0, + offsetTop - element.clientHeight / 2 + activeElement.offsetHeight / 2 + ), + behavior: 'auto', + }); // navigationState is intentionally excluded // it's a stable object reference that doesn't need to trigger re-runs // eslint-disable-next-line @eslint-react/exhaustive-deps - }, [id, ref]); + }, [id, pathname, ref]); // Save scroll position on scroll const handleScroll = (position: { x: number; y: number }) => {