feat(menu): accessibility improvements to menu and menu item components

components/menu/src/flyout-menu/flyout-menu.js

@@ -1,6 +1,13 @@
 import { colors, elevations, spacers } from '@dhis2/ui-constants'
 import PropTypes from 'prop-types'
-import React, { Children, cloneElement, isValidElement, useState } from 'react'
+import React, {
+    Children,
+    cloneElement,
+    isValidElement,
+    useEffect,
+    useRef,
+    useState,
+} from 'react'
 import { Menu } from '../index.js'
 
 const FlyoutMenu = ({
@@ -16,9 +23,16 @@ const FlyoutMenu = ({
         const toggleValue = index === openedSubMenu ? null : index
         setOpenedSubMenu(toggleValue)
     }
+    const ref = useRef(null)
+
+    useEffect(() => {
+        if (ref.current && ref.current?.children[0]) {
+            ref.current.children[0].focus()
+        }
+    }, [])
 
     return (
-        <div className={className} data-test={dataTest}>
+        <div className={className} data-test={dataTest} ref={ref}>
             <Menu dense={dense}>
                 {Children.map(children, (child, index) =>
                     isValidElement(child)

components/menu/src/menu-divider/menu-divider.js

@@ -4,7 +4,7 @@ import PropTypes from 'prop-types'
 import React from 'react'
 
 const MenuDivider = ({ className, dataTest, dense }) => (
-    <li className={className} data-test={dataTest}>
+    <li className={className} data-test={dataTest} role="separator">
         <Divider dense={dense} />
 
         <style jsx>{`

components/menu/src/menu-item/__tests__/menu-item.test.js

@@ -0,0 +1,54 @@
+import { mount } from 'enzyme'
+import React from 'react'
+import { MenuItem } from '../menu-item.js'
+
+describe('Menu Component', () => {
+    it('Default menu item has role', () => {
+        const menuItemDataTest = 'data-test-menu-item'
+        const wrapper = mount(
+            <MenuItem dataTest={menuItemDataTest} label="Menu item" />
+        )
+        const menuItem = wrapper.find({ 'data-test': menuItemDataTest })
+        expect(menuItem.childAt(0).prop('role')).toBe('menuitem')
+        expect(menuItem.childAt(0).prop('aria-disabled')).toBe(undefined)
+        expect(menuItem.childAt(0).prop('aria-label')).toBe('Menu item')
+    })
+
+    it('Disabled menu item has aria-disabled attribute', () => {
+        const menuItemDataTest = 'data-test-menu-item'
+        const wrapper = mount(
+            <MenuItem
+                dataTest={menuItemDataTest}
+                label="Disabled menu item"
+                disabled={true}
+            />
+        )
+        const menuItem = wrapper.find({ 'data-test': menuItemDataTest })
+
+        expect(menuItem.childAt(0).prop('role')).toBe('menuitem')
+        expect(menuItem.childAt(0).prop('aria-disabled')).toBe(true)
+        expect(menuItem.childAt(0).prop('aria-label')).toBe(
+            'Disabled menu item'
+        )
+    })
+
+    it('Toggle-able menu item has menuitemcheckbox role', () => {
+        const menuItemDataTest = 'data-test-menu-item'
+        const wrapper = mount(
+            <MenuItem
+                dataTest={menuItemDataTest}
+                label="Toggle-able menu item"
+                checkbox={true}
+                checked={false}
+            />
+        )
+        const menuItem = wrapper.find({ 'data-test': menuItemDataTest })
+
+        expect(menuItem.childAt(0).prop('role')).not.toBe('menuitem')
+        expect(menuItem.childAt(0).prop('role')).toBe('menuitemcheckbox')
+        expect(menuItem.childAt(0).prop('aria-checked')).toBe(false)
+        expect(menuItem.childAt(0).prop('aria-label')).toBe(
+            'Toggle-able menu item'
+        )
+    })
+})

components/menu/src/menu-item/menu-item.js

@@ -3,7 +3,7 @@ import { Portal } from '@dhis2-ui/portal'
 import { IconChevronRight24 } from '@dhis2/ui-icons'
 import cx from 'classnames'
 import PropTypes from 'prop-types'
-import React, { useRef } from 'react'
+import React, { forwardRef, useCallback, useRef, useState } from 'react'
 import { FlyoutMenu } from '../index.js'
 import styles from './menu-item.styles.js'
 
@@ -22,26 +22,59 @@ const createOnClickHandler =
         onClick && onClick({ value }, evt)
         toggleSubMenu && toggleSubMenu()
     }
-const MenuItem = ({
-    href,
-    onClick,
-    children,
-    target,
-    icon,
-    className,
-    destructive,
-    disabled,
-    dense,
-    active,
-    dataTest,
-    chevron,
-    value,
-    label,
-    showSubMenu,
-    toggleSubMenu,
-    suffix,
-}) => {
+const MenuItem = forwardRef(function MenuItem(
+    {
+        href,
+        onClick,
+        children,
+        target,
+        icon,
+        className,
+        destructive,
+        disabled,
+        dense,
+        active,
+        dataTest,
+        chevron,
+        value,
+        label,
+        showSubMenu,
+        toggleSubMenu,
+        suffix,
+        tabIndex,
+        checkbox,
+        radio,
+        checked,
+    },
+    ref
+) {
     const menuItemRef = useRef()
+    const [openSubMenu, setOpenSubmenu] = useState(false)
+    const [subMenu, setSubMenu] = useState([])
+
+    const handleKeyDown = useCallback(
+        (event) => {
+            setSubMenu(document.querySelectorAll('[data-submenu-open=true]'))
+            switch (event.key) {
+                case 'ArrowRight':
+                    event.preventDefault()
+                    if (event.target.getAttribute('aria-haspopup')) {
+                        setOpenSubmenu(true)
+                    }
+                    break
+                case 'ArrowLeft':
+                    event.preventDefault()
+                    if (subMenu.length) {
+                        subMenu[subMenu.length - 1].focus()
+                    }
+                    setOpenSubmenu(false)
+                    break
+                default:
+                    return
+            }
+        },
+        [subMenu]
+    )
 
     return (
         <>
@@ -69,6 +102,26 @@ const MenuItem = ({
                               })
                             : undefined
                     }
+                    tabIndex={tabIndex}
+                    ref={ref}
+                    role={
+                        checkbox
+                            ? 'menuitemcheckbox'
+                            : radio
+                            ? 'menuitemradio'
+                            : 'menuitem'
+                    }
+                    aria-haspopup={children && 'menu'}
+                    aria-expanded={showSubMenu || openSubMenu}
+                    aria-disabled={disabled}
+                    aria-checked={checkbox && checked}
+                    aria-label={label}
+                    aria-owns={
+                        children &&
+                        `popper-${label.split(' ').join('-').toLowerCase()}`
+                    }
+                    data-submenu-open={children ? openSubMenu : null}
+                    onKeyDown={handleKeyDown}
                 >
                     {icon && <span className="icon">{icon}</span>}
 
@@ -85,23 +138,34 @@ const MenuItem = ({
 
                 <style jsx>{styles}</style>
             </li>
-            {children && showSubMenu && (
+            {children && (showSubMenu || openSubMenu) && (
                 <Portal>
-                    <Popper placement="right-start" reference={menuItemRef}>
+                    <Popper
+                        placement="right-start"
+                        reference={menuItemRef}
+                        id={`popper-${label
+                            .split(' ')
+                            .join('-')
+                            .toLowerCase()}`}
+                    >
                         <FlyoutMenu dense={dense}>{children}</FlyoutMenu>
                     </Popper>
                 </Portal>
             )}
         </>
     )
-}
+})
 
 MenuItem.defaultProps = {
     dataTest: 'dhis2-uicore-menuitem',
 }
 
 MenuItem.propTypes = {
     active: PropTypes.bool,
+    /* For using menuitem as checkbox */
+    checkbox: PropTypes.bool,
+    /* Value of checkbox */
+    checked: PropTypes.bool,
     chevron: PropTypes.bool,
     /**
      * Nested menu items can become submenus.
@@ -119,10 +183,13 @@ MenuItem.propTypes = {
     icon: PropTypes.node,
     /** Text in the menu item */
     label: PropTypes.node,
+    /* For using menuitem as radio */
+    radio: PropTypes.bool,
     /** When true, nested menu items are shown in a Popper */
     showSubMenu: PropTypes.bool,
     /** A supporting element shown at the end of the menu item */
     suffix: PropTypes.node,
+    tabIndex: PropTypes.number,
     /** For using menu item as a link */
     target: PropTypes.string,
     /** On click, this function is called (without args) */

components/menu/src/menu-item/menu-item.stories.js

@@ -133,6 +133,8 @@ export const ToggleMenuItem = (args) => {
                 onClick={toggleOn}
                 icon={icon}
                 label="A toggle menu item"
+                checkbox={true}
+                checked={on}
             />
         </Menu>
     )

components/menu/src/menu/__tests__/menu.test.js

@@ -0,0 +1,63 @@
+import { mount } from 'enzyme'
+import React from 'react'
+import { MenuDivider } from '../../menu-divider/menu-divider.js'
+import { MenuItem } from '../../menu-item/menu-item.js'
+import { MenuSectionHeader } from '../../menu-section-header/menu-section-header.js'
+import { Menu } from '../menu.js'
+
+describe('Menu Component', () => {
+    it('Menu and menu items have aria attributes', () => {
+        const menuDataTest = 'data-test-menu'
+        const menuItemDataTest = 'data-test-menu-item'
+        const dividerDataTest = 'data-test-menu-divider'
+        const wrapper = mount(
+            <Menu dataTest={menuDataTest} dense={false}>
+                <MenuSectionHeader label="Header" />
+                <MenuDivider dataTest={dividerDataTest} />
+                <MenuItem dataTest={menuItemDataTest} label="Menu item" />
+            </Menu>
+        )
+        const menuElement = wrapper.find({ 'data-test': menuDataTest })
+        const menuItem = wrapper.find({ 'data-test': menuItemDataTest })
+        const menuDivider = wrapper.find({ 'data-test': dividerDataTest })
+
+        expect(menuElement.prop('role')).toBe('menu')
+
+        expect(menuItem.childAt(0).props().role).toBe('menuitem')
+        expect(menuItem.childAt(0).prop('aria-label')).toBe('Menu item')
+        expect(menuDivider.prop('role')).toBe('separator')
+    })
+
+    it('Empty menu has role menu', () => {
+        const menuDataTest = 'data-test-menu'
+        const wrapper = mount(<Menu dataTest={menuDataTest} dense={false} />)
+        const menuElement = wrapper.find({ 'data-test': menuDataTest })
+        expect(menuElement.prop('role')).toBe('menu')
+    })
+
+    it('Submenu has relevant aria attributes', () => {
+        const showSubMenu = false
+        const wrapper = mount(
+            <Menu>
+                <MenuItem
+                    showSubMenu={showSubMenu}
+                    toggleSubMenu={() => jest.fn()}
+                    label="Parent of submenus"
+                >
+                    <MenuItem label="Test submenu child" />
+                </MenuItem>
+            </Menu>
+        )
+
+        const menuItem = wrapper.find({ role: 'menuitem' })
+
+        expect(menuItem.prop('aria-haspopup')).toBe('menu')
+        expect(menuItem.prop('aria-expanded')).toBe(false)
+        expect(menuItem.prop('aria-expanded')).toBe(false)
+        expect(menuItem.prop('aria-label')).toBe('Parent of submenus')
+        expect(wrapper.find({ label: 'Test submenu child' }).exists()).toBe(
+            false
+        )
+        // todo: simulate click to show submenu
+    })
+})