-
Notifications
You must be signed in to change notification settings - Fork 267
SPIKE: Ws 2118 investigate sticky navugation on scroll up #13760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: latest
Are you sure you want to change the base?
Changes from all commits
559d586
57062b0
ced2845
9e983b5
bdc1009
dd75236
e034c97
44774b6
7ff7d51
45942b9
12f76d7
6b379df
6bba1d9
b033e74
a3804ae
768abff
5def85b
7a47579
be0bf76
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,94 @@ | ||
| import CanonicalNavigationContainer from './index.canonical'; | ||
| import { | ||
| render, | ||
| screen, | ||
| fireEvent, | ||
| } from '../react-testing-library-with-providers'; | ||
|
|
||
| describe('Navigation - Canonical', () => { | ||
| it('should render', () => { | ||
| expect(true).toBeTruthy(); | ||
| }); | ||
|
|
||
| describe('CanonicalNavigationContainer sticky nav', () => { | ||
| const topScrollableListItems = ( | ||
| <ul> | ||
| <li>Top Item</li> | ||
| </ul> | ||
| ); | ||
| const bottomScrollableListItems = ( | ||
| <ul> | ||
| <li>Bottom Item</li> | ||
| </ul> | ||
| ); | ||
| const dropdownListItems = ( | ||
| <ul> | ||
| <li>Dropdown Item</li> | ||
| </ul> | ||
| ); | ||
| const menuAnnouncedText = 'Menu'; | ||
| const dir = 'ltr'; | ||
|
|
||
| it('renders sticky nav container but hides it by default', () => { | ||
| render( | ||
| <CanonicalNavigationContainer | ||
| dir={dir} | ||
| menuAnnouncedText={menuAnnouncedText} | ||
| topScrollableListItems={topScrollableListItems} | ||
| bottomScrollableListItems={bottomScrollableListItems} | ||
| dropdownListItems={dropdownListItems} | ||
| />, | ||
| ); | ||
| const stickyNav = screen.queryByLabelText('Sticky navigation'); | ||
| expect(stickyNav).toBeInTheDocument(); | ||
| expect(stickyNav).toHaveAttribute('aria-hidden', 'true'); | ||
| expect(stickyNav).toHaveStyle('transform: translateY(-100%)'); | ||
| }); | ||
|
|
||
| it('does not render sticky nav on lite site', () => { | ||
| render( | ||
| <CanonicalNavigationContainer | ||
| dir={dir} | ||
| menuAnnouncedText={menuAnnouncedText} | ||
| topScrollableListItems={topScrollableListItems} | ||
| bottomScrollableListItems={bottomScrollableListItems} | ||
| dropdownListItems={dropdownListItems} | ||
| />, | ||
| { isLite: true }, | ||
| ); | ||
| const stickyNav = screen.queryByLabelText('Sticky navigation'); | ||
| expect(stickyNav).toBeNull(); | ||
| }); | ||
|
|
||
| it('hides sticky nav when keyboard navigation is detected', () => { | ||
| render( | ||
| <CanonicalNavigationContainer | ||
| dir={dir} | ||
| menuAnnouncedText={menuAnnouncedText} | ||
| topScrollableListItems={topScrollableListItems} | ||
| bottomScrollableListItems={bottomScrollableListItems} | ||
| dropdownListItems={dropdownListItems} | ||
| />, | ||
| ); | ||
| fireEvent.keyDown(window, { key: 'Tab' }); | ||
| const stickyNav = screen.queryByLabelText('Sticky navigation'); | ||
| expect(stickyNav).toBeNull(); | ||
| }); | ||
|
|
||
| it('shows sticky nav again after pointer interaction', () => { | ||
| render( | ||
| <CanonicalNavigationContainer | ||
| dir={dir} | ||
| menuAnnouncedText={menuAnnouncedText} | ||
| topScrollableListItems={topScrollableListItems} | ||
| bottomScrollableListItems={bottomScrollableListItems} | ||
| dropdownListItems={dropdownListItems} | ||
| />, | ||
| ); | ||
| fireEvent.keyDown(window, { key: 'Tab' }); | ||
| fireEvent.mouseDown(window); | ||
| const stickyNav = screen.queryByLabelText('Sticky navigation'); | ||
| expect(stickyNav).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import React, { useState, use } from 'react'; | ||
| import React, { useState, useRef, useEffect, use } from 'react'; | ||
| import { css } from '@emotion/react'; | ||
| import Navigation from '#psammead/psammead-navigation/src'; | ||
| import { ScrollableNavigation } from '#psammead/psammead-navigation/src/ScrollableNavigation'; | ||
| import { | ||
|
|
@@ -35,16 +36,85 @@ const CanonicalNavigationContainer: React.FC< | |
| }) => { | ||
| const { isLite } = use(RequestContext); | ||
| const { enabled: topBarOJsEnabled } = useToggle('topBarOJs'); | ||
| const [isOpen, setIsOpen] = useState(false); | ||
| // Track which nav's dropdown is open: 'main', 'sticky', or null | ||
| const [openDropdown, setOpenDropdown] = useState<null | 'main' | 'sticky'>( | ||
| null, | ||
| ); | ||
| const [showSticky, setShowSticky] = useState(false); | ||
| // Refs and state for sticky nav | ||
| const [lastScrollY, setLastScrollY] = useState(0); | ||
| const [isKeyboardNav, setIsKeyboardNav] = useState(false); // this is to prevent showing sticky nav when keyboard navigation is detected | ||
| const navRef = useRef<HTMLDivElement>(null); | ||
| const stickyNavRef = useRef<HTMLDivElement>(null); | ||
|
|
||
| useMediaQuery(`(max-width: ${GROUP_2_MAX_WIDTH_BP}rem)`, event => { | ||
| if (!event.matches) { | ||
| setIsOpen(false); | ||
| setOpenDropdown(null); | ||
| } | ||
| }); | ||
|
|
||
| return ( | ||
| <Navigation dir={dir} isOpen={isOpen}> | ||
| // Sticky nav scroll logic | ||
| useEffect(() => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possible performance risk here in that we update the state on scroll, so the nav could keep re-rendering while the user scrolls, which may be expensive w/ regards to mobile data considerations |
||
| let ticking = false; | ||
| const handleScroll = () => { | ||
| if (!navRef.current) return; | ||
| if (!ticking) { | ||
| window.requestAnimationFrame(() => { | ||
| const navElement = navRef.current; | ||
| if (!navElement) return; | ||
| const mainNavBar = navElement.getBoundingClientRect(); | ||
| const { scrollY } = window; | ||
| const scrollingUp = scrollY < lastScrollY; // detects scroll direction | ||
| setLastScrollY(scrollY); | ||
| // Only show sticky nav if nav is fully out of view and user is scrolling up | ||
| // Hide sticky nav before original nav is visible (with threshold) | ||
| const threshold = 70; // px, adjust for not seeing both original and sticky nav at the same time | ||
| if (mainNavBar.bottom < -threshold && scrollingUp && !isKeyboardNav) { | ||
| // do not show sticky nav if keyboard navigation is detected | ||
| setShowSticky(true); | ||
| } else { | ||
| setShowSticky(false); | ||
| setOpenDropdown(null); // Close dropdown when sticky nav hides | ||
| } | ||
| ticking = false; // the ticking flag is used to prevent multiple requestAnimationFrame calls from stacking up and causing performance issues during fast scrolling. | ||
| }); | ||
| ticking = true; | ||
| } | ||
| }; | ||
| window.addEventListener('scroll', handleScroll, { passive: true }); | ||
| return () => window.removeEventListener('scroll', handleScroll); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [lastScrollY, isKeyboardNav]); | ||
|
|
||
| // Keyboard navigation detection | ||
| // do we need to add other keys? | ||
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if (e.key === 'Tab') { | ||
| setIsKeyboardNav(true); | ||
| } | ||
| }; | ||
| const handlePointer = () => { | ||
| setIsKeyboardNav(false); | ||
| }; | ||
| window.addEventListener('keydown', handleKeyDown); | ||
| window.addEventListener('mousedown', handlePointer); | ||
| window.addEventListener('touchstart', handlePointer); | ||
| return () => { | ||
| window.removeEventListener('keydown', handleKeyDown); | ||
| window.removeEventListener('mousedown', handlePointer); | ||
| window.removeEventListener('touchstart', handlePointer); | ||
| }; | ||
| }, []); | ||
|
|
||
| // Main nav (normal) | ||
| const mainNav = ( | ||
| <Navigation | ||
| dir={dir} | ||
| isOpen={openDropdown === 'main'} | ||
| ref={navRef} | ||
| role="navigation" | ||
| > | ||
| <div css={styles.navStack}> | ||
| <div style={{ position: 'relative', width: '100%' }}> | ||
| <div css={styles.topRow}> | ||
|
|
@@ -59,13 +129,18 @@ const CanonicalNavigationContainer: React.FC< | |
| <CanonicalMenuButton | ||
| css={styles.menuButton} | ||
| announcedText={menuAnnouncedText} | ||
| isOpen={isOpen} | ||
| onClick={() => setIsOpen(!isOpen)} | ||
| isOpen={openDropdown === 'main'} | ||
| onClick={() => | ||
| setOpenDropdown(openDropdown === 'main' ? null : 'main') | ||
| } | ||
| dir={dir} | ||
| /> | ||
| )} | ||
| </div> | ||
| <CanonicalDropdown isOpen={isOpen} css={styles.dropdown}> | ||
| <CanonicalDropdown | ||
| isOpen={openDropdown === 'main'} | ||
| css={styles.dropdown} | ||
| > | ||
| {dropdownListItems} | ||
| </CanonicalDropdown> | ||
| </div> | ||
|
|
@@ -83,6 +158,90 @@ const CanonicalNavigationContainer: React.FC< | |
| {topBarOJsEnabled && <TopBarOJs blocks={blocks ?? []} />} | ||
| </Navigation> | ||
| ); | ||
|
|
||
| // Sticky nav is rendered if it is not the lite site and if keyboard navigation has not been indicated with te tab key | ||
| const stickyNav = | ||
| !isLite && !isKeyboardNav ? ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth considering for this spike but I think the sticky nav is still mounted even when hidden, so keyboard users may still tab into off-screen links and we also end up duplicating the navigation markup
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't seem possible to tab into off-screen links when I test it. aria-hidden="true" makes it not focusable. If duplication of markup is a problem, a different option can be looked into. |
||
| <div | ||
| ref={stickyNavRef} | ||
| css={css` | ||
| position: fixed; | ||
| top: 0; | ||
| left: 0; | ||
| width: 100%; | ||
| z-index: 10000; | ||
| background: inherit; | ||
| box-shadow: 0 2px 8px rgba(0, 0, 0, 0.08); | ||
| pointer-events: ${showSticky ? 'auto' : 'none'}; | ||
| transform: ${showSticky ? 'translateY(0)' : 'translateY(-100%)'}; | ||
| transition: transform 0.4s cubic-bezier(0.4, 0, 0.2, 1); | ||
| @media (prefers-reduced-motion: reduce) { | ||
| transition: none; | ||
| } | ||
| `} | ||
| aria-label="Sticky navigation" | ||
| aria-hidden="true" | ||
| > | ||
| <Navigation | ||
| dir={dir} | ||
| isOpen={openDropdown === 'sticky'} | ||
| role="navigation" | ||
| > | ||
| <div css={styles.navStack}> | ||
| <div style={{ position: 'relative', width: '100%' }}> | ||
| <div css={styles.topRow}> | ||
| <ScrollableNavigation | ||
| dir={dir} | ||
| css={styles.topRowItems} | ||
| navPosition="primary" | ||
| isSticky | ||
| > | ||
| {topScrollableListItems} | ||
| </ScrollableNavigation> | ||
| {!isLite && ( | ||
| <CanonicalMenuButton | ||
| css={styles.menuButton} | ||
| announcedText={menuAnnouncedText} | ||
| isOpen={openDropdown === 'sticky'} | ||
| onClick={() => | ||
| setOpenDropdown( | ||
| openDropdown === 'sticky' ? null : 'sticky', | ||
| ) | ||
| } | ||
| dir={dir} | ||
| /> | ||
| )} | ||
| </div> | ||
| <CanonicalDropdown | ||
| isOpen={openDropdown === 'sticky'} | ||
| css={styles.dropdown} | ||
| isSticky | ||
| > | ||
| {dropdownListItems} | ||
| </CanonicalDropdown> | ||
| </div> | ||
| <div css={styles.lowerNavWrapper}> | ||
| <ScrollableNavigation | ||
| dir={dir} | ||
| css={styles.bottomRowItems} | ||
| navPosition="secondary" | ||
| isSticky | ||
| > | ||
| {bottomScrollableListItems} | ||
| </ScrollableNavigation> | ||
| </div> | ||
| </div> | ||
| <div css={styles.bottomDivider} /> | ||
| </Navigation> | ||
| </div> | ||
| ) : null; | ||
|
|
||
| return ( | ||
| <> | ||
| {mainNav} | ||
| {stickyNav} | ||
| </> | ||
| ); | ||
| }; | ||
|
|
||
| export default CanonicalNavigationContainer; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More for L39 but using one shared open/closed state for both navs might mean opening the sticky menu also opens the hidden original menu, which could lead to odd layout shifts or unexpected behaviour
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, this was actually causing a bug with the dropdown. I have added
const [openDropdown, setOpenDropdown] = useState<null | 'main' | 'sticky'>( null, );which should make it so only one dropdown menu can be open at a time.