fix(Menu): Allow for different MenuItem roles

packages/react-core/src/components/Menu/Menu.tsx

@@ -63,7 +63,8 @@ export interface MenuProps extends Omit<React.HTMLAttributes<HTMLDivElement>, '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;
 }
 

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';
+};

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<MenuItemProps> = ({
   }, [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<MenuItemProps> = ({
         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<MenuItemProps> = ({
         }}
         {...(flyoutMenu && !isAriaDisabled && { onKeyDown: handleFlyout })}
         ref={ref}
-        role={!hasCheckbox ? 'none' : 'menuitem'}
+        {...(listItemRole !== undefined && { role: listItemRole })}
         {...(hasCheckbox && { 'aria-label': ariaLabel })}
         {...props}
       >

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<HTMLDivElement> {
   /** Additional classes added to the action button */
@@ -34,42 +34,45 @@ const MenuItemActionBase: React.FunctionComponent<MenuItemActionProps> = ({
   ...props
 }: MenuItemActionProps) => (
   <MenuContext.Consumer>
-    {({ onActionClick }) => (
-      <MenuItemContext.Consumer>
-        {({ 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 (
-            <div
-              className={css(
-                styles.menuItemAction,
-                isFavorited !== null && 'pf-m-favorite',
-                isFavorited && styles.modifiers.favorited,
-                className
-              )}
-              {...props}
-            >
-              <Button
-                aria-label={ariaLabel}
-                onClick={onClickButton}
-                ref={innerRef}
-                role="menuitem"
-                variant="plain"
-                isFavorite={isFavorited !== null}
-                isFavorited={isFavorited ?? false}
-                tabIndex={-1}
-                isDisabled={isDisabled || isDisabledContext}
-                icon={isFavorited === null ? icon : undefined}
-              />
-            </div>
-          );
-        }}
-      </MenuItemContext.Consumer>
-    )}
+    {({ onActionClick, role: menuRole }) => {
+      const interactiveRole = getMenuItemActionInteractiveRole(menuRole);
+      return (
+        <MenuItemContext.Consumer>
+          {({ 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 (
+              <div
+                className={css(
+                  styles.menuItemAction,
+                  isFavorited !== null && 'pf-m-favorite',
+                  isFavorited && styles.modifiers.favorited,
+                  className
+                )}
+                {...props}
+              >
+                <Button
+                  aria-label={ariaLabel}
+                  onClick={onClickButton}
+                  ref={innerRef}
+                  {...(interactiveRole !== undefined && { role: interactiveRole })}
+                  variant="plain"
+                  isFavorite={isFavorited !== null}
+                  isFavorited={isFavorited ?? false}
+                  tabIndex={-1}
+                  isDisabled={isDisabled || isDisabledContext}
+                  icon={isFavorited === null ? icon : undefined}
+                />
+              </div>
+            );
+          }}
+        </MenuItemContext.Consumer>
+      );
+    }}
   </MenuContext.Consumer>
 );
 

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(
+        <Menu role="list">
+          <MenuContent>
+            <MenuList>
+              <MenuItem itemId={0}>Item</MenuItem>
+            </MenuList>
+          </MenuContent>
+        </Menu>
+      );
+
+      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(
+        <Menu role="listbox" selected={0}>
+          <MenuContent>
+            <MenuList>
+              <MenuItem itemId={0}>Item</MenuItem>
+            </MenuList>
+          </MenuContent>
+        </Menu>
+      );
+
+      expect(screen.getByRole('listbox')).toBeInTheDocument();
+      expect(screen.getByRole('option', { name: 'Item' })).toBeInTheDocument();
+    });
+
+    test('should not expose menu item actions as options', () => {
+      render(
+        <Menu role="listbox" selected={0}>
+          <MenuContent>
+            <MenuList>
+              <MenuItem
+                itemId={0}
+                actions={<MenuItemAction aria-label="Favorite action" actionId="fav" icon="favorites" />}
+              >
+                Item
+              </MenuItem>
+            </MenuList>
+          </MenuContent>
+        </Menu>
+      );
+
+      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(
+        <Menu>
+          <MenuContent>
+            <MenuList>
+              <MenuItem itemId={0}>Item</MenuItem>
+            </MenuList>
+          </MenuContent>
+        </Menu>
+      );
+
+      expect(screen.getByRole('menu')).toBeInTheDocument();
+      expect(screen.getByRole('menuitem', { name: 'Item' })).toBeInTheDocument();
+    });
+  });
 });