Skip to content

Commit

Permalink
feat(a11y): remove incorrect expanded attribute on OptionsMenu (#1792)
Browse files Browse the repository at this point in the history
  • Loading branch information
scurker authored Feb 18, 2025
1 parent dfa0905 commit b60e295
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 12 deletions.
14 changes: 7 additions & 7 deletions packages/react/src/components/OptionsMenu/OptionsMenu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,10 @@ test('should toggle menu on trigger clicks', async () => {

await user.click(button);
expect(button).toHaveAttribute('aria-expanded', 'true');
expect(screen.getByRole('menu')).toHaveAttribute('aria-expanded', 'true');

expect(screen.getByRole('menu')).toHaveClass('OptionsMenu--expanded');
await user.click(button);

expect(button).toHaveAttribute('aria-expanded', 'false');
expect(screen.getByRole('menu')).toHaveAttribute('aria-expanded', 'false');
expect(screen.getByRole('menu')).not.toHaveClass('OptionsMenu--expanded');
});

test('should click trigger with down key on trigger', () => {
Expand Down Expand Up @@ -188,10 +186,12 @@ test('should close menu when click outside event occurs', async () => {
</>
);

await user.click(screen.getByRole('button', { name: 'trigger' }));
expect(screen.getByRole('menu')).toHaveAttribute('aria-expanded', 'true');
const triggerButton = screen.getByRole('button', { name: 'trigger' });
await user.click(triggerButton);
expect(triggerButton).toHaveAttribute('aria-expanded', 'true');
await user.click(screen.getByRole('button', { name: 'Click me!' }));
expect(screen.getByRole('menu')).toHaveAttribute('aria-expanded', 'false');
expect(triggerButton).toHaveAttribute('aria-expanded', 'false');
expect(screen.getByRole('menu')).not.toHaveClass('OptionsMenu--expanded');
});

test('should return no axe violations when hidden', async () => {
Expand Down
7 changes: 3 additions & 4 deletions packages/react/src/components/OptionsMenu/OptionsMenuList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,9 @@ const OptionsMenuList = ({
>
<ul
{...other}
className={classnames('OptionsMenu__list', className)}
/* aria-expanded is not correct usage here, but the pattern library
currently styles the open state of the menu based on this attribute */
aria-expanded={show}
className={classnames('OptionsMenu__list', className, {
'OptionsMenu--expanded': show
})}
role="menu"
onClick={handleClick}
ref={(el) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/styles/options-menu.css
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
box-shadow: 0 2px 3px 0 rgba(0, 0, 0, 0.15);
}

.OptionsMenu__list[aria-expanded='true'] {
.OptionsMenu__list:is(.OptionsMenu--expanded) {
display: block;
}

Expand Down

0 comments on commit b60e295

Please sign in to comment.