Skip to content

Commit a0cc05b

Browse files
alirezamirianAlireza Miriansnowystinger
authored
fix(overlays): fix an edge case in hiding outside elements (#9365)
* fix(overlays): fix an edge case in hiding outside elements When working with WYSIWYG editors or similar components that mutate the dom, the order or dome mutations could be in a way that dom mutation observer is called for elements that are not yet appended to the modal dom at the time MutationObserver callback is called. This change adds a safe-guard for such cases, by checking if the target element is connected, as otherwise it will be considered outside the currently visible elements. * comment clarification --------- Co-authored-by: Alireza Mirian <saym@danskebank.com> Co-authored-by: Robert Snow <snowystinger@gmail.com>
1 parent c0101c3 commit a0cc05b

File tree

2 files changed

+77
-2
lines changed

2 files changed

+77
-2
lines changed

packages/@react-aria/overlays/src/ariaHideOutside.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,12 @@ export function ariaHideOutside(targets: Element[], options?: AriaHideOutsideOpt
147147

148148
// If the parent element of the added nodes is not within one of the targets,
149149
// and not already inside a hidden node, hide all of the new children.
150-
if (![...visibleNodes, ...hiddenNodes].some(node => node.contains(change.target))) {
150+
if (
151+
change.target.isConnected &&
152+
![...visibleNodes, ...hiddenNodes].some((node) =>
153+
node.contains(change.target)
154+
)
155+
) {
151156
for (let node of change.addedNodes) {
152157
if (
153158
(node instanceof HTMLElement || node instanceof SVGElement) &&

packages/@react-aria/overlays/test/ariaHideOutside.test.js

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
import {act, render, waitFor} from '@react-spectrum/test-utils-internal';
1414
import {ariaHideOutside} from '../src';
15-
import React, {useState} from 'react';
15+
import React, {useRef, useState} from 'react';
1616

1717
describe('ariaHideOutside', function () {
1818
it('should hide everything except the provided element [button]', function () {
@@ -275,6 +275,76 @@ describe('ariaHideOutside', function () {
275275
expect(() => getByTestId('test')).not.toThrow();
276276
});
277277

278+
it('should handle when a new element is added and then reparented', async function () {
279+
280+
let Test = () => {
281+
const ref = useRef(null);
282+
const mutate = () => {
283+
let parent = document.createElement('ul');
284+
let child = document.createElement('li');
285+
ref.current.append(parent);
286+
parent.appendChild(child);
287+
parent.remove(); // this results in a mutation record for a disconnected ul with a connected li (through the new ul parent) in `addedNodes`
288+
let newParent = document.createElement('ul');
289+
newParent.appendChild(child);
290+
ref.current.append(newParent);
291+
};
292+
293+
return (
294+
<>
295+
<div data-testid="test" ref={ref}>
296+
<button onClick={mutate}>Mutate</button>
297+
</div>
298+
</>
299+
);
300+
};
301+
302+
let {queryByRole, getAllByRole, getByTestId} = render(<Test />);
303+
304+
ariaHideOutside([getByTestId('test')]);
305+
306+
queryByRole('button').click();
307+
await Promise.resolve(); // Wait for mutation observer tick
308+
309+
expect(getAllByRole('listitem')).toHaveLength(1);
310+
});
311+
312+
it('should handle when a new element is added and then reparented to a hidden container', async function () {
313+
314+
let Test = () => {
315+
const ref = useRef(null);
316+
const mutate = () => {
317+
let parent = document.createElement('ul');
318+
let child = document.createElement('li');
319+
ref.current.append(parent);
320+
parent.appendChild(child);
321+
parent.remove(); // this results in a mutation record for a disconnected ul with a connected li (through the new ul parent) in `addedNodes`
322+
let newParent = document.createElement('ul');
323+
newParent.appendChild(child);
324+
ref.current.append(newParent);
325+
};
326+
327+
return (
328+
<>
329+
<div data-testid="test">
330+
<button onClick={mutate}>Mutate</button>
331+
</div>
332+
<div data-testid="sibling" ref={ref} />
333+
</>
334+
);
335+
};
336+
337+
let {queryByRole, queryAllByRole, getByTestId} = render(<Test />);
338+
339+
ariaHideOutside([getByTestId('test')]);
340+
341+
queryByRole('button').click();
342+
await Promise.resolve(); // Wait for mutation observer tick
343+
344+
expect(queryAllByRole('listitem')).toHaveLength(0);
345+
});
346+
347+
278348
it('work when called multiple times', function () {
279349
let {getByRole, getAllByRole} = render(
280350
<>

0 commit comments

Comments
 (0)