Skip to content

Commit 9795d8c

Browse files
refactor: generic getShadowRoots + addEvent for non-composed events
Address PR review feedback. Replace the scroll-specific addGlobalScrollListener with a generic getShadowRoots(node) helper plus the addEvent utility, since the shadow-DOM event-propagation problem applies to all non-composed events, not just scroll. addEvent and the EventMapType/EventTargetType type helpers are pulled in from #10102 verbatim and placed at the same locations so the overlap auto-resolves when #10102 lands. Callers (ScrollView, both useCloseOnScroll copies) now use: addEvent(window/document, 'scroll', listener, true) addEvent(getShadowRoots(ref.current), 'scroll', listener, true) Select.browser.test now opens the ComboBox via the @react-aria/test-utils combobox tester instead of dispatching raw PointerEvents. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a2dabe0 commit 9795d8c

9 files changed

Lines changed: 100 additions & 69 deletions

File tree

packages/@adobe/react-spectrum/src/menu/useCloseOnScroll.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,10 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13+
import {addEvent} from 'react-aria/private/utils/domHelpers';
1314
import {
14-
addGlobalScrollListener,
1515
getEventTarget,
16+
getShadowRoots,
1617
nodeContains
1718
} from 'react-aria/private/utils/shadowdom/DOMFunctions';
1819
import {RefObject} from '@react-types/shared';
@@ -67,6 +68,11 @@ export function useCloseOnScroll(opts: CloseOnScrollOptions): void {
6768
}
6869
};
6970

70-
return addGlobalScrollListener(window, triggerRef.current, onScroll, true);
71+
let cleanupGlobal = addEvent(window, 'scroll', onScroll, true);
72+
let cleanupShadow = addEvent(getShadowRoots(triggerRef.current), 'scroll', onScroll, true);
73+
return () => {
74+
cleanupGlobal();
75+
cleanupShadow();
76+
};
7177
}, [isOpen, onClose, triggerRef]);
7278
}

packages/@react-types/shared/src/events.d.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,20 @@
1313
import {FocusableElement} from './dom';
1414
import {FocusEvent, MouseEvent, KeyboardEvent as ReactKeyboardEvent, SyntheticEvent} from 'react';
1515

