From 07819798549e3208b7cb2c4f62705dcd5d4fd837 Mon Sep 17 00:00:00 2001 From: Malav Shah Date: Mon, 5 Jan 2026 18:36:19 -0700 Subject: [PATCH 01/11] feat(ui): implement sidebar scroll position preservation --- apps/site/components/withSidebar.tsx | 15 +++- apps/site/hooks/client/index.ts | 1 + apps/site/hooks/client/useNavigationState.ts | 42 ++++++++++ .../src/Containers/Sidebar/index.tsx | 84 +++++++++---------- 4 files changed, 95 insertions(+), 47 deletions(-) create mode 100644 apps/site/hooks/client/useNavigationState.ts diff --git a/apps/site/components/withSidebar.tsx b/apps/site/components/withSidebar.tsx index 6d5ab83b6605b..b7bf07175bfab 100644 --- a/apps/site/components/withSidebar.tsx +++ b/apps/site/components/withSidebar.tsx @@ -3,16 +3,18 @@ import Sidebar from '@node-core/ui-components/Containers/Sidebar'; import { usePathname } from 'next/navigation'; import { useLocale, useTranslations } from 'next-intl'; +import { useRef } from 'react'; import Link from '#site/components/Link'; -import { useClientContext } from '#site/hooks/client'; -import { useSiteNavigation } from '#site/hooks/generic'; import { useRouter } from '#site/navigation.mjs'; import type { NavigationKeys } from '#site/types'; import type { RichTranslationValues } from 'next-intl'; import type { FC } from 'react'; +import { useClientContext, useNavigationState } from '../hooks/client'; +import { useSiteNavigation } from '../hooks/generic'; + type WithSidebarProps = { navKeys: Array; context?: Record; @@ -25,8 +27,14 @@ const WithSidebar: FC = ({ navKeys, context, ...props }) => { const t = useTranslations(); const { push } = useRouter(); const { frontmatter } = useClientContext(); + const sidebarRef = useRef(null); const sideNavigation = getSideNavigation(navKeys, context); + const localePathname = pathname.replace(`/${locale}`, ''); + + // Preserve sidebar scroll position across navigations + useNavigationState('sidebar', sidebarRef); + const mappedSidebarItems = // If there's only a single navigation key, use it's sub-items // as our navigation. @@ -39,8 +47,9 @@ const WithSidebar: FC = ({ navKeys, context, ...props }) => { return ( ( + id: string, + ref: RefObject, + debounceTime = 300 +) => { + const navigationState = useContext(NavigationStateContext); + + const handleScroll = debounce(() => { + if (ref.current) { + navigationState[id] = { + x: ref.current.scrollLeft, + y: ref.current.scrollTop, + }; + } + }, debounceTime); + + useEffect(() => { + const element = ref.current; + if (element) { + if (navigationState[id] && navigationState[id].y !== element.scrollTop) { + element.scroll({ top: navigationState[id].y, behavior: 'instant' }); + } + + element.addEventListener('scroll', handleScroll, { passive: true }); + + return () => element.removeEventListener('scroll', handleScroll); + } + // We need this effect to run only on mount + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); +}; + +export default useNavigationState; diff --git a/packages/ui-components/src/Containers/Sidebar/index.tsx b/packages/ui-components/src/Containers/Sidebar/index.tsx index beb479d8a8bdc..b41c83e1630da 100644 --- a/packages/ui-components/src/Containers/Sidebar/index.tsx +++ b/packages/ui-components/src/Containers/Sidebar/index.tsx @@ -1,8 +1,10 @@ +import { forwardRef } from 'react'; + import WithNoScriptSelect from '#ui/Common/Select/NoScriptSelect'; import SidebarGroup from '#ui/Containers/Sidebar/SidebarGroup'; import type { LinkLike } from '#ui/types'; -import type { ComponentProps, FC, PropsWithChildren } from 'react'; +import type { ComponentProps, PropsWithChildren } from 'react'; import styles from './index.module.css'; @@ -17,52 +19,46 @@ type SidebarProps = { placeholder?: string; }; -const SideBar: FC> = ({ - groups, - pathname, - title, - onSelect, - as, - children, - placeholder, -}) => { - const selectItems = groups.map(({ items, groupName }) => ({ - label: groupName, - items: items.map(({ label, link }) => ({ value: link, label })), - })); +const SideBar = forwardRef>( + ({ groups, pathname, title, onSelect, as, children, placeholder }, ref) => { + const selectItems = groups.map(({ items, groupName }) => ({ + label: groupName, + items: items.map(({ label, link }) => ({ value: link, label })), + })); - const currentItem = selectItems - .flatMap(item => item.items) - .find(item => pathname === item.value); + const currentItem = selectItems + .flatMap(item => item.items) + .find(item => pathname === item.value); - return ( - + ); + } +); export default SideBar; From 41e38791ab49c898c9167ae19e2b0dc937acfdc1 Mon Sep 17 00:00:00 2001 From: Malav Shah Date: Mon, 5 Jan 2026 18:48:28 -0700 Subject: [PATCH 02/11] fix: correct import statements for hooks in withSidebar component --- apps/site/components/withSidebar.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/site/components/withSidebar.tsx b/apps/site/components/withSidebar.tsx index b7bf07175bfab..bbfa799d58e63 100644 --- a/apps/site/components/withSidebar.tsx +++ b/apps/site/components/withSidebar.tsx @@ -6,15 +6,15 @@ import { useLocale, useTranslations } from 'next-intl'; import { useRef } from 'react'; import Link from '#site/components/Link'; +import { useClientContext } from '#site/hooks/client'; +import { useNavigationState } from '#site/hooks/client'; +import { useSiteNavigation } from '#site/hooks/generic'; import { useRouter } from '#site/navigation.mjs'; import type { NavigationKeys } from '#site/types'; import type { RichTranslationValues } from 'next-intl'; import type { FC } from 'react'; -import { useClientContext, useNavigationState } from '../hooks/client'; -import { useSiteNavigation } from '../hooks/generic'; - type WithSidebarProps = { navKeys: Array; context?: Record; From 1ebf202cc040032dd4fe45ed9cb303a848449616 Mon Sep 17 00:00:00 2001 From: Malav Shah Date: Mon, 5 Jan 2026 21:13:39 -0700 Subject: [PATCH 03/11] fix: correct comment grammar, improve sidebar component display name and fixed potential memory leak issue in useNavigationState --- apps/site/components/withSidebar.tsx | 2 +- apps/site/hooks/client/useNavigationState.ts | 35 +++++++++++++------ .../src/Containers/Sidebar/index.tsx | 2 ++ 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/apps/site/components/withSidebar.tsx b/apps/site/components/withSidebar.tsx index bbfa799d58e63..cf019f59030a4 100644 --- a/apps/site/components/withSidebar.tsx +++ b/apps/site/components/withSidebar.tsx @@ -36,7 +36,7 @@ const WithSidebar: FC = ({ navKeys, context, ...props }) => { useNavigationState('sidebar', sidebarRef); const mappedSidebarItems = - // If there's only a single navigation key, use it's sub-items + // If there's only a single navigation key, use its sub-items // as our navigation. (navKeys.length === 1 ? sideNavigation[0][1].items : sideNavigation).map( ([, { label, items }]) => ({ diff --git a/apps/site/hooks/client/useNavigationState.ts b/apps/site/hooks/client/useNavigationState.ts index 961854280fe12..7c8fc8d022dcc 100644 --- a/apps/site/hooks/client/useNavigationState.ts +++ b/apps/site/hooks/client/useNavigationState.ts @@ -1,9 +1,8 @@ 'use client'; -import { useContext, useEffect } from 'react'; +import { useCallback, useContext, useEffect, useRef } from 'react'; import { NavigationStateContext } from '#site/providers/navigationStateProvider'; -import { debounce } from '#site/util/objects'; import type { RefObject } from 'react'; @@ -13,26 +12,40 @@ const useNavigationState = ( debounceTime = 300 ) => { const navigationState = useContext(NavigationStateContext); + const timeoutRef = useRef(null); - const handleScroll = debounce(() => { - if (ref.current) { - navigationState[id] = { - x: ref.current.scrollLeft, - y: ref.current.scrollTop, - }; + const handleScroll = useCallback(() => { + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); } - }, debounceTime); + + timeoutRef.current = setTimeout(() => { + if (ref.current) { + navigationState[id] = { + x: ref.current.scrollLeft, + y: ref.current.scrollTop, + }; + } + }, debounceTime); + }, [id, ref, navigationState, debounceTime]); useEffect(() => { const element = ref.current; if (element) { if (navigationState[id] && navigationState[id].y !== element.scrollTop) { - element.scroll({ top: navigationState[id].y, behavior: 'instant' }); + element.scroll({ top: navigationState[id].y, behavior: 'auto' }); } element.addEventListener('scroll', handleScroll, { passive: true }); - return () => element.removeEventListener('scroll', handleScroll); + return () => { + element.removeEventListener('scroll', handleScroll); + // Clear any pending debounced calls + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); + timeoutRef.current = null; + } + }; } // We need this effect to run only on mount // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/packages/ui-components/src/Containers/Sidebar/index.tsx b/packages/ui-components/src/Containers/Sidebar/index.tsx index b41c83e1630da..7c9dbf12c34b0 100644 --- a/packages/ui-components/src/Containers/Sidebar/index.tsx +++ b/packages/ui-components/src/Containers/Sidebar/index.tsx @@ -61,4 +61,6 @@ const SideBar = forwardRef>( } ); +SideBar.displayName = 'SideBar'; + export default SideBar; From ece53b49c1ed621ac2d6fb03b3be716bd6211366 Mon Sep 17 00:00:00 2001 From: Malav Shah Date: Tue, 6 Jan 2026 17:13:16 -0700 Subject: [PATCH 04/11] fix: streamline imports in withSidebar component and update pathname handling in Sidebar --- apps/site/components/withSidebar.tsx | 13 ++++--------- .../ui-components/src/Containers/Sidebar/index.tsx | 2 -- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/apps/site/components/withSidebar.tsx b/apps/site/components/withSidebar.tsx index cf019f59030a4..2cd4db2fd6bed 100644 --- a/apps/site/components/withSidebar.tsx +++ b/apps/site/components/withSidebar.tsx @@ -1,15 +1,13 @@ 'use client'; import Sidebar from '@node-core/ui-components/Containers/Sidebar'; -import { usePathname } from 'next/navigation'; -import { useLocale, useTranslations } from 'next-intl'; +import { useTranslations } from 'next-intl'; import { useRef } from 'react'; import Link from '#site/components/Link'; -import { useClientContext } from '#site/hooks/client'; -import { useNavigationState } from '#site/hooks/client'; +import { useClientContext, useNavigationState } from '#site/hooks/client'; import { useSiteNavigation } from '#site/hooks/generic'; -import { useRouter } from '#site/navigation.mjs'; +import { useRouter, usePathname } from '#site/navigation.mjs'; import type { NavigationKeys } from '#site/types'; import type { RichTranslationValues } from 'next-intl'; @@ -23,15 +21,12 @@ type WithSidebarProps = { const WithSidebar: FC = ({ navKeys, context, ...props }) => { const { getSideNavigation } = useSiteNavigation(); const pathname = usePathname()!; - const locale = useLocale(); const t = useTranslations(); const { push } = useRouter(); const { frontmatter } = useClientContext(); const sidebarRef = useRef(null); const sideNavigation = getSideNavigation(navKeys, context); - const localePathname = pathname.replace(`/${locale}`, ''); - // Preserve sidebar scroll position across navigations useNavigationState('sidebar', sidebarRef); @@ -49,7 +44,7 @@ const WithSidebar: FC = ({ navKeys, context, ...props }) => { >( } ); -SideBar.displayName = 'SideBar'; - export default SideBar; From 0f2b57b8c04dd04d9154c4a86ec16589def7ec34 Mon Sep 17 00:00:00 2001 From: Malav Shah Date: Tue, 6 Jan 2026 17:49:05 -0700 Subject: [PATCH 05/11] fix: remove unused imports and ensure 'use client' directive is present in Sidebar component --- apps/site/hooks/client/useNavigationState.ts | 61 ++++++++++--------- .../src/Containers/Sidebar/index.tsx | 2 + 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/apps/site/hooks/client/useNavigationState.ts b/apps/site/hooks/client/useNavigationState.ts index 7c8fc8d022dcc..be1ece5bebf7c 100644 --- a/apps/site/hooks/client/useNavigationState.ts +++ b/apps/site/hooks/client/useNavigationState.ts @@ -1,6 +1,6 @@ 'use client'; -import { useCallback, useContext, useEffect, useRef } from 'react'; +import { useContext, useEffect, useRef } from 'react'; import { NavigationStateContext } from '#site/providers/navigationStateProvider'; @@ -12,41 +12,44 @@ const useNavigationState = ( debounceTime = 300 ) => { const navigationState = useContext(NavigationStateContext); - const timeoutRef = useRef(null); + const timeoutRef = useRef(undefined); - const handleScroll = useCallback(() => { - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); + useEffect(() => { + const element = ref.current; + if (!element) { + return; } - timeoutRef.current = setTimeout(() => { - if (ref.current) { - navigationState[id] = { - x: ref.current.scrollLeft, - y: ref.current.scrollTop, - }; - } - }, debounceTime); - }, [id, ref, navigationState, debounceTime]); + // Restore scroll position if saved state exists + if (navigationState[id] && navigationState[id].y !== element.scrollTop) { + element.scroll({ top: navigationState[id].y, behavior: 'auto' }); + } - useEffect(() => { - const element = ref.current; - if (element) { - if (navigationState[id] && navigationState[id].y !== element.scrollTop) { - element.scroll({ top: navigationState[id].y, behavior: 'auto' }); + // Debounced scroll handler + const handleScroll = () => { + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); } - element.addEventListener('scroll', handleScroll, { passive: true }); - - return () => { - element.removeEventListener('scroll', handleScroll); - // Clear any pending debounced calls - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); - timeoutRef.current = null; + timeoutRef.current = setTimeout(() => { + if (element) { + navigationState[id] = { + x: element.scrollLeft, + y: element.scrollTop, + }; } - }; - } + }, debounceTime); + }; + + element.addEventListener('scroll', handleScroll, { passive: true }); + + return () => { + element.removeEventListener('scroll', handleScroll); + // Clear any pending debounced calls + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); + } + }; // We need this effect to run only on mount // eslint-disable-next-line react-hooks/exhaustive-deps }, []); diff --git a/packages/ui-components/src/Containers/Sidebar/index.tsx b/packages/ui-components/src/Containers/Sidebar/index.tsx index b41c83e1630da..f8c60843522f7 100644 --- a/packages/ui-components/src/Containers/Sidebar/index.tsx +++ b/packages/ui-components/src/Containers/Sidebar/index.tsx @@ -1,3 +1,5 @@ +'use client'; + import { forwardRef } from 'react'; import WithNoScriptSelect from '#ui/Common/Select/NoScriptSelect'; From 0212fba81d0ff70311bedcbfb95c0da3c67da68d Mon Sep 17 00:00:00 2001 From: Malav Shah Date: Tue, 6 Jan 2026 23:33:05 -0700 Subject: [PATCH 06/11] fix: replace useNavigationState with useScrollToElement in WithSidebar component and add scroll handling functionality --- apps/site/components/withSidebar.tsx | 4 +- .../__tests__/useScrollToElement.test.jsx | 145 ++++++++++++++++++ apps/site/hooks/client/index.ts | 3 +- .../{useNavigationState.ts => useScroll.ts} | 43 +++--- apps/site/hooks/client/useScrollToElement.ts | 49 ++++++ 5 files changed, 222 insertions(+), 22 deletions(-) create mode 100644 apps/site/hooks/client/__tests__/useScrollToElement.test.jsx rename apps/site/hooks/client/{useNavigationState.ts => useScroll.ts} (54%) create mode 100644 apps/site/hooks/client/useScrollToElement.ts diff --git a/apps/site/components/withSidebar.tsx b/apps/site/components/withSidebar.tsx index 2cd4db2fd6bed..91866828ac934 100644 --- a/apps/site/components/withSidebar.tsx +++ b/apps/site/components/withSidebar.tsx @@ -5,7 +5,7 @@ import { useTranslations } from 'next-intl'; import { useRef } from 'react'; import Link from '#site/components/Link'; -import { useClientContext, useNavigationState } from '#site/hooks/client'; +import { useClientContext, useScrollToElement } from '#site/hooks/client'; import { useSiteNavigation } from '#site/hooks/generic'; import { useRouter, usePathname } from '#site/navigation.mjs'; @@ -28,7 +28,7 @@ const WithSidebar: FC = ({ navKeys, context, ...props }) => { const sideNavigation = getSideNavigation(navKeys, context); // Preserve sidebar scroll position across navigations - useNavigationState('sidebar', sidebarRef); + useScrollToElement('sidebar', sidebarRef); const mappedSidebarItems = // If there's only a single navigation key, use its sub-items diff --git a/apps/site/hooks/client/__tests__/useScrollToElement.test.jsx b/apps/site/hooks/client/__tests__/useScrollToElement.test.jsx new file mode 100644 index 0000000000000..bc434f55097f8 --- /dev/null +++ b/apps/site/hooks/client/__tests__/useScrollToElement.test.jsx @@ -0,0 +1,145 @@ +import { renderHook } from '@testing-library/react'; +import { afterEach, beforeEach, describe, it, mock } from 'node:test'; +import assert from 'node:assert/strict'; + +import useScrollToElement from '#site/hooks/client/useScrollToElement.js'; +import { NavigationStateContext } from '#site/providers/navigationStateProvider'; + +describe('useScrollToElement', () => { + let mockElement; + let mockRef; + let navigationState; + + beforeEach(() => { + navigationState = {}; + + mockElement = { + scrollTop: 0, + scrollLeft: 0, + scroll: mock.fn(), + addEventListener: mock.fn(), + removeEventListener: mock.fn(), + }; + + mockRef = { current: mockElement }; + }); + + afterEach(() => { + mock.reset(); + }); + + it('should handle scroll restoration with various scenarios', () => { + const wrapper = ({ children }) => ( + + {children} + + ); + + // Should restore scroll position on mount if saved state exists + navigationState.sidebar = { x: 100, y: 200 }; + const { unmount: unmount1 } = renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper }); + + assert.equal(mockElement.scroll.mock.callCount(), 1); + assert.deepEqual(mockElement.scroll.mock.calls[0].arguments, [ + { top: 200, behavior: 'auto' }, + ]); + + unmount1(); + mock.reset(); + mockElement.scroll = mock.fn(); + + // Should not restore if no saved state exists + navigationState = {}; + const { unmount: unmount2 } = renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper }); + assert.equal(mockElement.scroll.mock.callCount(), 0); + + unmount2(); + mock.reset(); + mockElement.scroll = mock.fn(); + + // Should not restore if current position matches saved state + navigationState.sidebar = { x: 0, y: 0 }; + mockElement.scrollTop = 0; + const { unmount: unmount3 } = renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper }); + assert.equal(mockElement.scroll.mock.callCount(), 0); + + unmount3(); + mock.reset(); + mockElement.scroll = mock.fn(); + + // Should restore scroll to element that was outside viewport (deep scroll) + navigationState.sidebar = { x: 0, y: 1500 }; + mockElement.scrollTop = 0; + renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper }); + + assert.equal(mockElement.scroll.mock.callCount(), 1); + assert.deepEqual(mockElement.scroll.mock.calls[0].arguments, [ + { top: 1500, behavior: 'auto' }, + ]); + }); + + it('should persist and restore scroll position across navigation', async () => { + const wrapper = ({ children }) => ( + + {children} + + ); + + // First render: user scrolls to position 800 + const { unmount } = renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper }); + + const scrollHandler = mockElement.addEventListener.mock.calls[0].arguments[1]; + mockElement.scrollTop = 800; + mockElement.scrollLeft = 0; + scrollHandler(); + + // Wait for debounce + await new Promise(resolve => setTimeout(resolve, 350)); + + // Position should be saved + assert.deepEqual(navigationState.sidebar, { x: 0, y: 800 }); + + // Simulate navigation (unmount) + unmount(); + + // Simulate navigation back (remount with element at top) + mockElement.scrollTop = 0; + mock.reset(); + mockElement.scroll = mock.fn(); + mockElement.addEventListener = mock.fn(); + mockElement.removeEventListener = mock.fn(); + mockRef.current = mockElement; + + renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper }); + + // Should restore to position 800 + assert.equal(mockElement.scroll.mock.callCount(), 1); + assert.deepEqual(mockElement.scroll.mock.calls[0].arguments, [ + { top: 800, behavior: 'auto' }, + ]); + + // Also test that scroll position is saved to navigation state during scroll + mock.reset(); + mockElement.addEventListener = mock.fn(); + mockElement.scroll = mock.fn(); + navigationState = {}; + + renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper }); + + // Get the scroll handler that was registered + const scrollHandler2 = mockElement.addEventListener.mock.calls[0].arguments[1]; + + // Simulate scroll + mockElement.scrollTop = 150; + mockElement.scrollLeft = 50; + + // Call the handler + scrollHandler2(); + + // Wait for debounce (default 300ms) + await new Promise(resolve => setTimeout(resolve, 350)); + + // Check that navigation state was updated + assert.deepEqual(navigationState.sidebar, { x: 50, y: 150 }); + }); +}); diff --git a/apps/site/hooks/client/index.ts b/apps/site/hooks/client/index.ts index 035b3b58548e4..bd399cf4e8b4a 100644 --- a/apps/site/hooks/client/index.ts +++ b/apps/site/hooks/client/index.ts @@ -1,4 +1,5 @@ export { default as useDetectOS } from './useDetectOS'; export { default as useMediaQuery } from './useMediaQuery'; export { default as useClientContext } from './useClientContext'; -export { default as useNavigationState } from './useNavigationState'; +export { default as useScrollToElement } from './useScrollToElement'; +export { default as useScroll } from './useScroll'; diff --git a/apps/site/hooks/client/useNavigationState.ts b/apps/site/hooks/client/useScroll.ts similarity index 54% rename from apps/site/hooks/client/useNavigationState.ts rename to apps/site/hooks/client/useScroll.ts index be1ece5bebf7c..6bdede83a8281 100644 --- a/apps/site/hooks/client/useNavigationState.ts +++ b/apps/site/hooks/client/useScroll.ts @@ -1,42 +1,49 @@ 'use client'; -import { useContext, useEffect, useRef } from 'react'; - -import { NavigationStateContext } from '#site/providers/navigationStateProvider'; +import { useEffect, useRef } from 'react'; import type { RefObject } from 'react'; -const useNavigationState = ( - id: string, +type ScrollPosition = { + x: number; + y: number; +}; + +type UseScrollOptions = { + debounceTime?: number; + onScroll?: (position: ScrollPosition) => void; +}; + +// Custom hook to handle scroll events with optional debouncing +const useScroll = ( ref: RefObject, - debounceTime = 300 + { debounceTime = 300, onScroll }: UseScrollOptions = {} ) => { - const navigationState = useContext(NavigationStateContext); const timeoutRef = useRef(undefined); useEffect(() => { + // Get the current element const element = ref.current; - if (!element) { - return; - } - // Restore scroll position if saved state exists - if (navigationState[id] && navigationState[id].y !== element.scrollTop) { - element.scroll({ top: navigationState[id].y, behavior: 'auto' }); + // Return early if no element or onScroll callback is provided + if (!element || !onScroll) { + return; } // Debounced scroll handler const handleScroll = () => { + // Clear existing timeout if (timeoutRef.current) { clearTimeout(timeoutRef.current); } + // Set new timeout to call onScroll after debounceTime timeoutRef.current = setTimeout(() => { if (element) { - navigationState[id] = { + onScroll({ x: element.scrollLeft, y: element.scrollTop, - }; + }); } }, debounceTime); }; @@ -50,9 +57,7 @@ const useNavigationState = ( clearTimeout(timeoutRef.current); } }; - // We need this effect to run only on mount - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [ref, onScroll, debounceTime]); }; -export default useNavigationState; +export default useScroll; diff --git a/apps/site/hooks/client/useScrollToElement.ts b/apps/site/hooks/client/useScrollToElement.ts new file mode 100644 index 0000000000000..ce4c3f0ae5e0d --- /dev/null +++ b/apps/site/hooks/client/useScrollToElement.ts @@ -0,0 +1,49 @@ +'use client'; + +import { useCallback, useContext, useEffect } from 'react'; + +import { NavigationStateContext } from '#site/providers/navigationStateProvider'; + +import type { RefObject } from 'react'; + +import useScroll from './useScroll'; + +const useScrollToElement = ( + id: string, + ref: RefObject, + debounceTime = 300 +) => { + const navigationState = useContext(NavigationStateContext); + + // Restore scroll position on mount + useEffect(() => { + const element = ref.current; + if (!element) { + return; + } + + // Restore scroll position if saved state exists + const savedState = navigationState[id]; + + // Scroll only if the saved position differs from current + if (savedState && savedState.y !== element.scrollTop) { + element.scroll({ top: savedState.y, behavior: 'auto' }); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [id, ref]); + + // Save scroll position on scroll + const handleScroll = useCallback( + (position: { x: number; y: number }) => { + // Direct mutation is safe here as navigationState is a ref's .current value + const state = navigationState as Record; + state[id] = position; + }, + [id, navigationState] + ); + + // Use the useScroll hook to handle scroll events with debouncing + useScroll(ref, { debounceTime, onScroll: handleScroll }); +}; + +export default useScrollToElement; From 3e54d01194331a95b773e64af9527b658923a726 Mon Sep 17 00:00:00 2001 From: Malav Shah Date: Tue, 6 Jan 2026 23:41:56 -0700 Subject: [PATCH 07/11] fix: clarify comment in handleScroll function to improve code readability --- apps/site/hooks/client/useScrollToElement.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/site/hooks/client/useScrollToElement.ts b/apps/site/hooks/client/useScrollToElement.ts index ce4c3f0ae5e0d..40f35fb1d25b7 100644 --- a/apps/site/hooks/client/useScrollToElement.ts +++ b/apps/site/hooks/client/useScrollToElement.ts @@ -35,7 +35,7 @@ const useScrollToElement = ( // Save scroll position on scroll const handleScroll = useCallback( (position: { x: number; y: number }) => { - // Direct mutation is safe here as navigationState is a ref's .current value + // Save the current scroll position in the navigation state const state = navigationState as Record; state[id] = position; }, From 9d06df5e8fe31568f351f5ca232729dbe3d1ebc0 Mon Sep 17 00:00:00 2001 From: Malav Shah Date: Thu, 8 Jan 2026 00:10:35 -0700 Subject: [PATCH 08/11] fix: addressed nitpicks and ran pnpm version path in ui-components directory --- apps/site/hooks/client/useScrollToElement.ts | 17 ++++++++--------- apps/site/hooks/server/index.ts | 2 ++ apps/site/hooks/server/useScroll.ts | 5 +++++ apps/site/hooks/server/useScrollToElement.ts | 5 +++++ packages/ui-components/package.json | 2 +- 5 files changed, 21 insertions(+), 10 deletions(-) create mode 100644 apps/site/hooks/server/useScroll.ts create mode 100644 apps/site/hooks/server/useScrollToElement.ts diff --git a/apps/site/hooks/client/useScrollToElement.ts b/apps/site/hooks/client/useScrollToElement.ts index 40f35fb1d25b7..d4dba677ed386 100644 --- a/apps/site/hooks/client/useScrollToElement.ts +++ b/apps/site/hooks/client/useScrollToElement.ts @@ -1,6 +1,6 @@ 'use client'; -import { useCallback, useContext, useEffect } from 'react'; +import { useContext, useEffect } from 'react'; import { NavigationStateContext } from '#site/providers/navigationStateProvider'; @@ -29,18 +29,17 @@ const useScrollToElement = ( if (savedState && savedState.y !== element.scrollTop) { element.scroll({ top: savedState.y, behavior: 'auto' }); } + // navigationState is intentionally excluded + // it's a stable object reference that doesn't need to trigger re-runs // eslint-disable-next-line react-hooks/exhaustive-deps }, [id, ref]); // Save scroll position on scroll - const handleScroll = useCallback( - (position: { x: number; y: number }) => { - // Save the current scroll position in the navigation state - const state = navigationState as Record; - state[id] = position; - }, - [id, navigationState] - ); + const handleScroll = (position: { x: number; y: number }) => { + // Save the current scroll position in the navigation state + const state = navigationState as Record; + state[id] = position; + }; // Use the useScroll hook to handle scroll events with debouncing useScroll(ref, { debounceTime, onScroll: handleScroll }); diff --git a/apps/site/hooks/server/index.ts b/apps/site/hooks/server/index.ts index 85c173af25f29..4493382a557e3 100644 --- a/apps/site/hooks/server/index.ts +++ b/apps/site/hooks/server/index.ts @@ -1 +1,3 @@ export { default as useClientContext } from './useClientContext'; +export { default as useScrollToElement } from './useScrollToElement'; +export { default as useScroll } from './useScroll'; diff --git a/apps/site/hooks/server/useScroll.ts b/apps/site/hooks/server/useScroll.ts new file mode 100644 index 0000000000000..89485a459dfe6 --- /dev/null +++ b/apps/site/hooks/server/useScroll.ts @@ -0,0 +1,5 @@ +const useScroll = () => { + throw new Error('Attempted to call useScroll from RSC'); +}; + +export default useScroll; diff --git a/apps/site/hooks/server/useScrollToElement.ts b/apps/site/hooks/server/useScrollToElement.ts new file mode 100644 index 0000000000000..30ec4d6505439 --- /dev/null +++ b/apps/site/hooks/server/useScrollToElement.ts @@ -0,0 +1,5 @@ +const useScrollToElement = () => { + throw new Error('Attempted to call useScrollToElement from RSC'); +}; + +export default useScrollToElement; diff --git a/packages/ui-components/package.json b/packages/ui-components/package.json index 03522769bab71..95c0e02c341eb 100644 --- a/packages/ui-components/package.json +++ b/packages/ui-components/package.json @@ -1,6 +1,6 @@ { "name": "@node-core/ui-components", - "version": "1.5.1", + "version": "1.5.2", "type": "module", "exports": { "./*": [ From c723ccfa4f5811bf797fea9c7a44aaca617e4f97 Mon Sep 17 00:00:00 2001 From: Malav Shah Date: Thu, 8 Jan 2026 17:16:06 -0700 Subject: [PATCH 09/11] fix: remove unnecessary 'use client' directive from Sidebar component --- packages/ui-components/src/Containers/Sidebar/index.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/ui-components/src/Containers/Sidebar/index.tsx b/packages/ui-components/src/Containers/Sidebar/index.tsx index f8c60843522f7..b41c83e1630da 100644 --- a/packages/ui-components/src/Containers/Sidebar/index.tsx +++ b/packages/ui-components/src/Containers/Sidebar/index.tsx @@ -1,5 +1,3 @@ -'use client'; - import { forwardRef } from 'react'; import WithNoScriptSelect from '#ui/Common/Select/NoScriptSelect'; From 48b3f00bc8550733ea82e21b34793f4ca97b8363 Mon Sep 17 00:00:00 2001 From: Malav Shah Date: Thu, 8 Jan 2026 17:39:25 -0700 Subject: [PATCH 10/11] fix: improve sidebar pathname handling --- apps/site/components/withSidebar.tsx | 5 +++-- apps/site/hooks/client/useScrollToElement.ts | 7 +++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/site/components/withSidebar.tsx b/apps/site/components/withSidebar.tsx index 91866828ac934..a42c883084ea2 100644 --- a/apps/site/components/withSidebar.tsx +++ b/apps/site/components/withSidebar.tsx @@ -1,7 +1,7 @@ 'use client'; import Sidebar from '@node-core/ui-components/Containers/Sidebar'; -import { useTranslations } from 'next-intl'; +import { useLocale, useTranslations } from 'next-intl'; import { useRef } from 'react'; import Link from '#site/components/Link'; @@ -21,6 +21,7 @@ type WithSidebarProps = { const WithSidebar: FC = ({ navKeys, context, ...props }) => { const { getSideNavigation } = useSiteNavigation(); const pathname = usePathname()!; + const locale = useLocale(); const t = useTranslations(); const { push } = useRouter(); const { frontmatter } = useClientContext(); @@ -44,7 +45,7 @@ const WithSidebar: FC = ({ navKeys, context, ...props }) => { ( // Restore scroll position on mount useEffect(() => { - const element = ref.current; - if (!element) { + if (!ref.current) { return; } @@ -26,8 +25,8 @@ const useScrollToElement = ( const savedState = navigationState[id]; // Scroll only if the saved position differs from current - if (savedState && savedState.y !== element.scrollTop) { - element.scroll({ top: savedState.y, behavior: 'auto' }); + if (savedState && savedState.y !== ref.current.scrollTop) { + ref.current.scroll({ top: savedState.y, behavior: 'auto' }); } // navigationState is intentionally excluded // it's a stable object reference that doesn't need to trigger re-runs From ae02e18636d4e12a9c0432d5ef8e9d1b9b5fe70c Mon Sep 17 00:00:00 2001 From: Malav Shah Date: Thu, 8 Jan 2026 17:45:39 -0700 Subject: [PATCH 11/11] fix: remove unused locale handling from WithSidebar component --- apps/site/components/withSidebar.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/apps/site/components/withSidebar.tsx b/apps/site/components/withSidebar.tsx index a42c883084ea2..91866828ac934 100644 --- a/apps/site/components/withSidebar.tsx +++ b/apps/site/components/withSidebar.tsx @@ -1,7 +1,7 @@ 'use client'; import Sidebar from '@node-core/ui-components/Containers/Sidebar'; -import { useLocale, useTranslations } from 'next-intl'; +import { useTranslations } from 'next-intl'; import { useRef } from 'react'; import Link from '#site/components/Link'; @@ -21,7 +21,6 @@ type WithSidebarProps = { const WithSidebar: FC = ({ navKeys, context, ...props }) => { const { getSideNavigation } = useSiteNavigation(); const pathname = usePathname()!; - const locale = useLocale(); const t = useTranslations(); const { push } = useRouter(); const { frontmatter } = useClientContext(); @@ -45,7 +44,7 @@ const WithSidebar: FC = ({ navKeys, context, ...props }) => {