Skip to content

Commit f3bbba5

Browse files
committed
Added close cleanup tweaks
1 parent 6106b3d commit f3bbba5

2 files changed

Lines changed: 33 additions & 7 deletions

File tree

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,19 +117,28 @@ class Modal extends Component<ModalProps, ModalState> {
117117
toggleSiblingsFromScreenReaders = (hide: boolean) => {
118118
const { appendTo } = this.props;
119119
const target: HTMLElement = this.getElement(appendTo);
120-
const stack = Modal.getStackForTarget(target);
121-
const idx = stack.indexOf(this.backdropId);
122120

123-
if (hide && idx === -1) {
124-
stack.push(this.backdropId);
125-
} else if (!hide && idx !== -1) {
126-
stack.splice(idx, 1);
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+
}
127135
if (stack.length === 0) {
128136
Modal.openModalStacks.delete(target);
129137
}
130138
}
131139

132-
const activeBackdropId = stack.length > 0 ? stack[stack.length - 1] : null;
140+
const stack = Modal.openModalStacks.get(target);
141+
const activeBackdropId = stack?.length ? stack[stack.length - 1] : null;
133142

134143
for (const child of Array.from(target.children)) {
135144
if (child.hasAttribute('data-popper-placement')) {

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,4 +316,21 @@ describe('Modal', () => {
316316
document.body.removeChild(targetA);
317317
document.body.removeChild(targetB);
318318
});
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+
});
319336
});

0 commit comments

Comments
 (0)