Skip to content

Commit b7e5c8b

Browse files
committed
[Modal] Fix stale aria-hidden when the container changes
1 parent 4b0aeee commit b7e5c8b

4 files changed

Lines changed: 67 additions & 2 deletions

File tree

packages/mui-material/src/Modal/Modal.test.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -799,6 +799,29 @@ describe('<Modal />', () => {
799799
setProps({ anchorEl: document.body });
800800
});
801801

802+
it('should remove aria-hidden from the previous container when the container changes', () => {
803+
const root = document.createElement('div');
804+
root.setAttribute('id', 'root');
805+
document.body.appendChild(root);
806+
807+
try {
808+
const { setProps } = render(
809+
<Modal open>
810+
<div data-testid="modal-content" />
811+
</Modal>,
812+
);
813+
814+
// The modal is mounted in the body, making #root an aria-hidden sibling.
815+
expect(root).toBeInaccessible();
816+
817+
// Moving the modal into #root must remove the stale aria-hidden from it.
818+
setProps({ container: root });
819+
expect(root).not.toBeInaccessible();
820+
} finally {
821+
document.body.removeChild(root);
822+
}
823+
});
824+
802825
it('should finish closing when the container changes during the exit transition', () => {
803826
function TestCase(props) {
804827
const firstContainerRef = React.useRef(null);

packages/mui-material/src/Modal/ModalManager.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,5 +438,36 @@ describe('ModalManager', () => {
438438
expect(container2.children[1]).toBeInaccessible();
439439
expect(container2.children[2]).not.toBeInaccessible();
440440
});
441+
442+
it('should restore aria-hidden when a modal is re-registered to a nested container', () => {
443+
const containerA = document.createElement('div');
444+
const containerB = document.createElement('div');
445+
const sibling = document.createElement('div');
446+
const modalRef = document.createElement('div');
447+
containerA.appendChild(sibling);
448+
containerA.appendChild(containerB);
449+
containerA.appendChild(modalRef);
450+
document.body.appendChild(containerA);
451+
452+
try {
453+
const modal = { mount: containerA, modalRef };
454+
modalManager.add(modal, containerA);
455+
modalManager.mount(modal, {});
456+
expect(containerB).toBeInaccessible();
457+
expect(sibling).toBeInaccessible();
458+
459+
// Simulates the portal having moved into containerB before the manager is notified.
460+
modal.mount = containerB;
461+
462+
modalManager.remove(modal);
463+
modalManager.add(modal, containerB);
464+
465+
expect(containerB).not.toBeInaccessible();
466+
expect(sibling).not.toBeInaccessible();
467+
expect(modalRef).not.toBeInaccessible();
468+
} finally {
469+
document.body.removeChild(containerA);
470+
}
471+
});
441472
});
442473
});

packages/mui-material/src/Modal/ModalManager.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ export class ModalManager {
268268

269269
ariaHiddenSiblings(
270270
containerInfo.container,
271-
modal.mount,
271+
containerInfo.container,
272272
modal.modalRef,
273273
containerInfo.hiddenSiblings,
274274
false,

packages/mui-material/src/Modal/useModal.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ function useModal(parameters: UseModalParameters): UseModalReturnValue {
4646
const mountNodeRef = React.useRef<HTMLElement>(null);
4747
const lastMountNodeRef = React.useRef<HTMLElement>(null);
4848
const modalRef = React.useRef<HTMLDivElement>(null);
49+
const registeredContainerRef = React.useRef<HTMLElement>(null);
4950
const handleRef = useForkRef(modalRef, rootRef);
5051
const [exited, setExited] = React.useState(!open);
5152
const hasTransition = getHasTransition(children);
@@ -74,7 +75,16 @@ function useModal(parameters: UseModalParameters): UseModalReturnValue {
7475
const handleOpen = useEventCallback(() => {
7576
const resolvedContainer = getContainer(container) || getDoc().body;
7677

78+
// When the container changes while the modal stays open, the previous container
79+
// keeps the `aria-hidden` state computed for the previous sibling set.
80+
// Unregister the modal first so the new container is evaluated against its own siblings.
81+
if (registeredContainerRef.current && registeredContainerRef.current !== resolvedContainer) {
82+
manager.remove(getModal(), ariaHiddenProp);
83+
registeredContainerRef.current = null;
84+
}
85+
7786
manager.add(getModal(), resolvedContainer as HTMLElement);
87+
registeredContainerRef.current = resolvedContainer as HTMLElement;
7888

7989
// The element was already mounted.
8090
if (modalRef.current) {
@@ -101,6 +111,7 @@ function useModal(parameters: UseModalParameters): UseModalReturnValue {
101111
});
102112

103113
const handleClose = React.useCallback(() => {
114+
registeredContainerRef.current = null;
104115
manager.remove(getModal(), ariaHiddenProp);
105116
}, [ariaHiddenProp]);
106117

@@ -116,7 +127,7 @@ function useModal(parameters: UseModalParameters): UseModalReturnValue {
116127
} else if (!hasTransition || !closeAfterTransition) {
117128
handleClose();
118129
}
119-
}, [open, handleClose, hasTransition, closeAfterTransition, handleOpen]);
130+
}, [open, handleClose, hasTransition, closeAfterTransition, handleOpen, container]);
120131

121132
const createHandleKeyDown = (otherHandlers: EventHandlers) => (event: React.KeyboardEvent) => {
122133
otherHandlers.onKeyDown?.(event);

0 commit comments

Comments
 (0)