Skip to content

Commit db32b34

Browse files
committed
♿️(frontend) fix menu semantics and trigger ARIA for screen readers
Dropdown menu exposes proper ARIA, Popover no longer announces as dialog
1 parent f454a4f commit db32b34

2 files changed

Lines changed: 105 additions & 7 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# @gouvfr-lasuite/ui-kit
22

3+
### Patch changes
4+
5+
- ♿️(frontend) fix menu semantics and trigger ARIA for screen readers #210
6+
37
## 0.20.1
48

59
### Patch Changes

src/components/dropdown-menu/DropdownMenu.tsx

Lines changed: 101 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,18 @@ import {
77
SubmenuTrigger,
88
} from "react-aria-components";
99
import { DropdownMenuItem, DropdownMenuOption } from "./types";
10-
import { Fragment, PropsWithChildren, ReactNode, useId, useRef } from "react";
10+
import {
11+
Children,
12+
cloneElement,
13+
Fragment,
14+
isValidElement,
15+
PropsWithChildren,
16+
ReactElement,
17+
ReactNode,
18+
useEffect,
19+
useId,
20+
useRef,
21+
} from "react";
1122
import { MenuItemSeparator } from "../menu/types";
1223
import clsx from "clsx";
1324

@@ -33,6 +44,42 @@ const MenuItemContent = ({ option }: { option: DropdownMenuOption }) => (
3344
</>
3445
);
3546

47+
/**
48+
* Walk the React tree to find the first interactive element (button, link,
49+
* or anything with onClick/onPress) and inject ARIA trigger attributes on it.
50+
* This way consumers can wrap their trigger in layout divs without breaking
51+
* the screen reader announcement.
52+
*/
53+
const injectAriaAttrs = (
54+
node: ReactNode,
55+
attrs: Record<string, unknown>,
56+
state: { done: boolean },
57+
): ReactNode => {
58+
if (state.done || !isValidElement(node)) return node;
59+
const element = node as ReactElement<Record<string, unknown>>;
60+
61+
const isInteractive =
62+
element.type === "button" ||
63+
element.type === "a" ||
64+
"onClick" in element.props ||
65+
"onPress" in element.props;
66+
67+
if (isInteractive) {
68+
state.done = true;
69+
return cloneElement(element, attrs);
70+
}
71+
72+
if (element.props.children) {
73+
const newChildren = Children.map(
74+
element.props.children as ReactNode,
75+
(child) => injectAriaAttrs(child, attrs, state),
76+
);
77+
return cloneElement(element, {}, newChildren);
78+
}
79+
80+
return node;
81+
};
82+
3683
export type DropdownMenuProps = {
3784
options: DropdownMenuItem[];
3885
onOpenChange?: (isOpen: boolean) => void;
@@ -56,12 +103,55 @@ export const DropdownMenu = ({
56103
variant = "default",
57104
}: PropsWithChildren<DropdownMenuProps>) => {
58105
const id = useId();
59-
const triggerRef = useRef(null);
106+
const triggerRef = useRef<HTMLDivElement>(null);
107+
const popoverRef = useRef<HTMLDivElement>(null);
60108
const menuClassName = `c__dropdown-menu${variant === "tiny" ? " c__dropdown-menu--tiny" : ""}`;
61109
const onOpenChangeHandler = (isOpen: boolean) => {
62110
onOpenChange?.(isOpen);
63111
};
64112

113+
// React Aria's Popover always adds role="dialog" on the overlay element.
114+
// That's correct for dialogs, but wrong for menus: a screen reader would
115+
// announce "dialogue" when opening a simple dropdown, which is confusing.
116+
// The WAI-ARIA Menu Button pattern expects role="menu", not role="dialog".
117+
// We use a MutationObserver to remove it as soon as Popover sets it,
118+
// before the screen reader gets a chance to read it.
119+
useEffect(() => {
120+
const node = popoverRef.current;
121+
if (!node) return;
122+
123+
const removeDialogRole = () => {
124+
if (node.getAttribute("role") === "dialog") {
125+
node.removeAttribute("role");
126+
}
127+
if (node.getAttribute("tabindex") === "-1") {
128+
node.removeAttribute("tabindex");
129+
}
130+
};
131+
132+
removeDialogRole();
133+
const observer = new MutationObserver(removeDialogRole);
134+
observer.observe(node, {
135+
attributes: true,
136+
attributeFilter: ["role", "tabindex"],
137+
});
138+
139+
return () => observer.disconnect();
140+
}, [isOpen]);
141+
142+
// Inject aria-expanded / aria-haspopup / aria-controls on the actual
143+
// trigger button, not the wrapper div. This tells the SR what kind of
144+
// popup to expect and whether it is currently open.
145+
const childWithAria = injectAriaAttrs(
146+
children,
147+
{
148+
"aria-expanded": isOpen,
149+
"aria-haspopup": "menu" as const,
150+
"aria-controls": isOpen ? `${id}-menu` : undefined,
151+
},
152+
{ done: false },
153+
);
154+
65155
const renderMenuItems = (items: DropdownMenuItem[]) =>
66156
items.map((option, index) => {
67157
if (isSeparator(option)) {
@@ -141,21 +231,25 @@ export const DropdownMenu = ({
141231
e.preventDefault();
142232
}}
143233
>
144-
{children}
234+
{childWithAria}
145235
</div>
146236

147237
<Popover
238+
ref={popoverRef}
148239
triggerRef={triggerRef}
149-
style={{
150-
marginTop: "0px",
151-
}}
240+
style={{ marginTop: "0px" }}
152241
isOpen={isOpen}
153242
shouldFlip
154243
containerPadding={16}
155244
shouldCloseOnInteractOutside={shouldCloseOnInteractOutside}
156245
onOpenChange={onOpenChangeHandler}
157246
>
158-
<Menu className={menuClassName} aria-labelledby={id} autoFocus="first">
247+
<Menu
248+
id={`${id}-menu`}
249+
className={menuClassName}
250+
aria-labelledby={id}
251+
autoFocus="first"
252+
>
159253
{topMessage && (
160254
<Header
161255
className="c__dropdown-menu-item-top-message"

0 commit comments

Comments
 (0)