diff --git a/packages/react-core/src/components/Menu/Menu.tsx b/packages/react-core/src/components/Menu/Menu.tsx index 73f7d4ac20b..ba91953e1e6 100644 --- a/packages/react-core/src/components/Menu/Menu.tsx +++ b/packages/react-core/src/components/Menu/Menu.tsx @@ -63,7 +63,8 @@ export interface MenuProps extends Omit, 'r /** Set the value of data-ouia-safe. Only set to true when the component is in a static state, i.e. no animations are occurring. At all other times, this value must be false. */ ouiaSafe?: boolean; /** Determines the accessible role of the menu. For a non-checkbox menu that can have - * one or more items selected, pass in "listbox". */ + * one or more items selected, pass in "listbox". For a static list of actions that is not + * a dismissible menu widget, pass in "list". */ role?: string; } diff --git a/packages/react-core/src/components/Menu/MenuContext.ts b/packages/react-core/src/components/Menu/MenuContext.ts index d1c9d626240..1fc9fc01c15 100644 --- a/packages/react-core/src/components/Menu/MenuContext.ts +++ b/packages/react-core/src/components/Menu/MenuContext.ts @@ -45,3 +45,30 @@ export const MenuItemContext = createContext<{ itemId: null, isDisabled: false }); + +/** Returns the ARIA role for a menu item's interactive element based on the parent menu role. */ +export const getMenuItemInteractiveRole = (menuRole?: string): string | undefined => { + if (menuRole === 'listbox') { + return 'option'; + } + if (menuRole === 'list') { + return undefined; + } + return 'menuitem'; +}; + +/** Returns the ARIA role for a menu item action button based on the parent menu role. */ +export const getMenuItemActionInteractiveRole = (menuRole?: string): string | undefined => { + if (menuRole === 'listbox' || menuRole === 'list') { + return undefined; + } + return 'menuitem'; +}; + +/** Returns the ARIA role for a menu item's list item wrapper based on the parent menu role. */ +export const getMenuListItemRole = (menuRole: string | undefined, hasCheckbox: boolean): string | undefined => { + if (menuRole === 'list') { + return undefined; + } + return hasCheckbox ? 'menuitem' : 'none'; +}; diff --git a/packages/react-core/src/components/Menu/MenuItem.tsx b/packages/react-core/src/components/Menu/MenuItem.tsx index 355d77ad0a1..4245e0ea1ef 100644 --- a/packages/react-core/src/components/Menu/MenuItem.tsx +++ b/packages/react-core/src/components/Menu/MenuItem.tsx @@ -9,7 +9,7 @@ import RhMicronsCaretLeftIcon from '@patternfly/react-icons/dist/esm/icons/rh-mi import RhMicronsCaretRightIcon from '@patternfly/react-icons/dist/esm/icons/rh-microns-caret-right-icon'; import RhMicronsCheckmarkIcon from '@patternfly/react-icons/dist/esm/icons/rh-microns-checkmark-icon'; import { Checkbox } from '../Checkbox'; -import { MenuContext, MenuItemContext } from './MenuContext'; +import { getMenuItemInteractiveRole, getMenuListItemRole, MenuContext, MenuItemContext } from './MenuContext'; import { MenuItemAction } from './MenuItemAction'; import { Tooltip, TooltipProps } from '../Tooltip'; import { canUseDOM } from '../../helpers/util'; @@ -341,6 +341,8 @@ const MenuItemBase: React.FunctionComponent = ({ }, [isFocused]); const isSelectMenu = menuRole === 'listbox'; + const interactiveRole = !hasCheckbox && !flyoutMenu ? getMenuItemInteractiveRole(menuRole) : undefined; + const listItemRole = getMenuListItemRole(menuRole, hasCheckbox); const renderItem = ( <> @@ -350,7 +352,7 @@ const MenuItemBase: React.FunctionComponent = ({ className={css(styles.menuItem, getIsSelected() && !hasCheckbox && styles.modifiers.selected, className)} aria-current={getAriaCurrent()} {...(!hasCheckbox && { disabled: isDisabled, 'aria-label': ariaLabel })} - {...(!hasCheckbox && !flyoutMenu && { role: isSelectMenu ? 'option' : 'menuitem' })} + {...(interactiveRole !== undefined && { role: interactiveRole })} {...(!hasCheckbox && !flyoutMenu && isSelectMenu && { 'aria-selected': getIsSelected() })} ref={innerComponentRef} {...(!hasCheckbox && { @@ -452,7 +454,7 @@ const MenuItemBase: React.FunctionComponent = ({ }} {...(flyoutMenu && !isAriaDisabled && { onKeyDown: handleFlyout })} ref={ref} - role={!hasCheckbox ? 'none' : 'menuitem'} + {...(listItemRole !== undefined && { role: listItemRole })} {...(hasCheckbox && { 'aria-label': ariaLabel })} {...props} > diff --git a/packages/react-core/src/components/Menu/MenuItemAction.tsx b/packages/react-core/src/components/Menu/MenuItemAction.tsx index d01ce5d22be..f83962e2c00 100644 --- a/packages/react-core/src/components/Menu/MenuItemAction.tsx +++ b/packages/react-core/src/components/Menu/MenuItemAction.tsx @@ -1,7 +1,7 @@ import { forwardRef } from 'react'; import styles from '@patternfly/react-styles/css/components/Menu/menu'; import { css } from '@patternfly/react-styles'; -import { MenuContext, MenuItemContext } from './MenuContext'; +import { getMenuItemActionInteractiveRole, MenuContext, MenuItemContext } from './MenuContext'; import { Button } from '../Button'; export interface MenuItemActionProps extends React.HTMLProps { /** Additional classes added to the action button */ @@ -34,42 +34,45 @@ const MenuItemActionBase: React.FunctionComponent = ({ ...props }: MenuItemActionProps) => ( - {({ onActionClick }) => ( - - {({ itemId, isDisabled: isDisabledContext }) => { - const onClickButton = (event: any) => { - // event specified on the MenuItemAction - onClick && onClick(event); - // event specified on the Menu - onActionClick && onActionClick(event, itemId, actionId); - }; - return ( -
-
- ); - }} -
- )} + {({ onActionClick, role: menuRole }) => { + const interactiveRole = getMenuItemActionInteractiveRole(menuRole); + return ( + + {({ itemId, isDisabled: isDisabledContext }) => { + const onClickButton = (event: any) => { + // event specified on the MenuItemAction + onClick && onClick(event); + // event specified on the Menu + onActionClick && onActionClick(event, itemId, actionId); + }; + return ( +
+
+ ); + }} +
+ ); + }}
); diff --git a/packages/react-core/src/components/Menu/__tests__/Menu.test.tsx b/packages/react-core/src/components/Menu/__tests__/Menu.test.tsx index 08b9ffcbdec..bc0af10c129 100644 --- a/packages/react-core/src/components/Menu/__tests__/Menu.test.tsx +++ b/packages/react-core/src/components/Menu/__tests__/Menu.test.tsx @@ -3,6 +3,7 @@ import '@testing-library/jest-dom'; import { Menu } from '../Menu'; import { MenuItem, MenuItemProps } from '../MenuItem'; +import { MenuItemAction } from '../MenuItemAction'; import { MenuList } from '../MenuList'; import { MenuContent } from '../MenuContent'; @@ -94,4 +95,83 @@ describe('Menu', () => { expect(screen.getByText('Checkbox 1')).toBeInTheDocument(); }); }); + + describe('with role="list"', () => { + test('should render list semantics on menu items', () => { + render( + + + + Item + + + + ); + + expect(screen.getByRole('list')).toBeInTheDocument(); + + const listItem = screen.getByRole('listitem'); + expect(listItem).not.toHaveAttribute('role'); + + const button = screen.getByRole('button', { name: 'Item' }); + expect(button).not.toHaveAttribute('role'); + }); + }); + + describe('with role="listbox"', () => { + test('should render option semantics on menu items', () => { + render( + + + + Item + + + + ); + + expect(screen.getByRole('listbox')).toBeInTheDocument(); + expect(screen.getByRole('option', { name: 'Item' })).toBeInTheDocument(); + }); + + test('should not expose menu item actions as options', () => { + render( + + + + } + > + Item + + + + + ); + + expect(screen.getByRole('option', { name: 'Item' })).toBeInTheDocument(); + + const actionButton = screen.getByRole('button', { name: 'Favorite action' }); + expect(actionButton).not.toHaveAttribute('role'); + expect(screen.queryByRole('option', { name: 'Favorite action' })).not.toBeInTheDocument(); + }); + }); + + describe('with default menu role', () => { + test('should render menuitem semantics on menu items', () => { + render( + + + + Item + + + + ); + + expect(screen.getByRole('menu')).toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: 'Item' })).toBeInTheDocument(); + }); + }); });