Skip to content

Commit 0743e54

Browse files
authored
feat(ui): implement sidebar scroll position preservation (#8517)
* feat(ui): implement sidebar scroll position preservation * fix: correct import statements for hooks in withSidebar component * fix: correct comment grammar, improve sidebar component display name and fixed potential memory leak issue in useNavigationState * fix: streamline imports in withSidebar component and update pathname handling in Sidebar * fix: remove unused imports and ensure 'use client' directive is present in Sidebar component * fix: replace useNavigationState with useScrollToElement in WithSidebar component and add scroll handling functionality * fix: clarify comment in handleScroll function to improve code readability * fix: addressed nitpicks and ran pnpm version path in ui-components directory * fix: remove unnecessary 'use client' directive from Sidebar component * fix: improve sidebar pathname handling * fix: remove unused locale handling from WithSidebar component --------- Signed-off-by: Malav Shah <[email protected]>
1 parent d394514 commit 0743e54

File tree

10 files changed

+321
-52
lines changed

10 files changed

+321
-52
lines changed

apps/site/components/withSidebar.tsx

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
'use client';
22

33
import Sidebar from '@node-core/ui-components/Containers/Sidebar';
4-
import { usePathname } from 'next/navigation';
5-
import { useLocale, useTranslations } from 'next-intl';
4+
import { useTranslations } from 'next-intl';
5+
import { useRef } from 'react';
66

77
import Link from '#site/components/Link';
8-
import { useClientContext } from '#site/hooks/client';
8+
import { useClientContext, useScrollToElement } from '#site/hooks/client';
99
import { useSiteNavigation } from '#site/hooks/generic';
10-
import { useRouter } from '#site/navigation.mjs';
10+
import { useRouter, usePathname } from '#site/navigation.mjs';
1111

1212
import type { NavigationKeys } from '#site/types';
1313
import type { RichTranslationValues } from 'next-intl';
@@ -21,14 +21,17 @@ type WithSidebarProps = {
2121
const WithSidebar: FC<WithSidebarProps> = ({ navKeys, context, ...props }) => {
2222
const { getSideNavigation } = useSiteNavigation();
2323
const pathname = usePathname()!;
24-
const locale = useLocale();
2524
const t = useTranslations();
2625
const { push } = useRouter();
2726
const { frontmatter } = useClientContext();
27+
const sidebarRef = useRef<HTMLElement>(null);
2828
const sideNavigation = getSideNavigation(navKeys, context);
2929

30+
// Preserve sidebar scroll position across navigations
31+
useScrollToElement('sidebar', sidebarRef);
32+
3033
const mappedSidebarItems =
31-
// If there's only a single navigation key, use it's sub-items
34+
// If there's only a single navigation key, use its sub-items
3235
// as our navigation.
3336
(navKeys.length === 1 ? sideNavigation[0][1].items : sideNavigation).map(
3437
([, { label, items }]) => ({
@@ -39,8 +42,9 @@ const WithSidebar: FC<WithSidebarProps> = ({ navKeys, context, ...props }) => {
3942

4043
return (
4144
<Sidebar
45+
ref={sidebarRef}
4246
groups={mappedSidebarItems}
43-
pathname={pathname.replace(`/${locale}`, '')}
47+
pathname={pathname}
4448
title={t('components.common.sidebar.title')}
4549
placeholder={frontmatter?.title}
4650
onSelect={push}
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
import { renderHook } from '@testing-library/react';
2+
import { afterEach, beforeEach, describe, it, mock } from 'node:test';
3+
import assert from 'node:assert/strict';
4+
5+
import useScrollToElement from '#site/hooks/client/useScrollToElement.js';
6+
import { NavigationStateContext } from '#site/providers/navigationStateProvider';
7+
8+
describe('useScrollToElement', () => {
9+
let mockElement;
10+
let mockRef;
11+
let navigationState;
12+
13+
beforeEach(() => {
14+
navigationState = {};
15+
16+
mockElement = {
17+
scrollTop: 0,
18+
scrollLeft: 0,
19+
scroll: mock.fn(),
20+
addEventListener: mock.fn(),
21+
removeEventListener: mock.fn(),
22+
};
23+
24+
mockRef = { current: mockElement };
25+
});
26+
27+
afterEach(() => {
28+
mock.reset();
29+
});
30+
31+
it('should handle scroll restoration with various scenarios', () => {
32+
const wrapper = ({ children }) => (
33+
<NavigationStateContext.Provider value={navigationState}>
34+
{children}
35+
</NavigationStateContext.Provider>
36+
);
37+
38+
// Should restore scroll position on mount if saved state exists
39+
navigationState.sidebar = { x: 100, y: 200 };
40+
const { unmount: unmount1 } = renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper });
41+
42+
assert.equal(mockElement.scroll.mock.callCount(), 1);
43+
assert.deepEqual(mockElement.scroll.mock.calls[0].arguments, [
44+
{ top: 200, behavior: 'auto' },
45+
]);
46+
47+
unmount1();
48+
mock.reset();
49+
mockElement.scroll = mock.fn();
50+
51+
// Should not restore if no saved state exists
52+
navigationState = {};
53+
const { unmount: unmount2 } = renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper });
54+
assert.equal(mockElement.scroll.mock.callCount(), 0);
55+
56+
unmount2();
57+
mock.reset();
58+
mockElement.scroll = mock.fn();
59+
60+
// Should not restore if current position matches saved state
61+
navigationState.sidebar = { x: 0, y: 0 };
62+
mockElement.scrollTop = 0;
63+
const { unmount: unmount3 } = renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper });
64+
assert.equal(mockElement.scroll.mock.callCount(), 0);
65+
66+
unmount3();
67+
mock.reset();
68+
mockElement.scroll = mock.fn();
69+
70+
// Should restore scroll to element that was outside viewport (deep scroll)
71+
navigationState.sidebar = { x: 0, y: 1500 };
72+
mockElement.scrollTop = 0;
73+
renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper });
74+
75+
assert.equal(mockElement.scroll.mock.callCount(), 1);
76+
assert.deepEqual(mockElement.scroll.mock.calls[0].arguments, [
77+
{ top: 1500, behavior: 'auto' },
78+
]);
79+
});
80+
81+
it('should persist and restore scroll position across navigation', async () => {
82+
const wrapper = ({ children }) => (
83+
<NavigationStateContext.Provider value={navigationState}>
84+
{children}
85+
</NavigationStateContext.Provider>
86+
);
87+
88+
// First render: user scrolls to position 800
89+
const { unmount } = renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper });
90+
91+
const scrollHandler = mockElement.addEventListener.mock.calls[0].arguments[1];
92+
mockElement.scrollTop = 800;
93+
mockElement.scrollLeft = 0;
94+
scrollHandler();
95+
96+
// Wait for debounce
97+
await new Promise(resolve => setTimeout(resolve, 350));
98+
99+
// Position should be saved
100+
assert.deepEqual(navigationState.sidebar, { x: 0, y: 800 });
101+
102+
// Simulate navigation (unmount)
103+
unmount();
104+
105+
// Simulate navigation back (remount with element at top)
106+
mockElement.scrollTop = 0;
107+
mock.reset();
108+
mockElement.scroll = mock.fn();
109+
mockElement.addEventListener = mock.fn();
110+
mockElement.removeEventListener = mock.fn();
111+
mockRef.current = mockElement;
112+
113+
renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper });
114+
115+
// Should restore to position 800
116+
assert.equal(mockElement.scroll.mock.callCount(), 1);
117+
assert.deepEqual(mockElement.scroll.mock.calls[0].arguments, [
118+
{ top: 800, behavior: 'auto' },
119+
]);
120+
121+
// Also test that scroll position is saved to navigation state during scroll
122+
mock.reset();
123+
mockElement.addEventListener = mock.fn();
124+
mockElement.scroll = mock.fn();
125+
navigationState = {};
126+
127+
renderHook(() => useScrollToElement('sidebar', mockRef), { wrapper });
128+
129+
// Get the scroll handler that was registered
130+
const scrollHandler2 = mockElement.addEventListener.mock.calls[0].arguments[1];
131+
132+
// Simulate scroll
133+
mockElement.scrollTop = 150;
134+
mockElement.scrollLeft = 50;
135+
136+
// Call the handler
137+
scrollHandler2();
138+
139+
// Wait for debounce (default 300ms)
140+
await new Promise(resolve => setTimeout(resolve, 350));
141+
142+
// Check that navigation state was updated
143+
assert.deepEqual(navigationState.sidebar, { x: 50, y: 150 });
144+
});
145+
});

