diff --git a/components/menu/src/menu-item/menu-item.js b/components/menu/src/menu-item/menu-item.js index edacfac93d..272f675c7a 100644 --- a/components/menu/src/menu-item/menu-item.js +++ b/components/menu/src/menu-item/menu-item.js @@ -53,7 +53,7 @@ const MenuItem = ({ destructive, disabled, dense, - active: active || showSubMenu, + active: active || showSubMenu || tabIndex === 0, 'with-chevron': children || chevron, })} ref={menuItemRef} diff --git a/components/menu/src/menu/__tests__/menu.test.js b/components/menu/src/menu/__tests__/menu.test.js index e265136d5a..d95efe58ef 100644 --- a/components/menu/src/menu/__tests__/menu.test.js +++ b/components/menu/src/menu/__tests__/menu.test.js @@ -139,46 +139,45 @@ describe('Menu Component', () => { expect(onClick).toHaveBeenCalledTimes(2) }) - it('can handle non MenuItem components', async () => { + it('can handle non MenuItem components', () => { const onClick = jest.fn() const { getByText } = render( - - Link 1 - - Item 1 + Span 1 - +
  • + + Link 2 + +
  • +
  • + Span 2 +
  • ) - const span = getByText(/Item 1/i) - const link = getByText(/link 1/i) - const menuItem = getByText(/menu item 2/i) + const nonListMenuItem = getByText(/span 1/i) + const listMenuItem = getByText(/link 2/i) + const plainListItem = getByText(/span 2/i) + + // all children must be list items + expect(nonListMenuItem.parentElement.nodeName).toBe('LI') userEvent.tab() + expect(nonListMenuItem.parentElement).toHaveFocus() + expect(nonListMenuItem.parentElement.tabIndex).toBe(0) - expect(link).toHaveFocus() - expect(link.tabIndex).toBe(0) - expect(span).not.toHaveFocus() - expect(menuItem.parentNode.parentNode).not.toHaveFocus() + expect(onClick).toHaveBeenCalledTimes(0) + userEvent.keyboard('[Space]') + expect(onClick).toHaveBeenCalledTimes(1) userEvent.keyboard('{ArrowDown}') - expect(link).not.toHaveFocus() - expect(span).toHaveFocus() - expect(menuItem.parentNode.parentNode).not.toHaveFocus() + expect(listMenuItem.parentElement).toHaveFocus() userEvent.keyboard('{ArrowDown}') - expect(link).not.toHaveFocus() - expect(span).not.toHaveFocus() - expect(menuItem.parentNode.parentNode).toHaveFocus() - - userEvent.keyboard('{ArrowUp}') - expect(span).toHaveFocus() - - expect(onClick).toHaveBeenCalledTimes(0) - userEvent.keyboard('[Space]') - expect(onClick).toHaveBeenCalledTimes(1) + expect(nonListMenuItem.parentElement).toHaveFocus() + // non menu items do not receive focus + expect(plainListItem.parentElement).not.toHaveFocus() }) }) diff --git a/components/menu/src/menu/helpers.js b/components/menu/src/menu/helpers.js index 74cce06411..c01b03b2d1 100644 --- a/components/menu/src/menu/helpers.js +++ b/components/menu/src/menu/helpers.js @@ -1,23 +1,26 @@ -export const isValidMenuItem = (node) => { - const role = node.getAttribute('role') - - const menuItemCheck = - role && ['menuitem', 'menuitemcheckbox', 'menuitemradio'].includes(role) +const isMenuItem = (role) => { + return ['menuitem', 'menuitemcheckbox', 'menuitemradio'].includes(role) +} - // UI Library MenuItem - if (role === 'presentation') { - return isValidMenuItem(node.children[0]) - } else { - return role !== 'separator' && menuItemCheck +const isValidMenuItemNode = (node) => { + if (node.nodeName === 'LI' && node.firstElementChild) { + return isValidMenuItemNode(node.firstElementChild) } + + const role = node.getAttribute('role') + return role && isMenuItem(role) } -export const filterValidMenuItemsIndices = (children) => { - const validIndices = [] - children.forEach((node, index) => { - if (isValidMenuItem(node)) { - validIndices.push(index) +export const getFocusableItemsIndices = (elements) => { + const focusableIndices = [] + elements.forEach((node, index) => { + if (isValidMenuItemNode(node)) { + focusableIndices.push(index) } }) - return validIndices + return focusableIndices +} + +export const hasMenuItemRole = (component) => { + return isMenuItem(component?.props?.['role']) } diff --git a/components/menu/src/menu/menu.js b/components/menu/src/menu/menu.js index 4667d715c9..c7874e76b6 100644 --- a/components/menu/src/menu/menu.js +++ b/components/menu/src/menu/menu.js @@ -1,9 +1,54 @@ import PropTypes from 'prop-types' import React, { Children, cloneElement, isValidElement } from 'react' +import { hasMenuItemRole } from './helpers.js' import { useMenuNavigation } from './use-menu.js' const Menu = ({ children, className, dataTest, dense }) => { const { menuRef, focusedIndex } = useMenuNavigation(children) + + const childrenToRender = Children.map(children, (child, index) => { + if (!isValidElement(child)) { + return child + } + const tabIndex = index === focusedIndex ? 0 : -1 + + const childProps = { + ...child.props, + } + + if (typeof child.type === 'string') { + // remove non-native props from native HTML elements + delete childProps.hideDivider + delete childProps.dense + delete childProps.active + + // all ul children must be li elements + // add tabindex for focus to those elements that are/contain a menuitem + if (child.type === 'li') { + return hasMenuItemRole(child.props.children[0]) + ? cloneElement(child, { ...childProps, tabIndex }) + : cloneElement(child, childProps) + } else { + return ( +
  • + {cloneElement(child, childProps)} +
  • + ) + } + } else { + // assign non-native props to custom elements + childProps.dense = + typeof child.props.dense === 'boolean' + ? child.props.dense + : dense + childProps.hideDivider = + typeof child.props.hideDivider !== 'boolean' && index === 0 + ? true + : child.props.hideDivider + return cloneElement(child, { ...childProps, tabIndex }) + } + }) + return (