From aa03beec4c227c84fbe81afad97a5db92b245934 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 3 Apr 2021 11:09:39 -0400 Subject: [PATCH 1/2] 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.) --- .../src/devtools/views/hooks.js | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/hooks.js b/packages/react-devtools-shared/src/devtools/views/hooks.js index 3af0ceaeb29..676400f4e45 100644 --- a/packages/react-devtools-shared/src/devtools/views/hooks.js +++ b/packages/react-devtools-shared/src/devtools/views/hooks.js @@ -207,13 +207,25 @@ export function useModalDismissSignal( return () => {}; } - const handleDocumentKeyDown = ({key}: any) => { - if (key === 'Escape') { + const timeOfEffect = performance.now(); + + const handleDocumentKeyDown = (event: any) => { + if (timeOfEffect > event.timeStamp) { + // Don't let the same event that showed the dialog also hide it. + return; + } + + if (event.key === 'Escape') { dismissCallback(); } }; const handleDocumentClick = (event: any) => { + if (timeOfEffect > event.timeStamp) { + // Don't let the same event that showed the dialog also hide it. + return; + } + // $FlowFixMe if ( modalRef.current !== null && @@ -229,7 +241,8 @@ 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; + const ownerDocument = ((modalRef.current: any): HTMLDivElement) + .ownerDocument; ownerDocument.addEventListener('keydown', handleDocumentKeyDown); if (dismissOnClickOutside) { ownerDocument.addEventListener('click', handleDocumentClick); From 8f1087eb970f8b6f9c472c0591bcbbd273c04b7a Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 5 Apr 2021 10:41:26 -0400 Subject: [PATCH 2/2] Replaced event.timeStamp check with setTimeout --- .../src/devtools/views/hooks.js | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/hooks.js b/packages/react-devtools-shared/src/devtools/views/hooks.js index 676400f4e45..54b1d7d20ce 100644 --- a/packages/react-devtools-shared/src/devtools/views/hooks.js +++ b/packages/react-devtools-shared/src/devtools/views/hooks.js @@ -207,26 +207,13 @@ export function useModalDismissSignal( return () => {}; } - const timeOfEffect = performance.now(); - const handleDocumentKeyDown = (event: any) => { - if (timeOfEffect > event.timeStamp) { - // Don't let the same event that showed the dialog also hide it. - return; - } - if (event.key === 'Escape') { dismissCallback(); } }; const handleDocumentClick = (event: any) => { - if (timeOfEffect > event.timeStamp) { - // Don't let the same event that showed the dialog also hide it. - return; - } - - // $FlowFixMe if ( modalRef.current !== null && !modalRef.current.contains(event.target) @@ -238,19 +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: any): HTMLDivElement) - .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]); }