Skip to content

Commit

Permalink
fix[devtools/useModalDismissSignal]: use getRootNode for shadow root …
Browse files Browse the repository at this point in the history
…case support (#28145)

In our custom implementation for handling modals dismiss signal, we use
element's `ownerDocument` field, which expectedly doesn't work well with
shadow root. Now using
[`getRootNode`](https://developer.mozilla.org/en-US/docs/Web/API/Node/getRootNode)
instead of `ownerDocument` to support shadow root case.

Without this, if RDT Frontend is hosted inside the shadow root, the
modal gets closed after any click, including on the buttons hosted by
modal:

https://github.com/facebook/react/blob/00d42ac3542179c55f936f395ede7abaeb5900a3/packages/react-devtools-shared/src/devtools/views/hooks.js#L228-L238

Test plan:
- Modals work as expected for Chrome DevTools integration
- Modals work as expected at every other surfaces: browser extension,
electron wrapper for RN, inline version for web
  • Loading branch information
hoxyq authored Feb 12, 2024
1 parent 269edb8 commit 947e796
Showing 1 changed file with 14 additions and 11 deletions.
25 changes: 14 additions & 11 deletions packages/react-devtools-shared/src/devtools/views/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,15 +219,18 @@ export function useModalDismissSignal(
return () => {};
}

const handleDocumentKeyDown = (event: any) => {
const handleRootNodeKeyDown = (event: KeyboardEvent) => {
if (event.key === 'Escape') {
dismissCallback();
}
};

const handleDocumentClick = (event: any) => {
const handleRootNodeClick: MouseEventHandler = event => {
if (
modalRef.current !== null &&
/* $FlowExpectedError[incompatible-call] Instead of dealing with possibly multiple realms
and multiple Node references to comply with Flow (e.g. checking with `event.target instanceof Node`)
just delegate it to contains call */
!modalRef.current.contains(event.target)
) {
event.stopPropagation();
Expand All @@ -237,7 +240,7 @@ export function useModalDismissSignal(
}
};

let ownerDocument = null;
let modalRootNode = null;

// Delay until after the current call stack is empty,
// in case this effect is being run while an event is currently bubbling.
Expand All @@ -248,12 +251,12 @@ export function useModalDismissSignal(
// It's important to listen to the ownerDocument to support the browser extension.
// Here we use portals to render individual tabs (e.g. Profiler),
// and the root document might belong to a different window.
const div = modalRef.current;
if (div != null) {
ownerDocument = div.ownerDocument;
ownerDocument.addEventListener('keydown', handleDocumentKeyDown);
const modalDOMElement = modalRef.current;
if (modalDOMElement != null) {
modalRootNode = modalDOMElement.getRootNode();
modalRootNode.addEventListener('keydown', handleRootNodeKeyDown);
if (dismissOnClickOutside) {
ownerDocument.addEventListener('click', handleDocumentClick, true);
modalRootNode.addEventListener('click', handleRootNodeClick, true);
}
}
}, 0);
Expand All @@ -263,9 +266,9 @@ export function useModalDismissSignal(
clearTimeout(timeoutID);
}

if (ownerDocument !== null) {
ownerDocument.removeEventListener('keydown', handleDocumentKeyDown);
ownerDocument.removeEventListener('click', handleDocumentClick, true);
if (modalRootNode !== null) {
modalRootNode.removeEventListener('keydown', handleRootNodeKeyDown);
modalRootNode.removeEventListener('click', handleRootNodeClick, true);
}
};
}, [modalRef, dismissCallback, dismissOnClickOutside]);
Expand Down

0 comments on commit 947e796

Please sign in to comment.