Skip to content
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

[Menubar] Adds keyboard behaviors to the menubar, improves accessibility #3320

Open
wants to merge 83 commits into
base: develop
Choose a base branch
from
Open
Changes from 1 commit
Commits
Show all changes
83 commits
Select commit Hold shift + click to select a range
0164594
feat: added context values for keyboard navigation
tespin Jan 16, 2025
656b740
feat: top-level keyboard navigation added
tespin Jan 16, 2025
ec66297
feat: implemented ref forwarding
tespin Jan 17, 2025
66b57a6
feat: added submenu state variables
tespin Jan 17, 2025
dfb447d
feat: added id to MenubarItems for tracking
tespin Jan 17, 2025
49e3ddb
feat: updated with relevant submenu states and key handlers
tespin Jan 17, 2025
a300c52
feat: menubaritem registers self and determines if active or not
tespin Jan 17, 2025
93f963d
feat: base implementation of keyboard-navigable submenus
tespin Jan 17, 2025
ef6a71c
chore: code cleanup
tespin Jan 17, 2025
c636ccf
refactor: moved logo outside menubar for proper keyboard navigation
tespin Jan 17, 2025
84202ce
fix: fixed inconsistent hook rendering in MenubarSubmenu
tespin Jan 17, 2025
9fb7b9e
fix: fixed useEffect getting a boolean instead of dependency array
tespin Jan 17, 2025
ce789d7
refactor: renamed variables in preparation of state management refactor
tespin Jan 18, 2025
63a6444
feat: added usePrevious hook for obtaining prev state value
tespin Jan 18, 2025
dc62a3b
refactor: migrated menu item collections from Array to Set
tespin Jan 18, 2025
d3ccf16
feat: added usePrevious hook for focus management
tespin Jan 22, 2025
f3555dc
refactor: migrated collections tracking for top level components from…
tespin Jan 22, 2025
f7f8438
fix: do not add menuitems to top level collection
tespin Jan 22, 2025
d41bd40
chore: code cleanup
tespin Jan 22, 2025
05f779c
refactor: adding menuitems to submenu collections set
tespin Jan 22, 2025
84edb18
refactor: menu tracking updated to use submenu id obtained from map l…
tespin Jan 22, 2025
83f2233
chore: cleanup and removing array system
tespin Jan 22, 2025
e012bf3
feat: submenus correctly focus first item when pressing the down arro…
tespin Jan 22, 2025
a6d9234
chore: removed debugging logs and unused comments
tespin Jan 22, 2025
ff3f866
chore: removed old array system for tracking menu items
tespin Jan 22, 2025
4c7414d
feat: added last() helper function for focusing last item
tespin Jan 22, 2025
dae30ba
feat: Menubar closes prev submenu and opens new one on arrow left or …
tespin Jan 22, 2025
fe673d4
fix: submenu wraps around correctly when index is reset to -1
tespin Jan 22, 2025
e5aff3b
feat: escape closes submenus and returns focus to previously focused …
tespin Jan 22, 2025
b28eb54
feat: pressing space opens the submenu if closed and activates focuse…
tespin Jan 23, 2025
4543c71
fix: submenu handles Enter event so no need for Menubar to handle it
tespin Jan 23, 2025
5eaadf0
chore: removed old state management variables
tespin Jan 23, 2025
a55cc1e
chore: removing unused ones from MenubarContext as well
tespin Jan 23, 2025
584dc92
feat: tabbing out of submenu closes submenu and moves focus outside o…
tespin Jan 23, 2025
6b95211
fix: explicitly added tabIndex to submenu items
tespin Jan 23, 2025
8ef96ea
feat: space has same functionality as enter within submenus
tespin Jan 23, 2025
626569a
feat: mouse hover states are synced with active menu indices
tespin Jan 23, 2025
bae193d
feat: base styles added for keyboard traversal and focus management
tespin Jan 23, 2025
2576bac
refactor: adding state to manage focus events when outside menubar
tespin Jan 23, 2025
0bcd0a7
fix: index updated appropriately when clicking on a submenu
tespin Jan 23, 2025
c582069
fix: focus styles for mouse events showing properly
tespin Jan 24, 2025
9f37e63
fix: added role and tabIndex to address lint errors
tespin Jan 24, 2025
8288ef6
fix: loosened the check for firing off the menubar key handlers
tespin Jan 25, 2025
91744a7
fix: let submenu handle enter and space events when a trigger is focused
tespin Jan 25, 2025
23825f9
fix: moved early return to top to fix rules of hooks error
tespin Jan 25, 2025
c099ab9
fix: improved HTML structure
tespin Jan 25, 2025
e55e427
fix: prevent submenus from closing prematurely during keyboard naviga…
tespin Jan 25, 2025
d78850c
fix: prevent focusing menubar container when focus moves into component
tespin Jan 25, 2025
861b73b
chore: code cleanup
tespin Jan 25, 2025
048ec3e
chore: code cleanup
tespin Jan 25, 2025
66c3042
refactor: extracted key handlers into helper functions
tespin Jan 26, 2025
8027b48
fix: submenu wraps correctly when going backwards
tespin Jan 26, 2025
20a2be1
fix: submenuActiveIndex resets correctly when switching submenus
tespin Jan 26, 2025
ad6b912
chore: code cleanup
tespin Jan 26, 2025
5c803fc
fix: tabbing within a submenu should not prevent default
tespin Jan 26, 2025
49e85b9
turn about component into a page
raclim Nov 7, 2024
72a0946
update routes and imports
raclim Nov 7, 2024
307ba54
update styling and translations
raclim Nov 7, 2024
ee1cacd
add in translations and logo
raclim Nov 12, 2024
327c596
remove SASS
raclim Dec 6, 2024
f5f4686
add styled components and rest of new content
raclim Dec 6, 2024
1aade22
adjust padding
raclim Dec 12, 2024
5656fbf
add theming and heart icon
raclim Jan 11, 2025
3aaddce
refactor: move about into new module, separate AboutSection
raclim Jan 16, 2025
8ede947
refactor: separate styles into About.styles
raclim Jan 16, 2025
a7e7612
refactor: separate about page content into static file
raclim Jan 16, 2025
ecd4aa4
fix: add missing translation for Get Involved section
raclim Jan 16, 2025
47e188d
refactor: update class names, add hyperlink to email address
raclim Jan 16, 2025
4a766e3
refactor: update translation title for about headline
raclim Jan 16, 2025
9ca1ece
refactor: adjust spacing and syntax in style file
raclim Jan 17, 2025
de2c80a
fix: use margin instead of padding for footer
raclim Jan 17, 2025
f728347
2.15.7
raclim Jan 23, 2025
c3713d2
docs: added jsdocs to menubar components
tespin Jan 31, 2025
86bd0bf
test: added unit test for Menubar and subcomponents
tespin Jan 31, 2025
8be2676
fix: add aria-label to submenus
tespin Jan 31, 2025
171021b
fix: disabled prop-types validation for mocked components in nav test
tespin Feb 1, 2025
da60d54
test: updated snapshots
tespin Feb 1, 2025
cb8001f
chore: fixed lint warnings and small optimizations
tespin Feb 1, 2025
9e13d79
Merge branch 'develop' into refactor/menubar-a11y-keyboard-mouse
tespin Feb 1, 2025
fd425ae
chore: fixed lint warning
tespin Feb 1, 2025
866f6fa
fix: removed 'title' from MenubarList proptypes
tespin Feb 6, 2025
9fad82d
Merge branch 'develop' into refactor/menubar-a11y-keyboard-mouse
tespin Feb 6, 2025
8a83fd3
Merge branch 'develop' into refactor/menubar-a11y-keyboard-mouse
raclim Feb 17, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: code cleanup
tespin committed Jan 25, 2025
commit 048ec3ecd597381592dc73e12cf295bffec467b5
65 changes: 32 additions & 33 deletions client/components/Menubar/Menubar.jsx
Original file line number Diff line number Diff line change
@@ -12,13 +12,12 @@ import usePrevious from '../../common/usePrevious';

