Skip to content

Commit

Permalink
feat(menu-item): add ariaLabel prop to use if label prop is not a str…
Browse files Browse the repository at this point in the history
…ing (#1638)
  • Loading branch information
KaiVandivier authored Nov 25, 2024
1 parent 2e6c549 commit 2c1c065
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 3 deletions.
42 changes: 42 additions & 0 deletions components/menu/src/menu-item/__tests__/menu-item.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,46 @@ describe('Menu Component', () => {
false
)
})

describe('aria-label attribute', () => {
it('produces a log if the label prop is not a string', () => {
// Prep to test logs
const originalConsoleDebug = console.debug
const consoleDebugMock = jest.fn()
console.debug = consoleDebugMock

const menuItemDataTest = 'data-test-menu-item'
const wrapper = mount(
<MenuItem
dataTest={menuItemDataTest}
label={<span>{'Node label'}</span>}
/>
)
const menuItem = wrapper.find({ 'data-test': menuItemDataTest })

expect(menuItem.childAt(0).prop('role')).toBe('menuitem')
expect(menuItem.childAt(0).prop('aria-label')).toBe(undefined)
expect(consoleDebugMock).toHaveBeenCalledWith(
'The label prop on MenuItem is not a string; a value for the ariaLabel prop should be provided'
)

// Teardown
console.debug = originalConsoleDebug
})

it('uses the ariaLabel prop for aria-label if defined', () => {
const menuItemDataTest = 'data-test-menu-item'
const wrapper = mount(
<MenuItem
dataTest={menuItemDataTest}
label={<span>{'Node label'}</span>}
ariaLabel="Aria label"
/>
)
const menuItem = wrapper.find({ 'data-test': menuItemDataTest })

expect(menuItem.childAt(0).prop('role')).toBe('menuitem')
expect(menuItem.childAt(0).prop('aria-label')).toBe('Aria label')
})
})
})
23 changes: 20 additions & 3 deletions components/menu/src/menu-item/menu-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Popper } from '@dhis2-ui/popper'
import { Portal } from '@dhis2-ui/portal'
import cx from 'classnames'
import PropTypes from 'prop-types'
import React, { useEffect, useRef, useState } from 'react'
import React, { useEffect, useRef, useState, useMemo } from 'react'
import { FlyoutMenu } from '../flyout-menu/index.js'
import styles from './menu-item.styles.js'

Expand Down Expand Up @@ -37,6 +37,7 @@ const MenuItem = ({
chevron,
value,
label,
ariaLabel,
showSubMenu,
toggleSubMenu,
suffix,
Expand Down Expand Up @@ -86,6 +87,16 @@ const MenuItem = ({
}
}, [openSubMenus])

const resolvedAriaLabel = useMemo(() => {
if (typeof label !== 'string' && ariaLabel === undefined) {
console.debug(
'The label prop on MenuItem is not a string; a value for the ariaLabel prop should be provided'
)
}

return ariaLabel ?? (typeof label === 'string' ? label : undefined)
}, [ariaLabel, label])

return (
<>
<li
Expand Down Expand Up @@ -120,7 +131,7 @@ const MenuItem = ({
aria-disabled={disabled}
aria-haspopup={children && 'menu'}
aria-expanded={showSubMenu}
aria-label={label}
aria-label={resolvedAriaLabel}
>
{icon && <span className="icon">{icon}</span>}

Expand Down Expand Up @@ -150,6 +161,12 @@ const MenuItem = ({

MenuItem.propTypes = {
active: PropTypes.bool,
/**
* By default, the label prop is used for the aria-label attribute on the
* underlying HTML element. If this prop is defined, it will be used as the
* aria-label instead
*/
ariaLabel: PropTypes.string,
checkbox: PropTypes.bool,
checked: PropTypes.bool,
chevron: PropTypes.bool,
Expand All @@ -167,7 +184,7 @@ MenuItem.propTypes = {
href: PropTypes.string,
/** An icon for the left side of the menu item */
icon: PropTypes.node,
/** Text in the menu item */
/** Text in the menu item. If it's a string, will be used as aria-label */
label: PropTypes.node,
/** When true, nested menu items are shown in a Popper */
showSubMenu: PropTypes.bool,
Expand Down

0 comments on commit 2c1c065

Please sign in to comment.