16+
// Type helper to extract the target element type from an event
17+
export type EventTargetType<T> = T extends SyntheticEvent<infer E, any> ? E : EventTarget;
18+
19+
// Type helper to extract the event map from a target
20+
export type EventMapType<T extends EventTarget> = T extends Window
21+
? WindowEventMap
22+
: T extends Document
23+
? DocumentEventMap
24+
: T extends Element
25+
? HTMLElementEventMap
26+
: T extends VisualViewport
27+
? VisualViewportEventMap
28+
: GlobalEventHandlersEventMap;
29+
1630
// Event bubbling can be problematic in real-world applications, so the default for React Spectrum components
1731
// is not to propagate. This can be overridden by calling continuePropagation() on the event.
1832
export type BaseEvent<T extends SyntheticEvent> = T & {

packages/react-aria-components/test/Select.browser.test.tsx

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {Label} from '../src/Label';
2828
import {ListBox, ListBoxItem} from '../src/ListBox';
2929
import {Popover} from '../src/Popover';
3030
import React from 'react';
31+
import {User} from '@react-aria/test-utils';
3132

3233
function TestComboBox() {
3334
return (
@@ -57,36 +58,26 @@ function makeScrollableContainer() {
5758
return {scrollable, mountPoint};
5859
}
5960

60-
async function openComboBox(container: Element) {
61-
let button = container.querySelector('button') as HTMLButtonElement;
62-
button.dispatchEvent(
63-
new PointerEvent('pointerdown', {bubbles: true, cancelable: true, isPrimary: true})
64-
);
65-
button.dispatchEvent(
66-
new PointerEvent('pointerup', {bubbles: true, cancelable: true, isPrimary: true})
67-
);
68-
button.dispatchEvent(new MouseEvent('click', {bubbles: true, cancelable: true}));
69-
await new Promise<void>(resolve => setTimeout(resolve, 100));
70-
}
71-
7261
it('overlay closes when a scrollable light DOM ancestor scrolls', async () => {
62+
let testUtilUser = new User();
7363
let {scrollable, mountPoint} = makeScrollableContainer();
7464
document.body.appendChild(scrollable);
7565

7666
let root = createRoot(mountPoint);
7767
root.render(<TestComboBox />);
7868
await new Promise<void>(resolve => setTimeout(resolve, 100));
7969

80-
await openComboBox(scrollable);
70+
let comboboxTester = testUtilUser.createTester('ComboBox', {root: scrollable});
71+
await comboboxTester.open();
8172

8273
// ComboBox listbox renders into document.body via portal.
83-
expect(document.querySelector('[role="listbox"]')).not.toBeNull();
74+
expect(comboboxTester.getListbox()).not.toBeNull();
8475

8576
// Scroll the ancestor that contains the trigger — window capturing listener should close the overlay.
8677
scrollable.dispatchEvent(new Event('scroll'));
8778
await new Promise<void>(resolve => setTimeout(resolve, 100));
8879

89-
expect(document.querySelector('[role="listbox"]')).toBeNull();
80+
expect(comboboxTester.getListbox()).toBeNull();
9081

9182
root.unmount();
9283
document.body.removeChild(scrollable);
@@ -101,6 +92,7 @@ describe('Shadow DOM', () => {
10192
enableShadowDOM();
10293

10394
it('overlay closes when a scrollable shadow DOM ancestor scrolls', async () => {
95+
let testUtilUser = new User();
10496
let outerHost = document.createElement('div');
10597
document.body.appendChild(outerHost);
10698
let shadowRoot = outerHost.attachShadow({mode: 'open'});
@@ -112,18 +104,19 @@ describe('Shadow DOM', () => {
112104
root.render(<TestComboBox />);
113105
await new Promise<void>(resolve => setTimeout(resolve, 100));
114106

115-
await openComboBox(scrollable);
107+
let comboboxTester = testUtilUser.createTester('ComboBox', {root: scrollable});
108+
await comboboxTester.open();
116109

117110
// Listbox renders into document.body via portal even in shadow DOM mode.
118-
expect(document.querySelector('[role="listbox"]')).not.toBeNull();
111+
expect(comboboxTester.getListbox()).not.toBeNull();
119112

120113
// Scroll inside the shadow root.
121114
// Without the fix, window never sees this event (composed: false).
122-
// With the fix (addGlobalScrollListener), the shadow root listener closes the overlay.
115+
// With the fix (getShadowRoots + addEvent), the shadow root listener closes the overlay.
123116
scrollable.dispatchEvent(new Event('scroll'));
124117
await new Promise<void>(resolve => setTimeout(resolve, 100));
125118

126-
expect(document.querySelector('[role="listbox"]')).toBeNull();
119+
expect(comboboxTester.getListbox()).toBeNull();
127120

128121
root.unmount();
129122
document.body.removeChild(outerHost);
Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,6 @@
1-
export {getOwnerDocument, getOwnerWindow, isShadowRoot} from '../../../src/utils/domHelpers';
1+
export {
2+
addEvent,
3+
getOwnerDocument,
4+
getOwnerWindow,
5+
isShadowRoot
6+
} from '../../../src/utils/domHelpers';

packages/react-aria/exports/private/utils/shadowdom/DOMFunctions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
export {
2-
addGlobalScrollListener,
32
getEventTarget,
3+
getShadowRoots,
44
nodeContains,
55
isFocusWithin,
66
getActiveElement

packages/react-aria/src/overlays/useCloseOnScroll.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,8 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13-
import {
14-
addGlobalScrollListener,
15-
getEventTarget,
16-
nodeContains
17-
} from '../utils/shadowdom/DOMFunctions';
13+
import {addEvent} from '../utils/domHelpers';
14+
import {getEventTarget, getShadowRoots, nodeContains} from '../utils/shadowdom/DOMFunctions';
1815
import {RefObject} from '@react-types/shared';
1916
import {useEffect} from 'react';
2017

@@ -64,6 +61,11 @@ export function useCloseOnScroll(opts: CloseOnScrollOptions): void {
6461
}
6562
};
6663

67-
return addGlobalScrollListener(window, triggerRef.current, onScroll, true);
64+
let cleanupGlobal = addEvent(window, 'scroll', onScroll, true);
65+
let cleanupShadow = addEvent(getShadowRoots(triggerRef.current), 'scroll', onScroll, true);
66+
return () => {
67+
cleanupGlobal();
68+
cleanupShadow();
69+
};
6870
}, [isOpen, onClose, triggerRef]);
6971
}

packages/react-aria/src/utils/domHelpers.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import type {EventMapType} from '@react-types/shared';
2+
13
export const getOwnerDocument = (el: Element | null | undefined): Document => {
24
return el?.ownerDocument ?? document;
35
};
@@ -32,3 +34,29 @@ function isNode(value: unknown): value is Node {
3234
export function isShadowRoot(node: Node | null): node is ShadowRoot {
3335
return isNode(node) && node.nodeType === Node.DOCUMENT_FRAGMENT_NODE && 'host' in node;
3436
}
37+
38+
/**
39+
* Attaches an event listener on target(s) and returns a cleanup function.
40+
*/
41+
export function addEvent<T extends EventTarget, K extends keyof EventMapType<Exclude<T, null>>>(
42+
target: T | EventTarget[] | null,
43+
event: Extract<K, string> | (string & {}),
44+
listener?: (this: T, ev: EventMapType<Exclude<T, null>>[K]) => any,
45+
options?: boolean | AddEventListenerOptions
46+
): () => void {
47+
if (listener == null || target == null) {
48+
return () => {};
49+
}
50+
51+
let eventTargets = Array.isArray(target) ? target : [target];
52+
53+
for (let eventTarget of eventTargets) {
54+
eventTarget.addEventListener(event, listener as EventListener, options);
55+
}
56+
57+
return () => {
58+
for (let eventTarget of eventTargets) {
59+
eventTarget.removeEventListener(event, listener as EventListener, options);
60+
}
61+
};
62+
}

packages/react-aria/src/utils/shadowdom/DOMFunctions.ts

Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -84,43 +84,25 @@ export function getEventTarget<T extends Event | SyntheticEvent>(event: T): Even
8484
}
8585

8686
/**
87-
* Adds a scroll listener to a global target (window or document).
88-
* When shadow DOM mode is enabled, also attaches a capturing listener to each
89-
* shadow root in the ancestor chain of refElement, since scroll events have
90-
* composed: false and do not propagate out of shadow roots.
87+
* Collects the enclosing ShadowRoots between a node and the document.
9188
*
92-
* Returns a cleanup function that removes all attached listeners.
89+
* Useful for attaching listeners for events that don't compose (e.g. scroll),
90+
* since those events do not propagate out of shadow roots even in the capture phase.
9391
*/
94-
export function addGlobalScrollListener(
95-
globalTarget: Window | Document,
96-
refElement: Element | null,
97-
listener: EventListener,
98-
options?: boolean | AddEventListenerOptions
99-
): () => void {
100-
globalTarget.addEventListener('scroll', listener, options);
101-
102-
let shadowRoots: ShadowRoot[] = [];
103-
if (shadowDOM() && refElement) {
104-
let node: Node | null = refElement;
105-
while (node) {
106-
if (isShadowRoot(node)) {
107-
shadowRoots.push(node as ShadowRoot);
108-
node = (node as ShadowRoot).host;
109-
} else {
110-
node = node.parentNode;
111-
}
112-
}
113-
for (let root of shadowRoots) {
114-
root.addEventListener('scroll', listener, options);
115-
}
92+
export function getShadowRoots(node: Node | null | undefined): ShadowRoot[] {
93+
if (!shadowDOM()) {
94+
return [];
11695
}
11796

118-
return () => {
119-
globalTarget.removeEventListener('scroll', listener, options);
120-
for (let root of shadowRoots) {
121-
root.removeEventListener('scroll', listener, options);
122-
}
123-
};
97+
let roots: ShadowRoot[] = [];
98+
let current: Node | null = node?.getRootNode() ?? null;
99+
100+
while (isShadowRoot(current)) {
101+
roots.push(current);
102+
current = current.host.getRootNode();
103+
}
104+
105+
return roots;
124106
}
125107

126108
/**

packages/react-aria/src/virtualizer/ScrollView.tsx

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,9 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13-
import {
14-
addGlobalScrollListener,
15-
getEventTarget,
16-
nodeContains
17-
} from '../utils/shadowdom/DOMFunctions';
18-
13+
import {addEvent} from '../utils/domHelpers';
1914
import {flushSync} from 'react-dom';
15+
import {getEventTarget, getShadowRoots, nodeContains} from '../utils/shadowdom/DOMFunctions';
2016
import {getScrollLeft} from './utils';
2117
import {Point, Rect, Size} from 'react-stately/useVirtualizerState';
2218
import React, {
@@ -225,7 +221,12 @@ export function useScrollView(
225221
// When inside a shadow DOM, also attach to each shadow root in the ancestor chain since scroll
226222
// events have composed: false and don't propagate out of shadow roots.
227223
useEffect(() => {
228-
return addGlobalScrollListener(document, ref.current, onScroll, true);
224+
let cleanupGlobal = addEvent(document, 'scroll', onScroll, true);
225+
let cleanupShadow = addEvent(getShadowRoots(ref.current), 'scroll', onScroll, true);
226+
return () => {
227+
cleanupGlobal();
228+
cleanupShadow();
229+
};
229230
}, [onScroll, ref]);
230231

231232
useEffect(() => {

0 commit comments

Comments
 (0)