function Menubar({ children, className }) {
const [menuOpen, setMenuOpen] = useState('none');

const menuItems = useRef(new Set()).current;
const [activeIndex, setActiveIndex] = useState(0);
const [hasFocus, setHasFocus] = useState(false);
const prevIndex = usePrevious(activeIndex);
const [isFirstChild, setIsFirstChild] = useState(false);
const menuItems = useRef(new Set()).current;
const menuItemToId = useRef(new Map()).current;
const prevIndex = usePrevious(activeIndex);

const timerRef = useRef(null);

@@ -52,6 +51,35 @@ function Menubar({ children, className }) {
setMenuOpen((prevState) => (prevState === id ? 'none' : id));
});

const clearHideTimeout = useCallback(() => {
if (timerRef.current) {
clearTimeout(timerRef.current);
timerRef.current = null;
}
}, [timerRef]);

const handleClose = useCallback(() => {
clearHideTimeout();
setMenuOpen('none');
}, [setMenuOpen]);

const nodeRef = useModalClose(handleClose);

const handleFocus = useCallback(() => {
setHasFocus(true);
}, []);

const handleBlur = useCallback(() => {
const isInMenu = nodeRef.current?.contains(document.activeElement);

if (!isInMenu) {
timerRef.current = setTimeout(() => {
setMenuOpen('none');
setHasFocus(false);
}, 10);
}
}, [nodeRef]);

