feat(menu): add aria-attributes and roles to menu and its components (#…

…1514) * feat: add relevant aria-attributes and roles to menu and its child components * fix: update role to menuitem in sharing-dialog-autocomplete test
dhis2 · Jul 1, 2024 · 54b816c · 54b816c
1 parent 4a37175
commit 54b816c
Show file tree

Hide file tree

Showing 8 changed files with 148 additions and 8 deletions.
diff --git a/components/menu/src/menu-divider/menu-divider.js b/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>{`

diff --git a/components/menu/src/menu-item/__tests__/menu-item.test.js b/components/menu/src/menu-item/__tests__/menu-item.test.js
@@ -0,0 +1,76 @@
+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
+                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'
+        )
+    })
+
+    it('Submenu has relevant aria attributes', () => {
+        const showSubMenu = false
+        const wrapper = mount(
+            <MenuItem
+                showSubMenu={showSubMenu}
+                toggleSubMenu={() => jest.fn()}
+                label="Parent of submenus"
+            >
+                <MenuItem label="Test submenu child" />
+            </MenuItem>
+        )
+
+        const menuItem = wrapper.find({ role: 'menuitem' })
+
+        expect(menuItem.prop('aria-haspopup')).toBe('menu')
+        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
+        )
+    })
+})
diff --git a/components/menu/src/menu-item/menu-item.js b/components/menu/src/menu-item/menu-item.js
@@ -40,6 +40,8 @@ const MenuItem = ({
     showSubMenu,
     toggleSubMenu,
     suffix,
+    checkbox,
+    checked,
 }) => {
     const menuItemRef = useRef()
 
@@ -55,6 +57,7 @@ const MenuItem = ({
                 })}
                 ref={menuItemRef}
                 data-test={dataTest}
+                role="presentation"
             >
                 <a
                     target={target}
@@ -69,6 +72,12 @@ const MenuItem = ({
                               })
                             : undefined
                     }
+                    role={checkbox ? 'menuitemcheckbox' : 'menuitem'}
+                    aria-checked={checkbox ? checked : null}
+                    aria-disabled={disabled}
+                    aria-haspopup={children && 'menu'}
+                    aria-expanded={showSubMenu}
+                    aria-label={label}
                 >
                     {icon && <span className="icon">{icon}</span>}
 
@@ -102,6 +111,8 @@ MenuItem.defaultProps = {
 
 MenuItem.propTypes = {
     active: PropTypes.bool,
+    checkbox: PropTypes.bool,
+    checked: PropTypes.bool,
     chevron: PropTypes.bool,
     /**
      * Nested menu items can become submenus.

diff --git a/components/menu/src/menu-item/menu-item.stories.js b/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
+                checked={on}
             />
         </Menu>
     )

diff --git a/components/menu/src/menu/__tests__/menu.test.js b/components/menu/src/menu/__tests__/menu.test.js
@@ -0,0 +1,37 @@
+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')
+    })
+})
diff --git a/components/menu/src/menu/menu.js b/components/menu/src/menu/menu.js
@@ -2,7 +2,7 @@ import PropTypes from 'prop-types'
 import React, { Children, cloneElement, isValidElement } from 'react'
 
 const Menu = ({ children, className, dataTest, dense }) => (
-    <ul className={className} data-test={dataTest}>
+    <ul className={className} data-test={dataTest} role="menu">
         {Children.map(children, (child, index) =>
             isValidElement(child)
                 ? cloneElement(child, {

diff --git a/components/menu/types/index.d.ts b/components/menu/types/index.d.ts
@@ -42,6 +42,14 @@ export const MenuDivider: React.FC<MenuDividerProps>
 
 export interface MenuItemProps {
     active?: boolean
+    /**
+     * Specifies if menu item is a checkbox
+     */
+    checkbox?: boolean
+    /**
+     * checkbox state for toggleable menu items
+     */
+    checked?: boolean
     chevron?: boolean
     /**
      * Nested menu items can become submenus.

diff --git a/components/sharing-dialog/src/autocomplete/sharing-autocomplete.test.js b/components/sharing-dialog/src/autocomplete/sharing-autocomplete.test.js
@@ -1,5 +1,10 @@
 import { CustomDataProvider } from '@dhis2/app-runtime'
-import { render, screen, waitFor } from '@testing-library/react'
+import {
+    render,
+    screen,
+    waitFor,
+    waitForElementToBeRemoved,
+} from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import '@testing-library/jest-dom'
 import React, { useState } from 'react'
@@ -43,17 +48,18 @@ describe('SharingAutocomplete', () => {
         userEvent.type(screen.getByRole('textbox'), searchString)
         expect(screen.getByRole('textbox')).toHaveValue(searchString)
 
-        await waitFor(() => screen.getByRole('listitem'))
-        userEvent.click(screen.getByRole('listitem').querySelector('a'))
+        await waitForElementToBeRemoved(() => screen.getByRole('progressbar'))
+
+        userEvent.click(screen.getByRole('menuitem'))
         expect(screen.getByRole('textbox')).toHaveValue(userDisplayName)
 
         try {
-            await waitFor(() => screen.getByRole('listitem'), { timeout: 1 })
+            await waitFor(() => screen.getByRole('menuitem'), { timeout: 1 })
         } catch (error) {
-            if (!error.message.startsWith('Unable to find role="listitem"')) {
+            if (!error.message.startsWith('Unable to find role="menuitem"')) {
                 throw error
             }
         }
-        expect(screen.queryByRole('listitem')).not.toBeInTheDocument()
+        expect(screen.queryByRole('menuitem')).not.toBeInTheDocument()
     })
 })