Skip to content

Commit

Permalink
DevTools: useModalDismissSignal bugfix (facebook#21173)
Browse files Browse the repository at this point in the history
* DevTools: useModalDismissSignal bugfix

Make useModalDismissSignal's manually added click/keyboard events more robust to sync flushed passive effects. (Don't let the same click event that shows a modal dialog also dismiss it.)

* Replaced event.timeStamp check with setTimeout
  • Loading branch information
Brian Vaughn authored and acdlite committed Apr 16, 2021
1 parent 58c42ed commit 9ff6f4c
Showing 1 changed file with 27 additions and 13 deletions.
40 changes: 27 additions & 13 deletions packages/react-devtools-shared/src/devtools/views/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,14 +207,13 @@ export function useModalDismissSignal(
return () => {};
}

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

const handleDocumentClick = (event: any) => {
// $FlowFixMe
if (
modalRef.current !== null &&
!modalRef.current.contains(event.target)
Expand All @@ -226,18 +225,33 @@ 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 ownerDocument = modalRef.current.ownerDocument;
ownerDocument.addEventListener('keydown', handleDocumentKeyDown);
if (dismissOnClickOutside) {
ownerDocument.addEventListener('click', handleDocumentClick);
}
let ownerDocument = null;

// Delay until after the current call stack is empty,
// in case this effect is being run while an event is currently bubbling.
// In that case, we don't want to listen to the pre-existing event.
let timeoutID = setTimeout(() => {
timeoutID = null;

// 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.
ownerDocument = ((modalRef.current: any): HTMLDivElement).ownerDocument;
ownerDocument.addEventListener('keydown', handleDocumentKeyDown);
if (dismissOnClickOutside) {
ownerDocument.addEventListener('click', handleDocumentClick);
}
}, 0);

return () => {
ownerDocument.removeEventListener('keydown', handleDocumentKeyDown);
ownerDocument.removeEventListener('click', handleDocumentClick);
if (timeoutID !== null) {
clearTimeout(timeoutID);
}

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

0 comments on commit 9ff6f4c

Please sign in to comment.