const keyHandlers = useMemo(
() => ({
ArrowLeft: (e) => {
@@ -99,35 +127,6 @@ function Menubar({ children, className }) {
[menuItems, activeIndex, menuOpen, toggleMenuOpen]
);

const clearHideTimeout = useCallback(() => {
if (timerRef.current) {
clearTimeout(timerRef.current);
timerRef.current = null;
}
}, [timerRef]);

const handleClose = useCallback(() => {
clearHideTimeout();
setMenuOpen('none');
}, [setMenuOpen]);

const nodeRef = useModalClose(handleClose);

const handleFocus = useCallback(() => {
setHasFocus(true);
}, []);

const handleBlur = useCallback(() => {
const isInMenu = nodeRef.current?.contains(document.activeElement);

if (!isInMenu) {
timerRef.current = setTimeout(() => {
setMenuOpen('none');
setHasFocus(false);
}, 10);
}
}, [nodeRef]);

useEffect(() => {
if (activeIndex !== prevIndex) {
const items = Array.from(menuItems);
@@ -201,8 +200,8 @@ function Menubar({ children, className }) {
<MenubarContext.Provider value={contextValue}>
<ul
className={className}
ref={nodeRef}
role="menubar"
ref={nodeRef}
aria-orientation="horizontal"
onFocus={handleFocus}
onKeyDown={(e) => {
15 changes: 7 additions & 8 deletions client/components/Menubar/MenubarItem.jsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import PropTypes from 'prop-types';
import React, { useState, useEffect, useContext, useRef, useMemo } from 'react';
import ButtonOrLink from '../../common/ButtonOrLink';
import React, { useEffect, useContext, useRef, useMemo } from 'react';
import { MenubarContext, SubmenuContext, ParentMenuContext } from './contexts';
import ButtonOrLink from '../../common/ButtonOrLink';

function MenubarItem({
id,
hideIf,
className,
id,
role: customRole,
hideIf,
selected,
...rest
}) {
@@ -17,15 +17,14 @@ function MenubarItem({
}

const { createMenuItemHandlers, hasFocus } = useContext(MenubarContext);

const {
setSubmenuActiveIndex,
submenuItems,
registerSubmenuItem,
isFirstChild,
setSubmenuActiveIndex
isFirstChild
} = useContext(SubmenuContext);
const menuItemRef = useRef(null);
const parent = useContext(ParentMenuContext);
const menuItemRef = useRef(null);

const handlers = useMemo(() => createMenuItemHandlers(parent), [
createMenuItemHandlers,
104 changes: 51 additions & 53 deletions client/components/Menubar/MenubarSubmenu.jsx
Original file line number Diff line number Diff line change
@@ -5,18 +5,18 @@ import PropTypes from 'prop-types';
import React, {
useState,
useEffect,
useCallback,
useContext,
useCallback,
useRef,
useMemo
} from 'react';
import TriangleIcon from '../../images/down-filled-triangle.svg';
import {
MenuOpenContext,
MenubarContext,
SubmenuContext,
ParentMenuContext
} from './contexts';
import TriangleIcon from '../../images/down-filled-triangle.svg';

export function useMenuProps(id) {
const activeMenu = useContext(MenuOpenContext);
@@ -37,20 +37,19 @@ export function useMenuProps(id) {
* MenubarTrigger
* -----------------------------------------------------------------------------------------------*/
const MenubarTrigger = React.forwardRef(
({ id, title, role, hasPopup, ...props }, ref) => {
({ id, role, title, hasPopup, ...props }, ref) => {
const { isOpen, handlers } = useMenuProps(id);
const {
setActiveIndex,
menuItems,
registerTopLevelItem,
toggleMenuOpen,
setActiveIndex,
hasFocus
hasFocus,
toggleMenuOpen
} = useContext(MenubarContext);
const { first, last } = useContext(SubmenuContext);

const handleMouseEnter = () => {
if (hasFocus) {
console.log('hasFocus', hasFocus);
const items = Array.from(menuItems);
const index = items.findIndex((item) => item === ref.current);

@@ -99,9 +98,9 @@ const MenubarTrigger = React.forwardRef(

return (
<button
ref={ref}
{...handlers}
{...props}
{...handlers}
ref={ref}
role={role}
onMouseEnter={handleMouseEnter}
onKeyDown={handleKeyDown}
@@ -121,8 +120,8 @@ const MenubarTrigger = React.forwardRef(

MenubarTrigger.propTypes = {
id: PropTypes.string.isRequired,
title: PropTypes.node.isRequired,
role: PropTypes.string,
title: PropTypes.node.isRequired,
hasPopup: PropTypes.oneOf(['menu', 'listbox', 'true'])
};

@@ -135,7 +134,7 @@ MenubarTrigger.defaultProps = {
* MenubarList
* -----------------------------------------------------------------------------------------------*/

function MenubarList({ id, children, role, ...props }) {
function MenubarList({ children, id, role, ...props }) {
return (
<ul className="nav__dropdown" role={role} {...props}>
<ParentMenuContext.Provider value={id}>
@@ -161,18 +160,18 @@ MenubarList.defaultProps = {
* -----------------------------------------------------------------------------------------------*/

function MenubarSubmenu({
children,
id,
title,
children,
triggerRole: customTriggerRole,
listRole: customListRole,
...props
}) {
const { isOpen, handlers } = useMenuProps(id);
const { toggleMenuOpen } = useContext(MenubarContext);
const submenuItems = useRef(new Set()).current;
const [submenuActiveIndex, setSubmenuActiveIndex] = useState(0);
const [isFirstChild, setIsFirstChild] = useState(false);
const { toggleMenuOpen } = useContext(MenubarContext);
const submenuItems = useRef(new Set()).current;

const buttonRef = useRef(null);
const listItemRef = useRef(null);
@@ -193,6 +192,25 @@ function MenubarSubmenu({
}
}, [submenuItems]);

const registerSubmenuItem = useCallback(
(ref) => {
const submenuItemNode = ref.current;

if (submenuItemNode) {
if (submenuItems.size === 0) {
setIsFirstChild(true);
}

submenuItems.add(submenuItemNode);
}

return () => {
submenuItems.delete(submenuItemNode);
};
},
[submenuItems]
);

const keyHandlers = useMemo(
() => ({
ArrowUp: (e) => {
@@ -211,7 +229,6 @@ function MenubarSubmenu({
e.stopPropagation();
e.preventDefault();

setSubmenuActiveIndex((prev) => (prev + 1) % submenuItems.size);
const newIndex =
submenuActiveIndex === submenuItems.size - 1
? 0
@@ -297,24 +314,20 @@ function MenubarSubmenu({
[isOpen, keyHandlers]
);

const registerSubmenuItem = useCallback(
(ref) => {
const submenuItemNode = ref.current;

if (submenuItemNode) {
if (submenuItems.size === 0) {
setIsFirstChild(true);
}

submenuItems.add(submenuItemNode);
}
// reset submenu active index when submenu is closed
useEffect(() => {
if (!isOpen) {
setSubmenuActiveIndex(-1);
}
}, [isOpen]);

return () => {
submenuItems.delete(submenuItemNode);
};
},
[submenuItems]
);
useEffect(() => {
const el = listItemRef.current;
if (el) {
el.addEventListener('keydown', handleKeyDown);
}
return () => el.removeEventListener('keydown', handleKeyDown);
}, [handleKeyDown]);

useEffect(() => {
if (isOpen && submenuItems.size > 0) {
@@ -332,37 +345,22 @@ function MenubarSubmenu({
}
}, [isOpen, submenuItems, submenuActiveIndex]);

// reset submenu active index when submenu is closed
useEffect(() => {
if (!isOpen) {
setSubmenuActiveIndex(-1);
}
}, [isOpen]);

useEffect(() => {
const el = listItemRef.current;
if (el) {
el.addEventListener('keydown', handleKeyDown);
}
return () => el.removeEventListener('keydown', handleKeyDown);
}, [handleKeyDown]);

const submenuContext = useMemo(
() => ({
submenuItems,
submenuActiveIndex,
setSubmenuActiveIndex,
registerSubmenuItem,
isFirstChild,
first,
last,
setSubmenuActiveIndex,
submenuActiveIndex
last
}),
[
submenuItems,
submenuActiveIndex,
setSubmenuActiveIndex,
registerSubmenuItem,
isFirstChild,
setSubmenuActiveIndex,
submenuActiveIndex,
first,
last
]
@@ -394,8 +392,8 @@ function MenubarSubmenu({

MenubarSubmenu.propTypes = {
id: PropTypes.string.isRequired,
title: PropTypes.node.isRequired,
children: PropTypes.node,
title: PropTypes.node.isRequired,
triggerRole: PropTypes.string,
listRole: PropTypes.string
};