Skip to content

Commit 4672e95

Browse files
authored
fix(modal): skip hidden elements when setting focus (#1400)
1 parent 83d7604 commit 4672e95

2 files changed

Lines changed: 112 additions & 18 deletions

File tree

src/components/Modal/Modal.test.tsx

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,4 +262,60 @@ describe("Modal ", () => {
262262
const proceedButton = screen.getByRole("button", { name: /^proceed$/i });
263263
expect(proceedButton).toHaveFocus();
264264
});
265+
266+
it("excludes hidden focusable elements from the focus trap", async () => {
267+
const user = userEvent.setup();
268+
const { container } = render(
269+
<Modal
270+
title="Test"
271+
close={jest.fn()}
272+
buttonRow={
273+
<>
274+
<button id="test-cancel">Cancel</button>
275+
<button id="test-hidden" style={{ display: "none" }}>
276+
Hidden
277+
</button>
278+
</>
279+
}
280+
>
281+
Bare bones
282+
</Modal>,
283+
);
284+
285+
const closeButton = container.querySelector("button.p-modal__close");
286+
const cancelButton = container.querySelector("button#test-cancel");
287+
288+
expect(closeButton).toHaveFocus();
289+
await user.tab({ shift: true });
290+
expect(cancelButton).toHaveFocus();
291+
292+
await user.tab();
293+
expect(closeButton).toHaveFocus();
294+
});
295+
296+
it("initially focuses the first visible focusable element when the close button is hidden", () => {
297+
const style = document.createElement("style");
298+
style.innerHTML = ".p-modal__close { display: none; }";
299+
document.head.appendChild(style);
300+
301+
try {
302+
const { container } = render(
303+
<Modal
304+
title="Test"
305+
close={jest.fn()}
306+
buttonRow={<button id="test-cancel">Cancel</button>}
307+
>
308+
Bare bones
309+
</Modal>,
310+
);
311+
312+
const closeButton = container.querySelector("button.p-modal__close");
313+
const cancelButton = container.querySelector("button#test-cancel");
314+
315+
expect(closeButton).not.toHaveFocus();
316+
expect(cancelButton).toHaveFocus();
317+
} finally {
318+
document.head.removeChild(style);
319+
}
320+
});
265321
});

src/components/Modal/Modal.tsx

Lines changed: 56 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -67,24 +67,46 @@ export const Modal = ({
6767

6868
const modalRef: RefObject<HTMLDivElement> = useRef(null);
6969
const closeButtonRef: RefObject<HTMLButtonElement> = useRef(null);
70-
const handleTabKey = (event: React.KeyboardEvent<HTMLDivElement>) => {
71-
const focusableModalElements = modalRef.current.querySelectorAll(
72-
focusableElementSelectors,
73-
);
74-
if (focusableModalElements.length > 0) {
75-
const firstElement = focusableModalElements[0];
76-
const lastElement =
77-
focusableModalElements[focusableModalElements.length - 1];
78-
79-
if (!event.shiftKey && document.activeElement === lastElement) {
80-
(firstElement as HTMLElement).focus();
81-
event.preventDefault();
82-
}
8370

84-
if (event.shiftKey && document.activeElement === firstElement) {
85-
(lastElement as HTMLElement).focus();
86-
return event.preventDefault();
71+
// determines whether an element is visible by checking computed styles
72+
// fails open: an element is only treated as hidden when getComputedStyle
73+
// explicitly reports `display:none` or `visibility:hidden`
74+
const isElementVisible = (element: Element): boolean => {
75+
let current: Element | null = element;
76+
while (current) {
77+
const style = window.getComputedStyle(current);
78+
if (style.display === "none" || style.visibility === "hidden") {
79+
return false;
8780
}
81+
current = current.parentElement;
82+
}
83+
return true;
84+
};
85+
86+
const getVisibleFocusableElements = (): HTMLElement[] => {
87+
if (!modalRef.current) {
88+
return [];
89+
}
90+
return Array.from(
91+
modalRef.current.querySelectorAll<HTMLElement>(focusableElementSelectors),
92+
).filter(isElementVisible);
93+
};
94+
95+
const handleTabKey = (event: React.KeyboardEvent<HTMLDivElement>) => {
96+
const focusableModalElements = getVisibleFocusableElements();
97+
if (focusableModalElements.length === 0) {
98+
return;
99+
}
100+
const firstElement = focusableModalElements[0];
101+
const lastElement =
102+
focusableModalElements[focusableModalElements.length - 1];
103+
104+
if (event.shiftKey && document.activeElement === firstElement) {
105+
lastElement.focus();
106+
event.preventDefault();
107+
} else if (!event.shiftKey && document.activeElement === lastElement) {
108+
firstElement.focus();
109+
event.preventDefault();
88110
}
89111
};
90112

@@ -103,13 +125,29 @@ export const Modal = ({
103125
}
104126
};
105127

128+
const focusModalWrapper = () => {
129+
if (modalRef.current) {
130+
modalRef.current.tabIndex = -1;
131+
modalRef.current.focus();
132+
}
133+
};
134+
106135
useEffect(() => {
107136
if (focusRef?.current) {
108137
focusRef.current.focus();
109138
} else if (closeButtonRef.current) {
110-
closeButtonRef.current.focus();
139+
if (isElementVisible(closeButtonRef.current)) {
140+
closeButtonRef.current.focus();
141+
} else {
142+
const firstFocusable = getVisibleFocusableElements()[0];
143+
if (firstFocusable) {
144+
firstFocusable.focus();
145+
} else {
146+
focusModalWrapper();
147+
}
148+
}
111149
} else {
112-
modalRef.current.focus();
150+
focusModalWrapper();
113151
}
114152
}, [focusRef]);
115153

0 commit comments

Comments
 (0)