Skip to content

Commit ae41bf4

Browse files
committed
Address CodeRabbit feedback
1 parent 4b019af commit ae41bf4

3 files changed

Lines changed: 34 additions & 2 deletions

File tree

packages/react-core/src/components/Menu/MenuContext.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,14 @@ export const getMenuItemInteractiveRole = (menuRole?: string): string | undefine
5757
return 'menuitem';
5858
};
5959

60+
/** Returns the ARIA role for a menu item action button based on the parent menu role. */
61+
export const getMenuItemActionInteractiveRole = (menuRole?: string): string | undefined => {
62+
if (menuRole === 'listbox' || menuRole === 'list') {
63+
return undefined;
64+
}
65+
return 'menuitem';
66+
};
67+
6068
/** Returns the ARIA role for a menu item's list item wrapper based on the parent menu role. */
6169
export const getMenuListItemRole = (menuRole: string | undefined, hasCheckbox: boolean): string | undefined => {
6270
if (menuRole === 'list') {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { forwardRef } from 'react';
22
import styles from '@patternfly/react-styles/css/components/Menu/menu';
33
import { css } from '@patternfly/react-styles';
4-
import { getMenuItemInteractiveRole, MenuContext, MenuItemContext } from './MenuContext';
4+
import { getMenuItemActionInteractiveRole, MenuContext, MenuItemContext } from './MenuContext';
55
import { Button } from '../Button';
66
export interface MenuItemActionProps extends React.HTMLProps<HTMLDivElement> {
77
/** Additional classes added to the action button */
@@ -35,7 +35,7 @@ const MenuItemActionBase: React.FunctionComponent<MenuItemActionProps> = ({
3535
}: MenuItemActionProps) => (
3636
<MenuContext.Consumer>
3737
{({ onActionClick, role: menuRole }) => {
38-
const interactiveRole = getMenuItemInteractiveRole(menuRole);
38+
const interactiveRole = getMenuItemActionInteractiveRole(menuRole);
3939
return (
4040
<MenuItemContext.Consumer>
4141
{({ itemId, isDisabled: isDisabledContext }) => {

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import '@testing-library/jest-dom';
33

44
import { Menu } from '../Menu';
55
import { MenuItem, MenuItemProps } from '../MenuItem';
6+
import { MenuItemAction } from '../MenuItemAction';
67
import { MenuList } from '../MenuList';
78
import { MenuContent } from '../MenuContent';
89

@@ -132,6 +133,29 @@ describe('Menu', () => {
132133
expect(screen.getByRole('listbox')).toBeInTheDocument();
133134
expect(screen.getByRole('option', { name: 'Item' })).toBeInTheDocument();
134135
});
136+
137+
test('should not expose menu item actions as options', () => {
138+
render(
139+
<Menu role="listbox" selected={0}>
140+
<MenuContent>
141+
<MenuList>
142+
<MenuItem
143+
itemId={0}
144+
actions={<MenuItemAction aria-label="Favorite action" actionId="fav" icon="favorites" />}
145+
>
146+
Item
147+
</MenuItem>
148+
</MenuList>
149+
</MenuContent>
150+
</Menu>
151+
);
152+
153+
expect(screen.getByRole('option', { name: 'Item' })).toBeInTheDocument();
154+
155+
const actionButton = screen.getByRole('button', { name: 'Favorite action' });
156+
expect(actionButton).not.toHaveAttribute('role');
157+
expect(screen.queryByRole('option', { name: 'Favorite action' })).not.toBeInTheDocument();
158+
});
135159
});
136160

137161
describe('with default menu role', () => {

0 commit comments

Comments
 (0)