Skip to content

Commit e943265

Browse files
committed
Prevent nav from collapsing after menu click
When users click a chapter link in the mobile menu, the navigation bar immediately collapses due to the scroll triggered by jumping to the chapter. Keep the nav expanded make it easier to click through chapters. Also ensure scroll margin matches height of the navigation bar.
1 parent f125c36 commit e943265

4 files changed

Lines changed: 142 additions & 30 deletions

File tree

entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigationPresenceProvider-spec.js

Lines changed: 109 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,24 @@
11
import React from 'react';
22

3-
import {DefaultNavigationPresenceProvider} from 'widgets/defaultNavigation/DefaultNavigationPresenceProvider';
3+
import {
4+
DefaultNavigationPresenceProvider,
5+
useDefaultNavigationState
6+
} from 'widgets/defaultNavigation/DefaultNavigationPresenceProvider';
47

58
import styles from 'widgets/defaultNavigation/presenceClassNames.module.css';
69

710
import {renderInEntry} from 'pageflow-scrolled/testHelpers';
811
import {act} from '@testing-library/react';
912
import '@testing-library/jest-dom/extend-expect';
1013

14+
function LockNavButton() {
15+
const {lockNavExpanded} = useDefaultNavigationState();
16+
return <button onClick={lockNavExpanded}>Lock</button>;
17+
}
18+
1119
describe('DefaultNavigationPresenceProvider', () => {
1220
afterEach(() => jest.restoreAllMocks());
21+
1322
it('renders wrapper with class setting --widget-margin-top-max by default', () => {
1423
const {container} = renderInEntry(
1524
<DefaultNavigationPresenceProvider configuration={{}}>
@@ -66,41 +75,114 @@ describe('DefaultNavigationPresenceProvider', () => {
6675
expect(container.firstChild).toHaveClass(styles.expanded);
6776
});
6877

69-
it('toggles expanded class based on scroll direction', () => {
70-
Object.defineProperty(window, 'scrollY', {
71-
writable: true,
72-
value: 0
78+
describe('scroll direction', () => {
79+
beforeEach(() => {
80+
jest.useFakeTimers();
81+
82+
Object.defineProperty(window, 'scrollY', {
83+
writable: true,
84+
value: 0
85+
});
86+
87+
jest.spyOn(document.body, 'getBoundingClientRect').mockImplementation(() => ({
88+
top: -window.scrollY,
89+
left: 0,
90+
right: 1024,
91+
bottom: 768 - window.scrollY,
92+
width: 1024,
93+
height: 768
94+
}));
7395
});
7496

75-
jest.spyOn(document.body, 'getBoundingClientRect').mockImplementation(() => ({
76-
top: -window.scrollY,
77-
left: 0,
78-
right: 1024,
79-
bottom: 768 - window.scrollY,
80-
width: 1024,
81-
height: 768
82-
}));
97+
afterEach(() => {
98+
jest.useRealTimers();
99+
});
83100

84-
const {container} = renderInEntry(
85-
<DefaultNavigationPresenceProvider configuration={{}}>
86-
<div />
87-
</DefaultNavigationPresenceProvider>
88-
);
101+
it('toggles expanded class based on scroll direction', () => {
102+
const {container} = renderInEntry(
103+
<DefaultNavigationPresenceProvider configuration={{}}>
104+
<div />
105+
</DefaultNavigationPresenceProvider>
106+
);
89107

90-
expect(container.firstChild).toHaveClass(styles.expanded);
108+
expect(container.firstChild).toHaveClass(styles.expanded);
109+
110+
act(() => {
111+
window.scrollY = 100;
112+
window.dispatchEvent(new Event('scroll'));
113+
});
114+
115+
expect(container.firstChild).not.toHaveClass(styles.expanded);
116+
117+
act(() => {
118+
window.scrollY = 50;
119+
window.dispatchEvent(new Event('scroll'));
120+
});
121+
122+
expect(container.firstChild).toHaveClass(styles.expanded);
123+
});
91124

92-
act(() => {
93-
window.scrollY = 100;
94-
window.dispatchEvent(new Event('scroll'));
125+
it('stays expanded during scroll lock', () => {
126+
const {container, getByText} = renderInEntry(
127+
<DefaultNavigationPresenceProvider configuration={{}}>
128+
<LockNavButton />
129+
</DefaultNavigationPresenceProvider>
130+
);
131+
132+
act(() => {
133+
getByText('Lock').click();
134+
});
135+
136+
act(() => {
137+
window.scrollY = 100;
138+
window.dispatchEvent(new Event('scroll'));
139+
});
140+
141+
expect(container.firstChild).toHaveClass(styles.expanded);
95142
});
96143

97-
expect(container.firstChild).not.toHaveClass(styles.expanded);
144+
it('resumes collapsing after scroll lock timeout', () => {
145+
const {container, getByText} = renderInEntry(
146+
<DefaultNavigationPresenceProvider configuration={{}}>
147+
<LockNavButton />
148+
</DefaultNavigationPresenceProvider>
149+
);
150+
151+
act(() => {
152+
getByText('Lock').click();
153+
});
154+
155+
act(() => {
156+
jest.advanceTimersByTime(200);
157+
});
98158

99-
act(() => {
100-
window.scrollY = 50;
101-
window.dispatchEvent(new Event('scroll'));
159+
act(() => {
160+
window.scrollY = 100;
161+
window.dispatchEvent(new Event('scroll'));
162+
});
163+
164+
expect(container.firstChild).not.toHaveClass(styles.expanded);
102165
});
103166

104-
expect(container.firstChild).toHaveClass(styles.expanded);
167+
it('re-expands nav when lockNavExpanded is called while collapsed', () => {
168+
const {container, getByText} = renderInEntry(
169+
<DefaultNavigationPresenceProvider configuration={{}}>
170+
<LockNavButton />
171+
</DefaultNavigationPresenceProvider>
172+
);
173+
174+
act(() => {
175+
window.scrollY = 100;
176+
window.dispatchEvent(new Event('scroll'));
177+
});
178+
179+
expect(container.firstChild).not.toHaveClass(styles.expanded);
180+
181+
act(() => {
182+
getByText('Lock').click();
183+
});
184+
185+
expect(container.firstChild).toHaveClass(styles.expanded);
186+
});
105187
});
106188
});

entry_types/scrolled/package/src/widgets/defaultNavigation/DefaultNavigation.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export function DefaultNavigation({
3434
logo,
3535
omitChapterNavigation
3636
}) {
37-
const {navExpanded, setNavExpanded} = useDefaultNavigationState();
37+
const {navExpanded, setNavExpanded, lockNavExpanded} = useDefaultNavigationState();
3838
const [menuOpen, setMenuOpen] = useState(!!configuration.defaultMobileNavVisible);
3939
const [readingProgress, setReadingProgress] = useState(0);
4040

@@ -78,6 +78,7 @@ export function DefaultNavigation({
7878
};
7979

8080
function handleMenuClick(chapterLinkId) {
81+
lockNavExpanded();
8182
setMenuOpen(false);
8283
};
8384

entry_types/scrolled/package/src/widgets/defaultNavigation/DefaultNavigationPresenceProvider.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
usePhonePlatform
88
} from 'pageflow-scrolled/frontend';
99

10+
import {useTimeoutFlag} from './useTimeoutFlag';
1011
import styles from './presenceClassNames.module.css';
1112

1213
const DefaultNavigationContext = createContext({
@@ -21,9 +22,17 @@ export function useDefaultNavigationState() {
2122
export function DefaultNavigationPresenceProvider({configuration, children}) {
2223
const [navExpanded, setNavExpanded] = useState(true);
2324
const isPhonePlatform = usePhonePlatform();
25+
const [scrollLockRef, activateScrollLock] = useTimeoutFlag(200);
26+
27+
const lockNavExpanded = useCallback(() => {
28+
activateScrollLock();
29+
setNavExpanded(true);
30+
}, [activateScrollLock]);
2431

2532
useScrollPosition(
2633
({prevPos, currPos}) => {
34+
if (scrollLockRef.current) return;
35+
2736
const expand = currPos.y > prevPos.y ||
2837
// Mobile Safari reports positive scroll position
2938
// during scroll bounce animation when scrolling
@@ -51,8 +60,8 @@ export function DefaultNavigationPresenceProvider({configuration, children}) {
5160
);
5261

5362
const contextValue = useMemo(
54-
() => ({navExpanded, setNavExpanded}),
55-
[navExpanded]
63+
() => ({navExpanded, setNavExpanded, lockNavExpanded}),
64+
[navExpanded, lockNavExpanded]
5665
);
5766

5867
return (
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import {useCallback, useEffect, useRef} from 'react';
2+
3+
export function useTimeoutFlag(duration) {
4+
const flagRef = useRef(false);
5+
const timeoutRef = useRef(null);
6+
7+
const activate = useCallback(() => {
8+
flagRef.current = true;
9+
clearTimeout(timeoutRef.current);
10+
timeoutRef.current = setTimeout(() => {
11+
flagRef.current = false;
12+
}, duration);
13+
}, [duration]);
14+
15+
useEffect(() => {
16+
return () => clearTimeout(timeoutRef.current);
17+
}, []);
18+
19+
return [flagRef, activate];
20+
}

0 commit comments

Comments
 (0)