Skip to content

Commit cb3161d

Browse files
fix(Modal): updated logic to set aria-hidden for tearsheets (#12627)
* fix(Modal): updated logic to set aria-hidden for tearsheets * Coderabbit suggestion * Added close cleanup tweaks * Reverted basic example
1 parent f20f073 commit cb3161d

2 files changed

Lines changed: 194 additions & 7 deletions

File tree

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

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ interface ModalState {
6969
class Modal extends Component<ModalProps, ModalState> {
7070
static displayName = 'Modal';
7171
static currentId = 0;
72+
static openModalStacks: Map<HTMLElement, string[]> = new Map();
7273
boxId = '';
7374
backdropId = '';
7475

@@ -106,16 +107,46 @@ class Modal extends Component<ModalProps, ModalState> {
106107
return appendTo || document.body;
107108
};
108109

110+
static getStackForTarget(target: HTMLElement): string[] {
111+
if (!Modal.openModalStacks.has(target)) {
112+
Modal.openModalStacks.set(target, []);
113+
}
114+
return Modal.openModalStacks.get(target)!;
115+
}
116+
109117
toggleSiblingsFromScreenReaders = (hide: boolean) => {
110118
const { appendTo } = this.props;
111119
const target: HTMLElement = this.getElement(appendTo);
112-
const bodyChildren = target.children;
113-
for (const child of Array.from(bodyChildren)) {
114-
const isPopperElement = child.hasAttribute('data-popper-placement');
115-
if (child.id !== this.backdropId && !isPopperElement) {
116-
hide ? child.setAttribute('aria-hidden', '' + hide) : child.removeAttribute('aria-hidden');
120+
121+
if (hide) {
122+
const stack = Modal.getStackForTarget(target);
123+
if (stack.indexOf(this.backdropId) === -1) {
124+
stack.push(this.backdropId);
125+
}
126+
} else {
127+
const stack = Modal.openModalStacks.get(target);
128+
if (!stack) {
129+
return;
130+
}
131+
const idx = stack.indexOf(this.backdropId);
132+
if (idx !== -1) {
133+
stack.splice(idx, 1);
134+
}
135+
if (stack.length === 0) {
136+
Modal.openModalStacks.delete(target);
117137
}
118138
}
139+
140+
const stack = Modal.openModalStacks.get(target);
141+
const activeBackdropId = stack?.length ? stack[stack.length - 1] : null;
142+
143+
for (const child of Array.from(target.children)) {
144+
if (child.hasAttribute('data-popper-placement')) {
145+
continue;
146+
}
147+
const shouldHide = activeBackdropId && child.id !== activeBackdropId;
148+
shouldHide ? child.setAttribute('aria-hidden', 'true') : child.removeAttribute('aria-hidden');
149+
}
119150
};
120151

121152
isEmpty = (value: string | null | undefined) => value === null || value === undefined || value === '';
@@ -140,8 +171,10 @@ class Modal extends Component<ModalProps, ModalState> {
140171
this.toggleSiblingsFromScreenReaders(true);
141172
} else {
142173
if (prevProps.isOpen !== this.props.isOpen) {
143-
target.classList.remove(css(styles.backdropOpen));
144174
this.toggleSiblingsFromScreenReaders(false);
175+
if (!Modal.openModalStacks.has(target)) {
176+
target.classList.remove(css(styles.backdropOpen));
177+
}
145178
}
146179
}
147180
}
@@ -150,8 +183,10 @@ class Modal extends Component<ModalProps, ModalState> {
150183
const { appendTo } = this.props;
151184
const target: HTMLElement = this.getElement(appendTo);
152185
target.removeEventListener('keydown', this.handleEscKeyClick, false);
153-
target.classList.remove(css(styles.backdropOpen));
154186
this.toggleSiblingsFromScreenReaders(false);
187+
if (!Modal.openModalStacks.has(target)) {
188+
target.classList.remove(css(styles.backdropOpen));
189+
}
155190
}
156191

157192
render() {

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

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,28 @@ const ModalWithAdjacentModal = () => {
6464
);
6565
};
6666

67+
const MultipleOpenModals = () => {
68+
const [isFirstOpen, setIsFirstOpen] = useState(true);
69+
const [isSecondOpen, setIsSecondOpen] = useState(false);
70+
71+
return (
72+
<>
73+
<aside>Aside sibling</aside>
74+
<Modal isOpen={isFirstOpen} appendTo={target} onClose={() => setIsFirstOpen(false)} aria-label="First modal">
75+
<button onClick={() => setIsSecondOpen(true)}>Open second modal</button>
76+
</Modal>
77+
<Modal isOpen={isSecondOpen} appendTo={target} onClose={() => setIsSecondOpen(false)} aria-label="Second modal">
78+
Second modal content
79+
</Modal>
80+
</>
81+
);
82+
};
83+
6784
describe('Modal', () => {
85+
beforeEach(() => {
86+
Modal.openModalStacks = new Map();
87+
});
88+
6889
test('Modal creates a container element once for div', () => {
6990
render(<Modal {...props} />);
7091
expect(document.createElement).toHaveBeenCalledWith('div');
@@ -181,4 +202,135 @@ describe('Modal', () => {
181202
'pf-v6-l-bullseye'
182203
);
183204
});
205+
206+
test('backdropOpen class remains when closing one of multiple open modals', async () => {
207+
const user = userEvent.setup();
208+
209+
render(<MultipleOpenModals />, { container: document.body.appendChild(target) });
210+
211+
await user.click(screen.getByRole('button', { name: 'Open second modal' }));
212+
213+
expect(target).toHaveClass(css(styles.backdropOpen));
214+
215+
const closeButtons = screen.getAllByRole('button', { name: 'Close', hidden: true });
216+
await user.click(closeButtons[closeButtons.length - 1]);
217+
218+
expect(target).toHaveClass(css(styles.backdropOpen));
219+
});
220+
221+
test('backdropOpen class is removed when all modals are closed', async () => {
222+
const user = userEvent.setup();
223+
224+
render(<MultipleOpenModals />, { container: document.body.appendChild(target) });
225+
226+
await user.click(screen.getByRole('button', { name: 'Open second modal' }));
227+
228+
const closeButtons = screen.getAllByRole('button', { name: 'Close', hidden: true });
229+
await user.click(closeButtons[closeButtons.length - 1]);
230+
await user.click(screen.getByRole('button', { name: 'Close' }));
231+
232+
expect(target).not.toHaveClass(css(styles.backdropOpen));
233+
});
234+
235+
test('only the most recent modal does not have aria-hidden when multiple modals are open', async () => {
236+
const user = userEvent.setup();
237+
238+
render(<MultipleOpenModals />, { container: document.body.appendChild(target) });
239+
240+
const firstBackdrop = screen.getByLabelText('First modal').closest('[class*="backdrop"]');
241+
242+
await user.click(screen.getByRole('button', { name: 'Open second modal' }));
243+
244+
const secondBackdrop = screen.getByLabelText('Second modal').closest('[class*="backdrop"]');
245+
246+
expect(firstBackdrop).toHaveAttribute('aria-hidden', 'true');
247+
expect(secondBackdrop).not.toHaveAttribute('aria-hidden');
248+
});
249+
250+
test('closing the active modal reveals the previous modal', async () => {
251+
const user = userEvent.setup();
252+
253+
render(<MultipleOpenModals />, { container: document.body.appendChild(target) });
254+
255+
await user.click(screen.getByRole('button', { name: 'Open second modal' }));
256+
257+
const firstBackdrop = screen
258+
.getByLabelText('First modal', { selector: '[role="dialog"]' })
259+
.closest('[class*="backdrop"]');
260+
261+
expect(firstBackdrop).toHaveAttribute('aria-hidden', 'true');
262+
263+
const closeButtons = screen.getAllByRole('button', { name: 'Close', hidden: true });
264+
await user.click(closeButtons[closeButtons.length - 1]);
265+
266+
expect(firstBackdrop).not.toHaveAttribute('aria-hidden');
267+
});
268+
269+
test('modals with different appendTo targets have independent stacks', async () => {
270+
const user = userEvent.setup();
271+
const targetA = document.createElement('div');
272+
const targetB = document.createElement('div');
273+
document.body.appendChild(targetA);
274+
document.body.appendChild(targetB);
275+
276+
const siblingA = document.createElement('aside');
277+
siblingA.textContent = 'Sibling A';
278+
targetA.appendChild(siblingA);
279+
280+
const siblingB = document.createElement('aside');
281+
siblingB.textContent = 'Sibling B';
282+
targetB.appendChild(siblingB);
283+
284+
const DistinctTargetModals = () => {
285+
const [isAOpen, setIsAOpen] = useState(true);
286+
const [isBOpen, setIsBOpen] = useState(true);
287+
288+
return (
289+
<>
290+
<Modal isOpen={isAOpen} appendTo={targetA} onClose={() => setIsAOpen(false)} aria-label="Modal A">
291+
Modal A content
292+
</Modal>
293+
<Modal isOpen={isBOpen} appendTo={targetB} onClose={() => setIsBOpen(false)} aria-label="Modal B">
294+
Modal B content
295+
</Modal>
296+
</>
297+
);
298+
};
299+
300+
render(<DistinctTargetModals />);
301+
302+
expect(siblingA).toHaveAttribute('aria-hidden', 'true');
303+
expect(siblingB).toHaveAttribute('aria-hidden', 'true');
304+
expect(targetA).toHaveClass(css(styles.backdropOpen));
305+
expect(targetB).toHaveClass(css(styles.backdropOpen));
306+
307+
const closeButtons = screen.getAllByRole('button', { name: 'Close', hidden: true });
308+
await user.click(closeButtons[1]);
309+
310+
expect(targetB).not.toHaveClass(css(styles.backdropOpen));
311+
expect(siblingB).not.toHaveAttribute('aria-hidden');
312+
313+
expect(targetA).toHaveClass(css(styles.backdropOpen));
314+
expect(siblingA).toHaveAttribute('aria-hidden', 'true');
315+
316+
document.body.removeChild(targetA);
317+
document.body.removeChild(targetB);
318+
});
319+
320+
test('unmounting a never-opened modal with a custom target does not leak a stack entry', () => {
321+
const customTarget = document.createElement('div');
322+
document.body.appendChild(customTarget);
323+
324+
const { unmount } = render(
325+
<Modal isOpen={false} appendTo={customTarget} onClose={() => {}}>
326+
Never opened
327+
</Modal>
328+
);
329+
330+
unmount();
331+
332+
expect(Modal.openModalStacks.has(customTarget)).toBe(false);
333+
334+
document.body.removeChild(customTarget);
335+
});
184336
});

0 commit comments

Comments
 (0)