apps/site/hooks/client/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
export { default as useDetectOS } from './useDetectOS';
22
export { default as useMediaQuery } from './useMediaQuery';
33
export { default as useClientContext } from './useClientContext';
4+
export { default as useScrollToElement } from './useScrollToElement';
5+
export { default as useScroll } from './useScroll';
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
'use client';
2+
3+
import { useEffect, useRef } from 'react';
4+
5+
import type { RefObject } from 'react';
6+
7+
type ScrollPosition = {
8+
x: number;
9+
y: number;
10+
};
11+
12+
type UseScrollOptions = {
13+
debounceTime?: number;
14+
onScroll?: (position: ScrollPosition) => void;
15+
};
16+
17+
// Custom hook to handle scroll events with optional debouncing
18+
const useScroll = <T extends HTMLElement>(
19+
ref: RefObject<T | null>,
20+
{ debounceTime = 300, onScroll }: UseScrollOptions = {}
21+
) => {
22+
const timeoutRef = useRef<NodeJS.Timeout | undefined>(undefined);
23+
24+
useEffect(() => {
25+
// Get the current element
26+
const element = ref.current;
27+
28+
// Return early if no element or onScroll callback is provided
29+
if (!element || !onScroll) {
30+
return;
31+
}
32+
33+
// Debounced scroll handler
34+
const handleScroll = () => {
35+
// Clear existing timeout
36+
if (timeoutRef.current) {
37+
clearTimeout(timeoutRef.current);
38+
}
39+
40+
// Set new timeout to call onScroll after debounceTime
41+
timeoutRef.current = setTimeout(() => {
42+
if (element) {
43+
onScroll({
44+
x: element.scrollLeft,
45+
y: element.scrollTop,
46+
});
47+
}
48+
}, debounceTime);
49+
};
50+
51+
element.addEventListener('scroll', handleScroll, { passive: true });
52+
53+
return () => {
54+
element.removeEventListener('scroll', handleScroll);
55+
// Clear any pending debounced calls
56+
if (timeoutRef.current) {
57+
clearTimeout(timeoutRef.current);
58+
}
59+
};
60+
}, [ref, onScroll, debounceTime]);
61+
};
62+
63+
export default useScroll;
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
'use client';
2+
3+
import { useContext, useEffect } from 'react';
4+
5+
import { NavigationStateContext } from '#site/providers/navigationStateProvider';
6+
7+
import type { RefObject } from 'react';
8+
9+
import useScroll from './useScroll';
10+
11+
const useScrollToElement = <T extends HTMLElement>(
12+
id: string,
13+
ref: RefObject<T | null>,
14+
debounceTime = 300
15+
) => {
16+
const navigationState = useContext(NavigationStateContext);
17+
18+
// Restore scroll position on mount
19+
useEffect(() => {
20+
if (!ref.current) {
21+
return;
22+
}
23+
24+
// Restore scroll position if saved state exists
25+
const savedState = navigationState[id];
26+
27+
// Scroll only if the saved position differs from current
28+
if (savedState && savedState.y !== ref.current.scrollTop) {
29+
ref.current.scroll({ top: savedState.y, behavior: 'auto' });
30+
}
31+
// navigationState is intentionally excluded
32+
// it's a stable object reference that doesn't need to trigger re-runs
33+
// eslint-disable-next-line react-hooks/exhaustive-deps
34+
}, [id, ref]);
35+
36+
// Save scroll position on scroll
37+
const handleScroll = (position: { x: number; y: number }) => {
38+
// Save the current scroll position in the navigation state
39+
const state = navigationState as Record<string, { x: number; y: number }>;
40+
state[id] = position;
41+
};
42+
43+
// Use the useScroll hook to handle scroll events with debouncing
44+
useScroll(ref, { debounceTime, onScroll: handleScroll });
45+
};
46+
47+
export default useScrollToElement;

apps/site/hooks/server/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
11
export { default as useClientContext } from './useClientContext';
2+
export { default as useScrollToElement } from './useScrollToElement';
3+
export { default as useScroll } from './useScroll';
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
const useScroll = () => {
2+
throw new Error('Attempted to call useScroll from RSC');
3+
};
4+
5+
export default useScroll;
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
const useScrollToElement = () => {
2+
throw new Error('Attempted to call useScrollToElement from RSC');
3+
};
4+
5+
export default useScrollToElement;

packages/ui-components/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@node-core/ui-components",
3-
"version": "1.5.3",
3+
"version": "1.5.4",
44
"type": "module",
55
"exports": {
66
"./*": [

0 commit comments

Comments
 (0)