From a249e49ddbf4a45e64c3ba27e05d0ae6a446c6e0 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 9 Aug 2022 16:48:22 -0700 Subject: [PATCH 01/18] Fix no blur called on activeelement removed from dom --- packages/@react-aria/focus/src/FocusScope.tsx | 40 ++++----- .../focus/stories/FocusScope.stories.tsx | 18 ++--- .../@react-aria/focus/test/FocusScope.test.js | 81 +++++++++---------- .../@react-aria/interactions/src/utils.ts | 34 +++++++- 4 files changed, 101 insertions(+), 72 deletions(-) diff --git a/packages/@react-aria/focus/src/FocusScope.tsx b/packages/@react-aria/focus/src/FocusScope.tsx index a86d985d63e..045b9c13cec 100644 --- a/packages/@react-aria/focus/src/FocusScope.tsx +++ b/packages/@react-aria/focus/src/FocusScope.tsx @@ -13,8 +13,9 @@ import {FocusableElement} from '@react-types/shared'; import {focusSafely} from './focusSafely'; import {isElementVisible} from './isElementVisible'; -import React, {ReactNode, RefObject, useContext, useEffect, useRef} from 'react'; +import React, {ReactNode, RefObject, useCallback, useContext, useEffect, useRef} from 'react'; import {useLayoutEffect} from '@react-aria/utils'; +import {useSyntheticBlurEvent} from '@react-aria/interactions/src/utils'; export interface FocusScopeProps { @@ -254,6 +255,23 @@ function useFocusContainment(scopeRef: RefObject, contain: boolean) { let focusedNode = useRef(); let raf = useRef(null); + + let onBlur = useCallback((e) => { + // Firefox doesn't shift focus back to the Dialog properly without this + raf.current = requestAnimationFrame(() => { + // Use document.activeElement instead of e.relatedTarget so we can tell if user clicked into iframe + if (shouldContainFocus(scopeRef) && !isElementInChildScope(document.activeElement, scopeRef)) { + activeScope = scopeRef; + if (document.body.contains(e.target)) { + focusedNode.current = e.target; + focusedNode.current.focus(); + } else if (activeScope) { + focusFirstInScope(activeScope.current); + } + } + }); + }, [scopeRef, focusedNode]); + let onSyntheticFocus = useSyntheticBlurEvent(onBlur); useLayoutEffect(() => { let scope = scopeRef.current; if (!contain) { @@ -297,6 +315,7 @@ function useFocusContainment(scopeRef: RefObject, contain: boolean) { if (!activeScope || isAncestorScope(activeScope, scopeRef)) { activeScope = scopeRef; focusedNode.current = e.target; + onSyntheticFocus(e); } else if (shouldContainFocus(scopeRef) && !isElementInChildScope(e.target, scopeRef)) { // If a focus event occurs outside the active scope (e.g. user tabs from browser location bar), // restore focus to the previously focused node or the first tabbable element in the active scope. @@ -307,25 +326,10 @@ function useFocusContainment(scopeRef: RefObject, contain: boolean) { } } else if (shouldContainFocus(scopeRef)) { focusedNode.current = e.target; + onSyntheticFocus(e); } }; - let onBlur = (e) => { - // Firefox doesn't shift focus back to the Dialog properly without this - raf.current = requestAnimationFrame(() => { - // Use document.activeElement instead of e.relatedTarget so we can tell if user clicked into iframe - if (shouldContainFocus(scopeRef) && !isElementInChildScope(document.activeElement, scopeRef)) { - activeScope = scopeRef; - if (document.body.contains(e.target)) { - focusedNode.current = e.target; - focusedNode.current.focus(); - } else if (activeScope) { - focusFirstInScope(activeScope.current); - } - } - }); - }; - document.addEventListener('keydown', onKeyDown, false); document.addEventListener('focusin', onFocus, false); scope.forEach(element => element.addEventListener('focusin', onFocus, false)); @@ -336,7 +340,7 @@ function useFocusContainment(scopeRef: RefObject, contain: boolean) { scope.forEach(element => element.removeEventListener('focusin', onFocus, false)); scope.forEach(element => element.removeEventListener('focusout', onBlur, false)); }; - }, [scopeRef, contain]); + }, [scopeRef, contain, onBlur]); // eslint-disable-next-line arrow-body-style useEffect(() => { diff --git a/packages/@react-aria/focus/stories/FocusScope.stories.tsx b/packages/@react-aria/focus/stories/FocusScope.stories.tsx index 6158760838f..60f801fbc6f 100644 --- a/packages/@react-aria/focus/stories/FocusScope.stories.tsx +++ b/packages/@react-aria/focus/stories/FocusScope.stories.tsx @@ -10,6 +10,7 @@ * governing permissions and limitations under the License. */ +import {Button} from '@react-spectrum/button'; import {FocusScope} from '../'; import {Meta, Story} from '@storybook/react'; import React, {ReactNode, useState} from 'react'; @@ -107,31 +108,26 @@ export function Example({isPortaled, contain}: StoryProps) { function FocusableFirstInScopeExample() { let [contentIndex, setContentIndex] = useState(0); - let [buttonRemoved, setButtonRemoved] = useState(false); function DialogContent(index = 0) { - const nextIndex = index === 2 ? 0 : index + 1; + const nextIndex = index + 1; return ( - <> +

Dialog {index + 1}

- {index === 2 ? + {index > 2 ? ( <>

The end of the road.

- - {buttonRemoved && -

With no tabbable elements within the scope, FocusScope will try to focus the first focusable element within the scope, in this case, the dialog itself.

- } +

With no tabbable elements within the scope, FocusScope will try to focus the first focusable element within the scope, in this case, the dialog itself.

) : ( <>

Content that will be replaced by Dialog {nextIndex + 1}.

- + ) } - - +
); } const contents = []; diff --git a/packages/@react-aria/focus/test/FocusScope.test.js b/packages/@react-aria/focus/test/FocusScope.test.js index e7f6e0217dc..c853a8519f1 100644 --- a/packages/@react-aria/focus/test/FocusScope.test.js +++ b/packages/@react-aria/focus/test/FocusScope.test.js @@ -16,7 +16,7 @@ import {DialogContainer} from '@react-spectrum/dialog'; import {FocusScope, useFocusManager} from '../'; import {focusScopeTree} from '../src/FocusScope'; import {Provider} from '@react-spectrum/provider'; -import React, {useState} from 'react'; +import React, {useEffect, useRef, useState} from 'react'; import ReactDOM from 'react-dom'; import {Example as StorybookExample} from '../stories/FocusScope.stories'; import userEvent from '@testing-library/user-event'; @@ -651,47 +651,44 @@ describe('FocusScope', function () { }); describe('focusable first in scope', function () { - it('should restore focus to the first focusable or tabbable element within the scope when focus is lost within the scope', async function () { - let {getByTestId} = render( -
- -
- Remove me! - Remove me, too! - Remove me, three! -
- -
- ); - - function Item(props) { - let focusManager = useFocusManager(); - let onClick = e => { - focusManager.focusNext(); - act(() => { - // remove fails to fire blur event in jest-dom - e.target.blur(); - e.target.remove(); - jest.runAllTimers(); - }); - }; - return } + // {contentIndex === 1 && } + // {contentIndex > 2 && } + // + // + // + // ); + // } + // let focusable = getByTestId('focusable'); + // let tabbable1 = getByTestId('tabbable1'); + // expect(document.activeElement).toBe(tabbable1); + // + // fireEvent.click(tabbable1); + // await waitFor(() => expect(tabbable1).not.toBeInTheDocument()); + // act(() => {jest.runAllTimers();}); + // let item1 = getByTestId('item1'); + // await waitFor(() => expect(document.activeElement).toBe(item1)); + // + // fireEvent.click(item1); + // act(() => {jest.runAllTimers();}); + // expect(item1).not.toBeInTheDocument(); + // let item2 = getByTestId('item2'); + // expect(document.activeElement).toBe(item2); + // + // fireEvent.click(item2); + // act(() => {jest.runAllTimers();}); + // expect(item2).not.toBeInTheDocument(); + // expect(document.activeElement).toBe(focusable); + // }); }); }); diff --git a/packages/@react-aria/interactions/src/utils.ts b/packages/@react-aria/interactions/src/utils.ts index ca38b4c0f01..1380c868713 100644 --- a/packages/@react-aria/interactions/src/utils.ts +++ b/packages/@react-aria/interactions/src/utils.ts @@ -85,7 +85,8 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { let stateRef = useRef({ isFocused: false, onBlur, - observer: null as MutationObserver + observer: null as MutationObserver, + removalObserver: null as MutationObserver }); stateRef.current.onBlur = onBlur; @@ -97,6 +98,8 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { if (state.observer) { state.observer.disconnect(); state.observer = null; + state.removalObserver.disconnect(); + state.removalObserver = null; } }; }, []); @@ -129,6 +132,12 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { stateRef.current.observer.disconnect(); stateRef.current.observer = null; } + + // We no longer need the MutationObserver once the target is blurred. + if (stateRef.current.removalObserver) { + stateRef.current.removalObserver.disconnect(); + stateRef.current.removalObserver = null; + } }; target.addEventListener('focusout', onBlurHandler, {once: true}); @@ -141,7 +150,30 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { } }); + // Some browsers do not fire onBlur when the document.activeElement is removed from the dom. + // Firefox has had a bug open about it for 13 years https://bugzilla.mozilla.org/show_bug.cgi?id=559561 + // A Safari bug has been logged for it as well https://bugs.webkit.org/show_bug.cgi?id=243749 + stateRef.current.removalObserver = new MutationObserver((mutationList) => { + if (stateRef.current.isFocused) { + for (const mutation of mutationList) { + for (const node of mutation.removedNodes) { + if (node === target || node.contains(target)) { + stateRef.current.removalObserver.disconnect(); + // For backward compatibility, dispatch a (fake) React synthetic event. + let blurEvent = new SyntheticFocusEvent('blur', new FocusEvent('blur')); + blurEvent.target = target; + stateRef.current.onBlur?.(blurEvent); + stateRef.current.observer = null; + // early return so we don't look at the rest of the DOM + return; + } + } + } + } + }); + stateRef.current.observer.observe(target, {attributes: true, attributeFilter: ['disabled']}); + stateRef.current.removalObserver.observe(document, {childList: true, subtree: true, attributes: false, characterData: false}); } }, []); } From 0c826dd58c4570e544c4cb7fc4e97b3e9cc7da9b Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 9 Aug 2022 17:54:42 -0700 Subject: [PATCH 02/18] lint --- packages/@react-aria/focus/test/FocusScope.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@react-aria/focus/test/FocusScope.test.js b/packages/@react-aria/focus/test/FocusScope.test.js index c853a8519f1..cedf5377057 100644 --- a/packages/@react-aria/focus/test/FocusScope.test.js +++ b/packages/@react-aria/focus/test/FocusScope.test.js @@ -10,13 +10,13 @@ * governing permissions and limitations under the License. */ -import {act, fireEvent, render, waitFor} from '@react-spectrum/test-utils'; +import {act, fireEvent, render} from '@react-spectrum/test-utils'; import {defaultTheme} from '@adobe/react-spectrum'; import {DialogContainer} from '@react-spectrum/dialog'; import {FocusScope, useFocusManager} from '../'; import {focusScopeTree} from '../src/FocusScope'; import {Provider} from '@react-spectrum/provider'; -import React, {useEffect, useRef, useState} from 'react'; +import React, {useState} from 'react'; import ReactDOM from 'react-dom'; import {Example as StorybookExample} from '../stories/FocusScope.stories'; import userEvent from '@testing-library/user-event'; From b702b3a83b30c8b8505334c05915596be66f1d16 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 10 Aug 2022 19:21:56 -0700 Subject: [PATCH 03/18] Save point - get mutation observer firing correctly --- packages/@react-aria/focus/src/FocusScope.tsx | 35 +++++++- .../@react-aria/focus/test/FocusScope.test.js | 81 ++++++++++--------- .../@react-aria/interactions/src/utils.ts | 26 +++--- 3 files changed, 91 insertions(+), 51 deletions(-) diff --git a/packages/@react-aria/focus/src/FocusScope.tsx b/packages/@react-aria/focus/src/FocusScope.tsx index 045b9c13cec..f546210c90f 100644 --- a/packages/@react-aria/focus/src/FocusScope.tsx +++ b/packages/@react-aria/focus/src/FocusScope.tsx @@ -272,6 +272,7 @@ function useFocusContainment(scopeRef: RefObject, contain: boolean) { }); }, [scopeRef, focusedNode]); let onSyntheticFocus = useSyntheticBlurEvent(onBlur); + let onSyntheticFocusDocument = useSyntheticBlurEvent(onBlur); useLayoutEffect(() => { let scope = scopeRef.current; if (!contain) { @@ -330,13 +331,34 @@ function useFocusContainment(scopeRef: RefObject, contain: boolean) { } }; + let onFocusDocument = (e) => { + // If focusing an element in a child scope of the currently active scope, the child becomes active. + // Moving out of the active scope to an ancestor is not allowed. + if (!activeScope || isAncestorScope(activeScope, scopeRef)) { + activeScope = scopeRef; + focusedNode.current = e.target; + onSyntheticFocusDocument(e); + } else if (shouldContainFocus(scopeRef) && !isElementInChildScope(e.target, scopeRef)) { + // If a focus event occurs outside the active scope (e.g. user tabs from browser location bar), + // restore focus to the previously focused node or the first tabbable element in the active scope. + if (focusedNode.current) { + focusedNode.current.focus(); + } else if (activeScope) { + focusFirstInScope(activeScope.current); + } + } else if (shouldContainFocus(scopeRef)) { + focusedNode.current = e.target; + onSyntheticFocusDocument(e); + } + }; + document.addEventListener('keydown', onKeyDown, false); - document.addEventListener('focusin', onFocus, false); + document.addEventListener('focusin', onFocusDocument, false); scope.forEach(element => element.addEventListener('focusin', onFocus, false)); scope.forEach(element => element.addEventListener('focusout', onBlur, false)); return () => { document.removeEventListener('keydown', onKeyDown, false); - document.removeEventListener('focusin', onFocus, false); + document.removeEventListener('focusin', onFocusDocument, false); scope.forEach(element => element.removeEventListener('focusin', onFocus, false)); scope.forEach(element => element.removeEventListener('focusout', onBlur, false)); }; @@ -350,6 +372,15 @@ function useFocusContainment(scopeRef: RefObject, contain: boolean) { } }; }, [raf]); + + useEffect(() => { + // the autofocus attribute is too fast for our useLayoutEffect, so we check if some element in scope already has focus on mount + // and attach our blur fixer here + if (shouldContainFocus(scopeRef) && isElementInChildScope(document.activeElement, scopeRef)) { + let event = {target: document.activeElement}; + onSyntheticFocusDocument(event as React.FocusEvent); + } + }, []); } function isElementInAnyScope(element: Element) { diff --git a/packages/@react-aria/focus/test/FocusScope.test.js b/packages/@react-aria/focus/test/FocusScope.test.js index cedf5377057..49671ef7202 100644 --- a/packages/@react-aria/focus/test/FocusScope.test.js +++ b/packages/@react-aria/focus/test/FocusScope.test.js @@ -16,7 +16,7 @@ import {DialogContainer} from '@react-spectrum/dialog'; import {FocusScope, useFocusManager} from '../'; import {focusScopeTree} from '../src/FocusScope'; import {Provider} from '@react-spectrum/provider'; -import React, {useState} from 'react'; +import React, {useEffect, useRef, useState} from 'react'; import ReactDOM from 'react-dom'; import {Example as StorybookExample} from '../stories/FocusScope.stories'; import userEvent from '@testing-library/user-event'; @@ -651,44 +651,47 @@ describe('FocusScope', function () { }); describe('focusable first in scope', function () { - // would love for this test to work, but it won't fire the mutation observer - // it.only('should restore focus to the first focusable or tabbable element within the scope when focus is lost within the scope', async function () { - // let {getByTestId} = render(); - // function Example(props) { - // let [contentIndex, setContentIndex] = useState(0); - // return ( - //
- // - //
- // {contentIndex === 0 && } - // {contentIndex === 1 && } - // {contentIndex > 2 && } - //
- // - //
- // ); - // } - // let focusable = getByTestId('focusable'); - // let tabbable1 = getByTestId('tabbable1'); - // expect(document.activeElement).toBe(tabbable1); - // - // fireEvent.click(tabbable1); - // await waitFor(() => expect(tabbable1).not.toBeInTheDocument()); - // act(() => {jest.runAllTimers();}); - // let item1 = getByTestId('item1'); - // await waitFor(() => expect(document.activeElement).toBe(item1)); - // - // fireEvent.click(item1); - // act(() => {jest.runAllTimers();}); - // expect(item1).not.toBeInTheDocument(); - // let item2 = getByTestId('item2'); - // expect(document.activeElement).toBe(item2); - // - // fireEvent.click(item2); - // act(() => {jest.runAllTimers();}); - // expect(item2).not.toBeInTheDocument(); - // expect(document.activeElement).toBe(focusable); - // }); + it('should restore focus to the first focusable or tabbable element within the scope when focus is lost within the scope', async function () { + let tree = render(); + function Example(props) { + let [contentIndex, setContentIndex] = useState(0); + + return ( +
+ +
+ {contentIndex === 0 && } + {contentIndex === 1 && } + {contentIndex === 2 && } +
+ +
+ ); + } + let focusable = tree.getByTestId('focusable'); + let item1 = tree.getByTestId('item1'); + expect(document.activeElement).toBe(item1); + + fireEvent.click(item1); + // wait for the async MutationObserver + await act(async () => Promise.resolve()); + act(() => {jest.runAllTimers();}); + let item2 = tree.getByTestId('item2');; + expect(document.activeElement).toBe(item2); + + fireEvent.click(item2); + await act(async () => Promise.resolve()); + act(() => {jest.runAllTimers();}); + expect(item1).not.toBeInTheDocument(); + let item3 = tree.getByTestId('item3'); + expect(document.activeElement).toBe(focusable); + + fireEvent.click(item3); + await act(async () => Promise.resolve()); + act(() => {jest.runAllTimers();}); + expect(item3).not.toBeInTheDocument(); + expect(document.activeElement).toBe(focusable); + }); }); }); diff --git a/packages/@react-aria/interactions/src/utils.ts b/packages/@react-aria/interactions/src/utils.ts index 1380c868713..4ec53ff24c5 100644 --- a/packages/@react-aria/interactions/src/utils.ts +++ b/packages/@react-aria/interactions/src/utils.ts @@ -98,6 +98,8 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { if (state.observer) { state.observer.disconnect(); state.observer = null; + } + if (state.removalObserver) { state.removalObserver.disconnect(); state.removalObserver = null; } @@ -122,7 +124,7 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { let onBlurHandler = (e: FocusEvent) => { stateRef.current.isFocused = false; - if (target.disabled) { + if (target.disabled || !document.body.contains(e.target)) { // For backward compatibility, dispatch a (fake) React synthetic event. stateRef.current.onBlur?.(new SyntheticFocusEvent('blur', e)); } @@ -157,15 +159,18 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { if (stateRef.current.isFocused) { for (const mutation of mutationList) { for (const node of mutation.removedNodes) { - if (node === target || node.contains(target)) { - stateRef.current.removalObserver.disconnect(); - // For backward compatibility, dispatch a (fake) React synthetic event. - let blurEvent = new SyntheticFocusEvent('blur', new FocusEvent('blur')); - blurEvent.target = target; - stateRef.current.onBlur?.(blurEvent); - stateRef.current.observer = null; - // early return so we don't look at the rest of the DOM - return; + if (node?.contains?.(target)) { + // this can be null because mutation observers fire async, so we might've disconnected and set to null before + // this fires, but it's still been scheduled by an event that happened before we disconnected + if (stateRef.current.removalObserver) { + stateRef.current.removalObserver.disconnect(); + stateRef.current.removalObserver = null; + // fire blur to cleanup any other synthetic blur handlers + target.dispatchEvent(new FocusEvent('blur')); + target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); + // early return so we don't look at the rest of the DOM + return; + } } } } @@ -173,6 +178,7 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { }); stateRef.current.observer.observe(target, {attributes: true, attributeFilter: ['disabled']}); + stateRef.current.removalObserver.observe(document, {childList: true, subtree: true, attributes: false, characterData: false}); } }, []); From 035e2c1cd6b0177261b6d2d6d9e242adc1ea2005 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Thu, 11 Aug 2022 12:21:59 -0700 Subject: [PATCH 04/18] fix tests --- packages/@react-spectrum/table/test/Table.test.js | 13 ++++++++----- .../@react-spectrum/table/test/TableSizing.test.js | 5 ++++- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index b2434df42b7..afbae51904b 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -3605,7 +3605,7 @@ describe('TableView', function () { ); - it('arrow keys interactions don\'t move the focus away from the textfield in the dialog', function () { + it('arrow keys interactions don\'t move the focus away from the textfield in the dialog', async function () { let tree = render(); let table = tree.getByRole('grid'); let rows = within(table).getAllByRole('row'); @@ -3622,17 +3622,17 @@ describe('TableView', function () { expect(document.activeElement).toEqual(input); expect(input.value).toBe('blah'); + fireEvent.keyDown(input, {key: 'ArrowLeft', code: 37, charCode: 37}); + fireEvent.keyUp(input, {key: 'ArrowLeft', code: 37, charCode: 37}); act(() => { - fireEvent.keyDown(input, {key: 'ArrowLeft', code: 37, charCode: 37}); - fireEvent.keyUp(input, {key: 'ArrowLeft', code: 37, charCode: 37}); jest.runAllTimers(); }); expect(document.activeElement).toEqual(input); + fireEvent.keyDown(input, {key: 'ArrowRight', code: 39, charCode: 39}); + fireEvent.keyUp(input, {key: 'ArrowRight', code: 39, charCode: 39}); act(() => { - fireEvent.keyDown(input, {key: 'ArrowRight', code: 39, charCode: 39}); - fireEvent.keyUp(input, {key: 'ArrowRight', code: 39, charCode: 39}); jest.runAllTimers(); }); @@ -3645,6 +3645,9 @@ describe('TableView', function () { }); expect(dialog).not.toBeInTheDocument(); + + // wait for any MutationObservers from synthetic blur + await act(() => Promise.resolve()); }); }); diff --git a/packages/@react-spectrum/table/test/TableSizing.test.js b/packages/@react-spectrum/table/test/TableSizing.test.js index d8053b33fcc..4b76889fd52 100644 --- a/packages/@react-spectrum/table/test/TableSizing.test.js +++ b/packages/@react-spectrum/table/test/TableSizing.test.js @@ -287,7 +287,7 @@ describe('TableViewSizing', function () { }); // To test https://github.com/adobe/react-spectrum/issues/1885 - it('should not throw error if selection mode changes with overflowMode="wrap" and selection was controlled', function () { + it('should not throw error if selection mode changes with overflowMode="wrap" and selection was controlled', async function () { function ControlledSelection(props) { let [selectedKeys, setSelectedKeys] = React.useState(new Set([])); return ( @@ -318,6 +318,9 @@ describe('TableViewSizing', function () { rerender(tree, ); act(() => {jest.runAllTimers();}); expect(tree.queryByRole('checkbox')).toBeNull(); + + // wait for any MutationObservers from synthetic blur + await act(() => Promise.resolve()); }); it('should return the proper cell z-indexes for overflowMode="wrap"', function () { From 9a8fc9f4971fa23607a92edf4ef54977b927ff32 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Thu, 11 Aug 2022 17:24:45 -0700 Subject: [PATCH 05/18] fix lint and types --- packages/@react-aria/focus/test/FocusScope.test.js | 4 ++-- packages/@react-aria/interactions/src/utils.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/@react-aria/focus/test/FocusScope.test.js b/packages/@react-aria/focus/test/FocusScope.test.js index 49671ef7202..2448b39ab34 100644 --- a/packages/@react-aria/focus/test/FocusScope.test.js +++ b/packages/@react-aria/focus/test/FocusScope.test.js @@ -16,7 +16,7 @@ import {DialogContainer} from '@react-spectrum/dialog'; import {FocusScope, useFocusManager} from '../'; import {focusScopeTree} from '../src/FocusScope'; import {Provider} from '@react-spectrum/provider'; -import React, {useEffect, useRef, useState} from 'react'; +import React, {useState} from 'react'; import ReactDOM from 'react-dom'; import {Example as StorybookExample} from '../stories/FocusScope.stories'; import userEvent from '@testing-library/user-event'; @@ -676,7 +676,7 @@ describe('FocusScope', function () { // wait for the async MutationObserver await act(async () => Promise.resolve()); act(() => {jest.runAllTimers();}); - let item2 = tree.getByTestId('item2');; + let item2 = tree.getByTestId('item2'); expect(document.activeElement).toBe(item2); fireEvent.click(item2); diff --git a/packages/@react-aria/interactions/src/utils.ts b/packages/@react-aria/interactions/src/utils.ts index 4ec53ff24c5..8fba0cab740 100644 --- a/packages/@react-aria/interactions/src/utils.ts +++ b/packages/@react-aria/interactions/src/utils.ts @@ -124,7 +124,7 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { let onBlurHandler = (e: FocusEvent) => { stateRef.current.isFocused = false; - if (target.disabled || !document.body.contains(e.target)) { + if (target.disabled || !document.body.contains(e.target as HTMLElement)) { // For backward compatibility, dispatch a (fake) React synthetic event. stateRef.current.onBlur?.(new SyntheticFocusEvent('blur', e)); } From 996ef85f55766f9e9ebbffd6dc229bd73d8ad161 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Fri, 12 Aug 2022 11:49:45 -0700 Subject: [PATCH 06/18] expose useSyntheticBlur as a util --- packages/@react-aria/focus/src/FocusScope.tsx | 2 +- .../@react-aria/interactions/src/useFocus.ts | 2 +- .../interactions/src/useFocusWithin.ts | 2 +- .../@react-aria/interactions/src/utils.ts | 154 ---------------- packages/@react-aria/utils/src/index.ts | 1 + .../utils/src/useSyntheticBlurEvent.ts | 165 ++++++++++++++++++ 6 files changed, 169 insertions(+), 157 deletions(-) create mode 100644 packages/@react-aria/utils/src/useSyntheticBlurEvent.ts diff --git a/packages/@react-aria/focus/src/FocusScope.tsx b/packages/@react-aria/focus/src/FocusScope.tsx index f546210c90f..e224e959aea 100644 --- a/packages/@react-aria/focus/src/FocusScope.tsx +++ b/packages/@react-aria/focus/src/FocusScope.tsx @@ -15,7 +15,7 @@ import {focusSafely} from './focusSafely'; import {isElementVisible} from './isElementVisible'; import React, {ReactNode, RefObject, useCallback, useContext, useEffect, useRef} from 'react'; import {useLayoutEffect} from '@react-aria/utils'; -import {useSyntheticBlurEvent} from '@react-aria/interactions/src/utils'; +import {useSyntheticBlurEvent} from '@react-aria/utils'; export interface FocusScopeProps { diff --git a/packages/@react-aria/interactions/src/useFocus.ts b/packages/@react-aria/interactions/src/useFocus.ts index fc3bfbcc018..b472a98585e 100644 --- a/packages/@react-aria/interactions/src/useFocus.ts +++ b/packages/@react-aria/interactions/src/useFocus.ts @@ -17,7 +17,7 @@ import {DOMAttributes, FocusEvents} from '@react-types/shared'; import {FocusEvent, useCallback} from 'react'; -import {useSyntheticBlurEvent} from './utils'; +import {useSyntheticBlurEvent} from '@react-aria/utils'; export interface FocusProps extends FocusEvents { /** Whether the focus events should be disabled. */ diff --git a/packages/@react-aria/interactions/src/useFocusWithin.ts b/packages/@react-aria/interactions/src/useFocusWithin.ts index e79796f8f41..cf03433f465 100644 --- a/packages/@react-aria/interactions/src/useFocusWithin.ts +++ b/packages/@react-aria/interactions/src/useFocusWithin.ts @@ -17,7 +17,7 @@ import {DOMAttributes} from '@react-types/shared'; import {FocusEvent, useCallback, useRef} from 'react'; -import {useSyntheticBlurEvent} from './utils'; +import {useSyntheticBlurEvent} from '@react-aria/utils'; export interface FocusWithinProps { /** Whether the focus within events should be disabled. */ diff --git a/packages/@react-aria/interactions/src/utils.ts b/packages/@react-aria/interactions/src/utils.ts index 8fba0cab740..d95fd43c6aa 100644 --- a/packages/@react-aria/interactions/src/utils.ts +++ b/packages/@react-aria/interactions/src/utils.ts @@ -10,9 +10,6 @@ * governing permissions and limitations under the License. */ -import {FocusEvent as ReactFocusEvent, useCallback, useRef} from 'react'; -import {useLayoutEffect} from '@react-aria/utils'; - // Original licensing for the following method can be found in the // NOTICE file in the root directory of this source tree. // See https://github.com/facebook/react/blob/3c713d513195a53788b3f8bb4b70279d68b15bcc/packages/react-interactions/events/src/dom/shared/index.js#L74-L87 @@ -32,154 +29,3 @@ export function isVirtualClick(event: MouseEvent | PointerEvent): boolean { return event.detail === 0 && !(event as PointerEvent).pointerType; } - -export class SyntheticFocusEvent implements ReactFocusEvent { - nativeEvent: FocusEvent; - target: Element; - currentTarget: Element; - relatedTarget: Element; - bubbles: boolean; - cancelable: boolean; - defaultPrevented: boolean; - eventPhase: number; - isTrusted: boolean; - timeStamp: number; - type: string; - - constructor(type: string, nativeEvent: FocusEvent) { - this.nativeEvent = nativeEvent; - this.target = nativeEvent.target as Element; - this.currentTarget = nativeEvent.currentTarget as Element; - this.relatedTarget = nativeEvent.relatedTarget as Element; - this.bubbles = nativeEvent.bubbles; - this.cancelable = nativeEvent.cancelable; - this.defaultPrevented = nativeEvent.defaultPrevented; - this.eventPhase = nativeEvent.eventPhase; - this.isTrusted = nativeEvent.isTrusted; - this.timeStamp = nativeEvent.timeStamp; - this.type = type; - } - - isDefaultPrevented(): boolean { - return this.nativeEvent.defaultPrevented; - } - - preventDefault(): void { - this.defaultPrevented = true; - this.nativeEvent.preventDefault(); - } - - stopPropagation(): void { - this.nativeEvent.stopPropagation(); - this.isPropagationStopped = () => true; - } - - isPropagationStopped(): boolean { - return false; - } - - persist() {} -} - -export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { - let stateRef = useRef({ - isFocused: false, - onBlur, - observer: null as MutationObserver, - removalObserver: null as MutationObserver - }); - stateRef.current.onBlur = onBlur; - - // Clean up MutationObserver on unmount. See below. - // eslint-disable-next-line arrow-body-style - useLayoutEffect(() => { - const state = stateRef.current; - return () => { - if (state.observer) { - state.observer.disconnect(); - state.observer = null; - } - if (state.removalObserver) { - state.removalObserver.disconnect(); - state.removalObserver = null; - } - }; - }, []); - - // This function is called during a React onFocus event. - return useCallback((e: ReactFocusEvent) => { - // React does not fire onBlur when an element is disabled. https://github.com/facebook/react/issues/9142 - // Most browsers fire a native focusout event in this case, except for Firefox. In that case, we use a - // MutationObserver to watch for the disabled attribute, and dispatch these events ourselves. - // For browsers that do, focusout fires before the MutationObserver, so onBlur should not fire twice. - if ( - e.target instanceof HTMLButtonElement || - e.target instanceof HTMLInputElement || - e.target instanceof HTMLTextAreaElement || - e.target instanceof HTMLSelectElement - ) { - stateRef.current.isFocused = true; - - let target = e.target; - let onBlurHandler = (e: FocusEvent) => { - stateRef.current.isFocused = false; - - if (target.disabled || !document.body.contains(e.target as HTMLElement)) { - // For backward compatibility, dispatch a (fake) React synthetic event. - stateRef.current.onBlur?.(new SyntheticFocusEvent('blur', e)); - } - - // We no longer need the MutationObserver once the target is blurred. - if (stateRef.current.observer) { - stateRef.current.observer.disconnect(); - stateRef.current.observer = null; - } - - // We no longer need the MutationObserver once the target is blurred. - if (stateRef.current.removalObserver) { - stateRef.current.removalObserver.disconnect(); - stateRef.current.removalObserver = null; - } - }; - - target.addEventListener('focusout', onBlurHandler, {once: true}); - - stateRef.current.observer = new MutationObserver(() => { - if (stateRef.current.isFocused && target.disabled) { - stateRef.current.observer.disconnect(); - target.dispatchEvent(new FocusEvent('blur')); - target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); - } - }); - - // Some browsers do not fire onBlur when the document.activeElement is removed from the dom. - // Firefox has had a bug open about it for 13 years https://bugzilla.mozilla.org/show_bug.cgi?id=559561 - // A Safari bug has been logged for it as well https://bugs.webkit.org/show_bug.cgi?id=243749 - stateRef.current.removalObserver = new MutationObserver((mutationList) => { - if (stateRef.current.isFocused) { - for (const mutation of mutationList) { - for (const node of mutation.removedNodes) { - if (node?.contains?.(target)) { - // this can be null because mutation observers fire async, so we might've disconnected and set to null before - // this fires, but it's still been scheduled by an event that happened before we disconnected - if (stateRef.current.removalObserver) { - stateRef.current.removalObserver.disconnect(); - stateRef.current.removalObserver = null; - // fire blur to cleanup any other synthetic blur handlers - target.dispatchEvent(new FocusEvent('blur')); - target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); - // early return so we don't look at the rest of the DOM - return; - } - } - } - } - } - }); - - stateRef.current.observer.observe(target, {attributes: true, attributeFilter: ['disabled']}); - - stateRef.current.removalObserver.observe(document, {childList: true, subtree: true, attributes: false, characterData: false}); - } - }, []); -} diff --git a/packages/@react-aria/utils/src/index.ts b/packages/@react-aria/utils/src/index.ts index f3401686391..95ccf30890a 100644 --- a/packages/@react-aria/utils/src/index.ts +++ b/packages/@react-aria/utils/src/index.ts @@ -33,3 +33,4 @@ export {useEvent} from './useEvent'; export {useValueEffect} from './useValueEffect'; export {scrollIntoView} from './scrollIntoView'; export {clamp, snapValueToStep} from '@react-stately/utils'; +export {useSyntheticBlurEvent} from './useSyntheticBlurEvent'; diff --git a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts new file mode 100644 index 00000000000..baed3a23ad7 --- /dev/null +++ b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts @@ -0,0 +1,165 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {FocusEvent as ReactFocusEvent, useCallback, useRef} from 'react'; +import {useLayoutEffect} from './useLayoutEffect'; + +export class UseSyntheticBlurEvent implements ReactFocusEvent { + nativeEvent: FocusEvent; + target: Element; + currentTarget: Element; + relatedTarget: Element; + bubbles: boolean; + cancelable: boolean; + defaultPrevented: boolean; + eventPhase: number; + isTrusted: boolean; + timeStamp: number; + type: string; + + constructor(type: string, nativeEvent: FocusEvent) { + this.nativeEvent = nativeEvent; + this.target = nativeEvent.target as Element; + this.currentTarget = nativeEvent.currentTarget as Element; + this.relatedTarget = nativeEvent.relatedTarget as Element; + this.bubbles = nativeEvent.bubbles; + this.cancelable = nativeEvent.cancelable; + this.defaultPrevented = nativeEvent.defaultPrevented; + this.eventPhase = nativeEvent.eventPhase; + this.isTrusted = nativeEvent.isTrusted; + this.timeStamp = nativeEvent.timeStamp; + this.type = type; + } + + isDefaultPrevented(): boolean { + return this.nativeEvent.defaultPrevented; + } + + preventDefault(): void { + this.defaultPrevented = true; + this.nativeEvent.preventDefault(); + } + + stopPropagation(): void { + this.nativeEvent.stopPropagation(); + this.isPropagationStopped = () => true; + } + + isPropagationStopped(): boolean { + return false; + } + + persist() {} +} + +export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { + let stateRef = useRef({ + isFocused: false, + onBlur, + observer: null as MutationObserver, + removalObserver: null as MutationObserver + }); + stateRef.current.onBlur = onBlur; + + // Clean up MutationObserver on unmount. See below. + // eslint-disable-next-line arrow-body-style + useLayoutEffect(() => { + const state = stateRef.current; + return () => { + if (state.observer) { + state.observer.disconnect(); + state.observer = null; + } + if (state.removalObserver) { + state.removalObserver.disconnect(); + state.removalObserver = null; + } + }; + }, []); + + // This function is called during a React onFocus event. + return useCallback((e: ReactFocusEvent) => { + // React does not fire onBlur when an element is disabled. https://github.com/facebook/react/issues/9142 + // Most browsers fire a native focusout event in this case, except for Firefox. In that case, we use a + // MutationObserver to watch for the disabled attribute, and dispatch these events ourselves. + // For browsers that do, focusout fires before the MutationObserver, so onBlur should not fire twice. + if ( + e.target instanceof HTMLButtonElement || + e.target instanceof HTMLInputElement || + e.target instanceof HTMLTextAreaElement || + e.target instanceof HTMLSelectElement + ) { + stateRef.current.isFocused = true; + + let target = e.target; + let onBlurHandler = (e: FocusEvent) => { + stateRef.current.isFocused = false; + + if (target.disabled || !document.body.contains(e.target as HTMLElement)) { + // For backward compatibility, dispatch a (fake) React synthetic event. + stateRef.current.onBlur?.(new UseSyntheticBlurEvent('blur', e)); + } + + // We no longer need the MutationObserver once the target is blurred. + if (stateRef.current.observer) { + stateRef.current.observer.disconnect(); + stateRef.current.observer = null; + } + + // We no longer need the MutationObserver once the target is blurred. + if (stateRef.current.removalObserver) { + stateRef.current.removalObserver.disconnect(); + stateRef.current.removalObserver = null; + } + }; + + target.addEventListener('focusout', onBlurHandler, {once: true}); + + stateRef.current.observer = new MutationObserver(() => { + if (stateRef.current.isFocused && target.disabled) { + stateRef.current.observer.disconnect(); + target.dispatchEvent(new FocusEvent('blur')); + target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); + } + }); + + // Some browsers do not fire onBlur when the document.activeElement is removed from the dom. + // Firefox has had a bug open about it for 13 years https://bugzilla.mozilla.org/show_bug.cgi?id=559561 + // A Safari bug has been logged for it as well https://bugs.webkit.org/show_bug.cgi?id=243749 + stateRef.current.removalObserver = new MutationObserver((mutationList) => { + if (stateRef.current.isFocused) { + for (const mutation of mutationList) { + for (const node of mutation.removedNodes) { + if (node?.contains?.(target)) { + // this can be null because mutation observers fire async, so we might've disconnected and set to null before + // this fires, but it's still been scheduled by an event that happened before we disconnected + if (stateRef.current.removalObserver) { + stateRef.current.removalObserver.disconnect(); + stateRef.current.removalObserver = null; + // fire blur to cleanup any other synthetic blur handlers + target.dispatchEvent(new FocusEvent('blur')); + target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); + // early return so we don't look at the rest of the DOM + return; + } + } + } + } + } + }); + + stateRef.current.observer.observe(target, {attributes: true, attributeFilter: ['disabled']}); + + stateRef.current.removalObserver.observe(document, {childList: true, subtree: true, attributes: false, characterData: false}); + } + }, []); +} From 44dc441f2b6a0e9ff9d61b9dcda509bd7c41ce30 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Fri, 12 Aug 2022 12:35:44 -0700 Subject: [PATCH 07/18] Fix cases where onBlur fired after unmount of component --- packages/@react-aria/focus/src/useFocusRing.ts | 16 ++++++++++++++-- .../table/test/TableSizing.test.js | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/@react-aria/focus/src/useFocusRing.ts b/packages/@react-aria/focus/src/useFocusRing.ts index 6e813ba6294..eb186a10539 100644 --- a/packages/@react-aria/focus/src/useFocusRing.ts +++ b/packages/@react-aria/focus/src/useFocusRing.ts @@ -1,6 +1,7 @@ import {DOMAttributes} from '@react-types/shared'; import {isFocusVisible, useFocus, useFocusVisibleListener, useFocusWithin} from '@react-aria/interactions'; import {useCallback, useState} from 'react'; +import {useLayoutEffect} from '@react-aria/utils'; import {useRef} from 'react'; export interface AriaFocusRingProps { @@ -47,12 +48,23 @@ export function useFocusRing(props: AriaFocusRingProps = {}): FocusRingAria { }); let [isFocused, setFocused] = useState(false); let [isFocusVisibleState, setFocusVisible] = useState(() => state.current.isFocused && state.current.isFocusVisible); + let safelySetFocused = useRef(setFocused); + let safelySetFocusVisible = useRef(setFocusVisible); - let updateState = useCallback(() => setFocusVisible(state.current.isFocused && state.current.isFocusVisible), []); + // blur can now be fired asynchronously after a component unmounts, so we can't set state in response to that blur + // eslint-disable-next-line arrow-body-style + useLayoutEffect(() => { + return () => { + safelySetFocused.current = () => {}; + safelySetFocusVisible.current = () => {}; + }; + }, []); + + let updateState = useCallback(() => safelySetFocusVisible.current(state.current.isFocused && state.current.isFocusVisible), []); let onFocusChange = useCallback(isFocused => { state.current.isFocused = isFocused; - setFocused(isFocused); + safelySetFocused.current(isFocused); updateState(); }, [updateState]); diff --git a/packages/@react-spectrum/table/test/TableSizing.test.js b/packages/@react-spectrum/table/test/TableSizing.test.js index f2b13805d61..49ed70eccd8 100644 --- a/packages/@react-spectrum/table/test/TableSizing.test.js +++ b/packages/@react-spectrum/table/test/TableSizing.test.js @@ -322,7 +322,7 @@ describe('TableViewSizing', function () { expect(tree.queryByRole('checkbox')).toBeNull(); // wait for any MutationObservers from synthetic blur - await act(() => Promise.resolve()); + await act(async () => Promise.resolve()); }); it('should return the proper cell z-indexes for overflowMode="wrap"', function () { From 295e2ed7bedd0a39b36b0fa4373b2198d594afc0 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Fri, 12 Aug 2022 18:10:28 -0700 Subject: [PATCH 08/18] clean up blur handles when unmounting todo: turn into single mutation observer --- .../@react-aria/focus/src/useFocusRing.ts | 16 +-- .../@react-aria/focus/test/FocusRing.test.tsx | 49 ++++++++ .../utils/src/useSyntheticBlurEvent.ts | 109 ++++++++++-------- 3 files changed, 111 insertions(+), 63 deletions(-) create mode 100644 packages/@react-aria/focus/test/FocusRing.test.tsx diff --git a/packages/@react-aria/focus/src/useFocusRing.ts b/packages/@react-aria/focus/src/useFocusRing.ts index eb186a10539..6e813ba6294 100644 --- a/packages/@react-aria/focus/src/useFocusRing.ts +++ b/packages/@react-aria/focus/src/useFocusRing.ts @@ -1,7 +1,6 @@ import {DOMAttributes} from '@react-types/shared'; import {isFocusVisible, useFocus, useFocusVisibleListener, useFocusWithin} from '@react-aria/interactions'; import {useCallback, useState} from 'react'; -import {useLayoutEffect} from '@react-aria/utils'; import {useRef} from 'react'; export interface AriaFocusRingProps { @@ -48,23 +47,12 @@ export function useFocusRing(props: AriaFocusRingProps = {}): FocusRingAria { }); let [isFocused, setFocused] = useState(false); let [isFocusVisibleState, setFocusVisible] = useState(() => state.current.isFocused && state.current.isFocusVisible); - let safelySetFocused = useRef(setFocused); - let safelySetFocusVisible = useRef(setFocusVisible); - // blur can now be fired asynchronously after a component unmounts, so we can't set state in response to that blur - // eslint-disable-next-line arrow-body-style - useLayoutEffect(() => { - return () => { - safelySetFocused.current = () => {}; - safelySetFocusVisible.current = () => {}; - }; - }, []); - - let updateState = useCallback(() => safelySetFocusVisible.current(state.current.isFocused && state.current.isFocusVisible), []); + let updateState = useCallback(() => setFocusVisible(state.current.isFocused && state.current.isFocusVisible), []); let onFocusChange = useCallback(isFocused => { state.current.isFocused = isFocused; - safelySetFocused.current(isFocused); + setFocused(isFocused); updateState(); }, [updateState]); diff --git a/packages/@react-aria/focus/test/FocusRing.test.tsx b/packages/@react-aria/focus/test/FocusRing.test.tsx new file mode 100644 index 00000000000..61483b8f454 --- /dev/null +++ b/packages/@react-aria/focus/test/FocusRing.test.tsx @@ -0,0 +1,49 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {act, render} from '@react-spectrum/test-utils'; +import {FocusRing} from '../'; +import React from 'react'; +import userEvent from '@testing-library/user-event'; + + +describe('FocusScope', function () { + beforeEach(() => { + jest.useFakeTimers(); + }); + afterEach(() => { + // make sure to clean up any raf's that may be running to restore focus on unmount + act(() => {jest.runAllTimers();}); + }); + + describe('focus containment', function () { + it('should contain focus within the scope', function () { + let {getByTestId, rerender} = render( + <> + + + + + + + ); + + let input1 = getByTestId('input1'); + userEvent.tab(); + expect(document.activeElement).toBe(input1); + rerender((<> +
+ )); + expect(input1).not.toBeInTheDocument(); + }); + }); +}); diff --git a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts index baed3a23ad7..3cf0de07254 100644 --- a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts +++ b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts @@ -13,7 +13,7 @@ import {FocusEvent as ReactFocusEvent, useCallback, useRef} from 'react'; import {useLayoutEffect} from './useLayoutEffect'; -export class UseSyntheticBlurEvent implements ReactFocusEvent { +class SyntheticBlurEvent implements ReactFocusEvent { nativeEvent: FocusEvent; target: Element; currentTarget: Element; @@ -66,7 +66,8 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { isFocused: false, onBlur, observer: null as MutationObserver, - removalObserver: null as MutationObserver + removalObserver: null as MutationObserver, + target: null as HTMLElement }); stateRef.current.onBlur = onBlur; @@ -75,6 +76,9 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { useLayoutEffect(() => { const state = stateRef.current; return () => { + if (stateRef.current.target) { + stateRef.current.target.removeEventListener('focusout', onBlurHandler); + } if (state.observer) { state.observer.disconnect(); state.observer = null; @@ -86,8 +90,37 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { }; }, []); + + let onBlurHandler = useCallback((e: FocusEvent) => { + let target = e.target; + stateRef.current.isFocused = false; + + if (((target instanceof HTMLButtonElement || + target instanceof HTMLInputElement || + target instanceof HTMLTextAreaElement || + target instanceof HTMLSelectElement) && target.disabled) || !document.body.contains(e.target as HTMLElement)) { + // For backward compatibility, dispatch a (fake) React synthetic event. + stateRef.current.onBlur?.(new SyntheticBlurEvent('blur', e)); + } + + // We no longer need the MutationObserver once the target is blurred. + if (stateRef.current.observer) { + stateRef.current.observer.disconnect(); + stateRef.current.observer = null; + } + + // We no longer need the MutationObserver once the target is blurred. + if (stateRef.current.removalObserver) { + stateRef.current.removalObserver.disconnect(); + stateRef.current.removalObserver = null; + } + }, [stateRef]); + // This function is called during a React onFocus event. return useCallback((e: ReactFocusEvent) => { + + let target = e.target; + stateRef.current.target = target as HTMLElement; // React does not fire onBlur when an element is disabled. https://github.com/facebook/react/issues/9142 // Most browsers fire a native focusout event in this case, except for Firefox. In that case, we use a // MutationObserver to watch for the disabled attribute, and dispatch these events ourselves. @@ -100,66 +133,44 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { ) { stateRef.current.isFocused = true; - let target = e.target; - let onBlurHandler = (e: FocusEvent) => { - stateRef.current.isFocused = false; - - if (target.disabled || !document.body.contains(e.target as HTMLElement)) { - // For backward compatibility, dispatch a (fake) React synthetic event. - stateRef.current.onBlur?.(new UseSyntheticBlurEvent('blur', e)); - } - - // We no longer need the MutationObserver once the target is blurred. - if (stateRef.current.observer) { - stateRef.current.observer.disconnect(); - stateRef.current.observer = null; - } - - // We no longer need the MutationObserver once the target is blurred. - if (stateRef.current.removalObserver) { - stateRef.current.removalObserver.disconnect(); - stateRef.current.removalObserver = null; - } - }; - target.addEventListener('focusout', onBlurHandler, {once: true}); stateRef.current.observer = new MutationObserver(() => { - if (stateRef.current.isFocused && target.disabled) { + if (stateRef.current.isFocused && (target as any).disabled) { stateRef.current.observer.disconnect(); target.dispatchEvent(new FocusEvent('blur')); target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); } }); - // Some browsers do not fire onBlur when the document.activeElement is removed from the dom. - // Firefox has had a bug open about it for 13 years https://bugzilla.mozilla.org/show_bug.cgi?id=559561 - // A Safari bug has been logged for it as well https://bugs.webkit.org/show_bug.cgi?id=243749 - stateRef.current.removalObserver = new MutationObserver((mutationList) => { - if (stateRef.current.isFocused) { - for (const mutation of mutationList) { - for (const node of mutation.removedNodes) { - if (node?.contains?.(target)) { - // this can be null because mutation observers fire async, so we might've disconnected and set to null before - // this fires, but it's still been scheduled by an event that happened before we disconnected - if (stateRef.current.removalObserver) { - stateRef.current.removalObserver.disconnect(); - stateRef.current.removalObserver = null; - // fire blur to cleanup any other synthetic blur handlers - target.dispatchEvent(new FocusEvent('blur')); - target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); - // early return so we don't look at the rest of the DOM - return; - } + stateRef.current.observer.observe(target, {attributes: true, attributeFilter: ['disabled']}); + } + + // Some browsers do not fire onBlur when the document.activeElement is removed from the dom. + // Firefox has had a bug open about it for 13 years https://bugzilla.mozilla.org/show_bug.cgi?id=559561 + // A Safari bug has been logged for it as well https://bugs.webkit.org/show_bug.cgi?id=243749 + stateRef.current.removalObserver = new MutationObserver((mutationList) => { + if (stateRef.current.isFocused) { + for (const mutation of mutationList) { + for (const node of mutation.removedNodes) { + if (node?.contains?.(target)) { + // this can be null because mutation observers fire async, so we might've disconnected and set to null before + // this fires, but it's still been scheduled by an event that happened before we disconnected + if (stateRef.current.removalObserver) { + stateRef.current.removalObserver.disconnect(); + stateRef.current.removalObserver = null; + // fire blur to cleanup any other synthetic blur handlers + target.dispatchEvent(new FocusEvent('blur')); + target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); + // early return so we don't look at the rest of the DOM + return; } } } } - }); - - stateRef.current.observer.observe(target, {attributes: true, attributeFilter: ['disabled']}); + } + }); - stateRef.current.removalObserver.observe(document, {childList: true, subtree: true, attributes: false, characterData: false}); - } + stateRef.current.removalObserver.observe(document, {childList: true, subtree: true, attributes: false, characterData: false}); }, []); } From 3123afdec12735cf954c2cfc6300014958db9cfb Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 16 Aug 2022 19:57:22 -0700 Subject: [PATCH 09/18] only one node removal mutation observer --- .../utils/src/useSyntheticBlurEvent.ts | 155 +++++++++++------- .../datepicker/test/DatePicker.test.js | 14 +- .../datepicker/test/DateRangePicker.test.js | 9 +- .../list/test/ListView.test.js | 3 +- .../@react-spectrum/table/test/Table.test.js | 23 ++- 5 files changed, 128 insertions(+), 76 deletions(-) diff --git a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts index 3cf0de07254..8b1912bcd80 100644 --- a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts +++ b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts @@ -61,66 +61,97 @@ class SyntheticBlurEvent implements ReactFocusEvent { persist() {} } +type Listener = (mutationList: MutationRecord[], target: HTMLElement) => boolean; + export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { let stateRef = useRef({ isFocused: false, onBlur, - observer: null as MutationObserver, - removalObserver: null as MutationObserver, + disabledObserver: null as MutationObserver, target: null as HTMLElement }); stateRef.current.onBlur = onBlur; - // Clean up MutationObserver on unmount. See below. - // eslint-disable-next-line arrow-body-style - useLayoutEffect(() => { - const state = stateRef.current; - return () => { - if (stateRef.current.target) { - stateRef.current.target.removeEventListener('focusout', onBlurHandler); - } - if (state.observer) { - state.observer.disconnect(); - state.observer = null; - } - if (state.removalObserver) { - state.removalObserver.disconnect(); - state.removalObserver = null; + let domNodeRemovedHandler = useCallback((mutationList, target) => { + if (stateRef.current.isFocused && target) { + for (const mutation of mutationList) { + for (const node of mutation.removedNodes) { + if (node?.contains?.(target)) { + domNodeRemovedObserver.removeListener(domNodeRemovedHandler); + // fire blur which will be picked up by the 'once' blur handler defined below + target.dispatchEvent(new FocusEvent('blur')); + target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); + // early return so we don't look at the rest of the DOM + return true; + } + } } - }; - }, []); - + } + return false; + }, [stateRef]); let onBlurHandler = useCallback((e: FocusEvent) => { let target = e.target; stateRef.current.isFocused = false; - if (((target instanceof HTMLButtonElement || - target instanceof HTMLInputElement || - target instanceof HTMLTextAreaElement || - target instanceof HTMLSelectElement) && target.disabled) || !document.body.contains(e.target as HTMLElement)) { + if ( + ( + (target instanceof HTMLButtonElement || + target instanceof HTMLInputElement || + target instanceof HTMLTextAreaElement || + target instanceof HTMLSelectElement) && + target.disabled + ) || + !document.body.contains(e.target as HTMLElement) + ) { // For backward compatibility, dispatch a (fake) React synthetic event. stateRef.current.onBlur?.(new SyntheticBlurEvent('blur', e)); } // We no longer need the MutationObserver once the target is blurred. - if (stateRef.current.observer) { - stateRef.current.observer.disconnect(); - stateRef.current.observer = null; + if (stateRef.current.disabledObserver) { + stateRef.current.disabledObserver.disconnect(); + stateRef.current.disabledObserver = null; } - // We no longer need the MutationObserver once the target is blurred. - if (stateRef.current.removalObserver) { - stateRef.current.removalObserver.disconnect(); - stateRef.current.removalObserver = null; - } + domNodeRemovedObserver.removeListener(domNodeRemovedHandler); }, [stateRef]); + // Clean up MutationObserver on unmount. See below. + useLayoutEffect(() => { + const state = stateRef.current; + + return () => { + if (stateRef.current.isFocused) { + stateRef.current.isFocused = false; + } + if (stateRef.current.target) { + stateRef.current.target.removeEventListener('focusout', onBlurHandler); + stateRef.current.target = null; + } + if (state.disabledObserver) { + state.disabledObserver.disconnect(); + state.disabledObserver = null; + } + domNodeRemovedObserver.removeListener(domNodeRemovedHandler); + }; + }, [domNodeRemovedHandler]); + // This function is called during a React onFocus event. return useCallback((e: ReactFocusEvent) => { let target = e.target; + stateRef.current.isFocused = true; + + if (stateRef.current.target && target !== stateRef.current.target) { + stateRef.current.target.removeEventListener('focusout', onBlurHandler); + target.addEventListener('focusout', onBlurHandler, {once: true}); + } else if (!stateRef.current.target || e.target !== stateRef.current.target) { + e.target.addEventListener('focusout', onBlurHandler, {once: true}); + } + stateRef.current.target = target as HTMLElement; + // React does not fire onBlur when an element is disabled. https://github.com/facebook/react/issues/9142 // Most browsers fire a native focusout event in this case, except for Firefox. In that case, we use a // MutationObserver to watch for the disabled attribute, and dispatch these events ourselves. @@ -131,46 +162,52 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { e.target instanceof HTMLTextAreaElement || e.target instanceof HTMLSelectElement ) { - stateRef.current.isFocused = true; - target.addEventListener('focusout', onBlurHandler, {once: true}); - - stateRef.current.observer = new MutationObserver(() => { + stateRef.current.disabledObserver = new MutationObserver(() => { if (stateRef.current.isFocused && (target as any).disabled) { - stateRef.current.observer.disconnect(); + stateRef.current.disabledObserver.disconnect(); target.dispatchEvent(new FocusEvent('blur')); target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); } }); - stateRef.current.observer.observe(target, {attributes: true, attributeFilter: ['disabled']}); + stateRef.current.disabledObserver.observe(target, {attributes: true, attributeFilter: ['disabled']}); } // Some browsers do not fire onBlur when the document.activeElement is removed from the dom. // Firefox has had a bug open about it for 13 years https://bugzilla.mozilla.org/show_bug.cgi?id=559561 // A Safari bug has been logged for it as well https://bugs.webkit.org/show_bug.cgi?id=243749 - stateRef.current.removalObserver = new MutationObserver((mutationList) => { - if (stateRef.current.isFocused) { - for (const mutation of mutationList) { - for (const node of mutation.removedNodes) { - if (node?.contains?.(target)) { - // this can be null because mutation observers fire async, so we might've disconnected and set to null before - // this fires, but it's still been scheduled by an event that happened before we disconnected - if (stateRef.current.removalObserver) { - stateRef.current.removalObserver.disconnect(); - stateRef.current.removalObserver = null; - // fire blur to cleanup any other synthetic blur handlers - target.dispatchEvent(new FocusEvent('blur')); - target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); - // early return so we don't look at the rest of the DOM - return; - } - } - } + domNodeRemovedObserver.addListener(domNodeRemovedHandler, target); + }, []); +} + + +class DOMNodeRemovedObserver { + private observer; + public listeners = new Map(); + constructor() { + this.observer = new MutationObserver((mutationList) => { + for (let [listener, target] of this.listeners) { + if (listener(mutationList, target)) { + // early return if we find a place to call blur + return; } } }); - - stateRef.current.removalObserver.observe(document, {childList: true, subtree: true, attributes: false, characterData: false}); - }, []); + } + addListener(listener, target) { + if (this.listeners.size <= 0) { + this.observer.observe(document, {childList: true, subtree: true, attributes: false, characterData: false}); + } + this.listeners.set(listener, target); + } + removeListener(listener) { + if (this.listeners.has(listener)) { + this.listeners.delete(listener); + } + if (this.listeners.size <= 0) { + this.observer.disconnect(); + } + } } +let domNodeRemovedObserver = new DOMNodeRemovedObserver(); diff --git a/packages/@react-spectrum/datepicker/test/DatePicker.test.js b/packages/@react-spectrum/datepicker/test/DatePicker.test.js index 57ae559db68..1e564317499 100644 --- a/packages/@react-spectrum/datepicker/test/DatePicker.test.js +++ b/packages/@react-spectrum/datepicker/test/DatePicker.test.js @@ -55,7 +55,7 @@ function render(el) { describe('DatePicker', function () { beforeAll(() => { - jest.useFakeTimers('legacy'); + jest.useFakeTimers(); }); afterEach(() => { act(() => { @@ -186,7 +186,7 @@ describe('DatePicker', function () { }); describe('calendar popover', function () { - it('should emit onChange when selecting a date in the calendar in controlled mode', function () { + it('should emit onChange when selecting a date in the calendar in controlled mode', async function () { let onChange = jest.fn(); let {getByRole, getAllByRole, queryByLabelText} = render( @@ -215,9 +215,10 @@ describe('DatePicker', function () { expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenCalledWith(new CalendarDate(2019, 2, 4)); expect(getTextValue(combobox)).toBe('2/3/2019'); // controlled + await act(async () => Promise.resolve()); }); - it('should emit onChange when selecting a date in the calendar in uncontrolled mode', function () { + it('should emit onChange when selecting a date in the calendar in uncontrolled mode', async function () { let onChange = jest.fn(); let {getByRole, getAllByRole} = render( @@ -244,6 +245,7 @@ describe('DatePicker', function () { expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenCalledWith(new CalendarDate(2019, 2, 4)); expect(getTextValue(combobox)).toBe('2/4/2019'); // uncontrolled + await act(async () => Promise.resolve()); }); it('should display a time field when a CalendarDateTime value is used', function () { @@ -371,7 +373,7 @@ describe('DatePicker', function () { expectPlaceholder(combobox, formatter.format(value.toDate(getLocalTimeZone()))); }); - it('should confirm time placeholder on blur if date is selected', function () { + it('should confirm time placeholder on blur if date is selected', async function () { let onChange = jest.fn(); let {getByRole, getAllByRole} = render( @@ -398,6 +400,7 @@ describe('DatePicker', function () { userEvent.click(document.body); act(() => jest.runAllTimers()); + await act(async () => Promise.resolve()); expect(dialog).not.toBeInTheDocument(); @@ -407,7 +410,7 @@ describe('DatePicker', function () { expectPlaceholder(combobox, formatter.format(value.toDate(getLocalTimeZone()))); }); - it('should not confirm on blur if date is not selected', function () { + it('should not confirm on blur if date is not selected', async function () { let onChange = jest.fn(); let {getByRole, getAllByRole, getAllByLabelText} = render( @@ -440,6 +443,7 @@ describe('DatePicker', function () { userEvent.click(document.body); act(() => jest.runAllTimers()); + await act(async () => Promise.resolve()); expect(dialog).not.toBeInTheDocument(); expect(onChange).not.toHaveBeenCalled(); diff --git a/packages/@react-spectrum/datepicker/test/DateRangePicker.test.js b/packages/@react-spectrum/datepicker/test/DateRangePicker.test.js index 0237a47607a..5343512d5c9 100644 --- a/packages/@react-spectrum/datepicker/test/DateRangePicker.test.js +++ b/packages/@react-spectrum/datepicker/test/DateRangePicker.test.js @@ -264,7 +264,7 @@ describe('DateRangePicker', function () { }); describe('calendar popover', function () { - it('should emit onChange when selecting a date range in the calendar in uncontrolled mode', function () { + it('should emit onChange when selecting a date range in the calendar in uncontrolled mode', async function () { let onChange = jest.fn(); let {getByRole, getByTestId, getAllByRole, getByLabelText} = render( @@ -295,6 +295,7 @@ describe('DateRangePicker', function () { expect(onChange).toHaveBeenCalledWith({start: new CalendarDate(2019, 2, 10), end: new CalendarDate(2019, 2, 17)}); expect(getTextValue(startDate)).toBe('2/10/2019'); // uncontrolled expect(getTextValue(endDate)).toBe('2/17/2019'); + await act(async () => Promise.resolve()); }); it('should display time fields when a CalendarDateTime value is used', function () { @@ -457,7 +458,7 @@ describe('DateRangePicker', function () { expect(getTextValue(endDate)).toBe(formatter.format(endValue.toDate(getLocalTimeZone()))); }); - it('should confirm time placeholders on blur if date range is selected', function () { + it('should confirm time placeholders on blur if date range is selected', async function () { let onChange = jest.fn(); let {getByRole, getAllByRole, getByTestId} = render( @@ -486,6 +487,7 @@ describe('DateRangePicker', function () { userEvent.click(document.body); act(() => jest.runAllTimers()); + await act(async () => Promise.resolve()); expect(dialog).not.toBeInTheDocument(); @@ -497,7 +499,7 @@ describe('DateRangePicker', function () { expect(getTextValue(endDate)).toBe(formatter.format(endValue.toDate(getLocalTimeZone()))); }); - it('should not confirm on blur if date range is not selected', function () { + it('should not confirm on blur if date range is not selected', async function () { let onChange = jest.fn(); let {getByRole, getAllByLabelText, getByTestId} = render( @@ -532,6 +534,7 @@ describe('DateRangePicker', function () { userEvent.click(document.body); act(() => jest.runAllTimers()); + await act(async () => Promise.resolve()); expect(dialog).not.toBeInTheDocument(); expect(onChange).not.toHaveBeenCalled(); diff --git a/packages/@react-spectrum/list/test/ListView.test.js b/packages/@react-spectrum/list/test/ListView.test.js index 3875e23c49b..859e8760e89 100644 --- a/packages/@react-spectrum/list/test/ListView.test.js +++ b/packages/@react-spectrum/list/test/ListView.test.js @@ -1265,7 +1265,7 @@ describe('ListView', function () { }); describe('scrolling', function () { - it('should scroll to a row when it is focused', function () { + it('should scroll to a row when it is focused', async function () { let tree = render( Promise.resolve()); }); it('should scroll to a row when it is focused', function () { diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index 74438b6a5b5..854259adcbe 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -121,7 +121,8 @@ describe('TableView', function () { offsetHeight.mockReset(); }); - afterEach(() => { + afterEach(async () => { + await act(async () => Promise.resolve()); act(() => {jest.runAllTimers();}); }); @@ -1565,7 +1566,7 @@ describe('TableView', function () { expect(body.scrollTop).toBe(24); }); - it('should scroll to a cell when it is focused off screen', function () { + it('should scroll to a cell when it is focused off screen', async function () { let tree = renderManyColumns(); let body = tree.getByRole('grid').childNodes[1]; @@ -1574,7 +1575,7 @@ describe('TableView', function () { expect(document.activeElement).toBe(cell); expect(body.scrollTop).toBe(0); - // When scrolling the focused item out of view, focus should remaind on the item, + // When scrolling the focused item out of view, focus should remain on the item, // virtualizer keeps focused items from being reused body.scrollTop = 1000; body.scrollLeft = 1000; @@ -1598,6 +1599,8 @@ describe('TableView', function () { moveFocus('ArrowLeft'); expect(body.scrollTop).toBe(164); expect(document.activeElement).toBe(getCell(tree, 'Foo 5 4')); + act(() => {jest.runAllTimers();}); + await act(async () => Promise.resolve()); }); it('should not scroll when a column header receives focus', function () { @@ -2274,7 +2277,7 @@ describe('TableView', function () { checkRowSelection(rows.slice(1), true); }); - it('manually selecting all should not auto select new items', function () { + it('manually selecting all should not auto select new items', async function () { let onSelectionChange = jest.fn(); let tree = renderTable({onSelectionChange}, items); @@ -2295,6 +2298,7 @@ describe('TableView', function () { ])); act(() => jest.runAllTimers()); + await act(async () => Promise.resolve()); rows = tree.getAllByRole('row'); expect(getCell(tree, 'Foo 0')).toBeVisible(); @@ -2322,7 +2326,7 @@ describe('TableView', function () { }); }); - describe('annoucements', function () { + describe('announcements', function () { it('should announce the selected or deselected row', function () { let onSelectionChange = jest.fn(); let tree = renderTable({onSelectionChange}); @@ -2459,7 +2463,7 @@ describe('TableView', function () { expect(announce).toHaveBeenCalledTimes(2); }); - it('updates even if not focused', () => { + it('updates even if not focused', async () => { let tree = render(); let link = tree.getAllByRole('link')[1]; @@ -2496,6 +2500,7 @@ describe('TableView', function () { // TableWithBreadcrumbs has a setTimeout to load the results of the link navigation on Folder A jest.runAllTimers(); }); + await act(async () => Promise.resolve()); expect(announce).toHaveBeenCalledTimes(3); expect(announce).toHaveBeenLastCalledWith('No items selected.'); @@ -3444,7 +3449,7 @@ describe('TableView', function () { expect(checkbox.checked).toBe(false); }); - it('can edit items', function () { + it('can edit items', async function () { let tree = render(); let table = tree.getByRole('grid'); @@ -3475,6 +3480,7 @@ describe('TableView', function () { expect(dialog).not.toBeInTheDocument(); act(() => {jest.runAllTimers();}); + await act(async () => Promise.resolve()); let rowHeaders = within(rows[2]).getAllByRole('rowheader'); expect(rowHeaders[0]).toHaveTextContent('Jessica'); @@ -3571,7 +3577,7 @@ describe('TableView', function () { expect(document.activeElement).toBe(within(menu).getAllByRole('menuitem').pop()); }); - it('menu keyboard navigation does not affect table', function () { + it('menu keyboard navigation does not affect table', async function () { let tree = render(); let table = tree.getByRole('grid'); @@ -3597,6 +3603,7 @@ describe('TableView', function () { fireEvent.keyUp(document.activeElement, {key: 'Escape'}); act(() => jest.runAllTimers()); + await act(async () => Promise.resolve()); expect(menu).not.toBeInTheDocument(); expect(document.activeElement).toBe(within(rows[1]).getByRole('button')); From e2c7c23a2245bea7f01111f488e43b9005c0fa5b Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 16 Aug 2022 21:08:35 -0700 Subject: [PATCH 10/18] Revert "only one node removal mutation observer" This reverts commit 3123afdec12735cf954c2cfc6300014958db9cfb. --- .../utils/src/useSyntheticBlurEvent.ts | 155 +++++++----------- .../datepicker/test/DatePicker.test.js | 14 +- .../datepicker/test/DateRangePicker.test.js | 9 +- .../list/test/ListView.test.js | 3 +- .../@react-spectrum/table/test/Table.test.js | 23 +-- 5 files changed, 76 insertions(+), 128 deletions(-) diff --git a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts index 8b1912bcd80..3cf0de07254 100644 --- a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts +++ b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts @@ -61,97 +61,66 @@ class SyntheticBlurEvent implements ReactFocusEvent { persist() {} } -type Listener = (mutationList: MutationRecord[], target: HTMLElement) => boolean; - export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { let stateRef = useRef({ isFocused: false, onBlur, - disabledObserver: null as MutationObserver, + observer: null as MutationObserver, + removalObserver: null as MutationObserver, target: null as HTMLElement }); stateRef.current.onBlur = onBlur; - let domNodeRemovedHandler = useCallback((mutationList, target) => { - if (stateRef.current.isFocused && target) { - for (const mutation of mutationList) { - for (const node of mutation.removedNodes) { - if (node?.contains?.(target)) { - domNodeRemovedObserver.removeListener(domNodeRemovedHandler); - // fire blur which will be picked up by the 'once' blur handler defined below - target.dispatchEvent(new FocusEvent('blur')); - target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); - // early return so we don't look at the rest of the DOM - return true; - } - } + // Clean up MutationObserver on unmount. See below. + // eslint-disable-next-line arrow-body-style + useLayoutEffect(() => { + const state = stateRef.current; + return () => { + if (stateRef.current.target) { + stateRef.current.target.removeEventListener('focusout', onBlurHandler); } - } - return false; - }, [stateRef]); + if (state.observer) { + state.observer.disconnect(); + state.observer = null; + } + if (state.removalObserver) { + state.removalObserver.disconnect(); + state.removalObserver = null; + } + }; + }, []); + let onBlurHandler = useCallback((e: FocusEvent) => { let target = e.target; stateRef.current.isFocused = false; - if ( - ( - (target instanceof HTMLButtonElement || - target instanceof HTMLInputElement || - target instanceof HTMLTextAreaElement || - target instanceof HTMLSelectElement) && - target.disabled - ) || - !document.body.contains(e.target as HTMLElement) - ) { + if (((target instanceof HTMLButtonElement || + target instanceof HTMLInputElement || + target instanceof HTMLTextAreaElement || + target instanceof HTMLSelectElement) && target.disabled) || !document.body.contains(e.target as HTMLElement)) { // For backward compatibility, dispatch a (fake) React synthetic event. stateRef.current.onBlur?.(new SyntheticBlurEvent('blur', e)); } // We no longer need the MutationObserver once the target is blurred. - if (stateRef.current.disabledObserver) { - stateRef.current.disabledObserver.disconnect(); - stateRef.current.disabledObserver = null; + if (stateRef.current.observer) { + stateRef.current.observer.disconnect(); + stateRef.current.observer = null; } - domNodeRemovedObserver.removeListener(domNodeRemovedHandler); + // We no longer need the MutationObserver once the target is blurred. + if (stateRef.current.removalObserver) { + stateRef.current.removalObserver.disconnect(); + stateRef.current.removalObserver = null; + } }, [stateRef]); - // Clean up MutationObserver on unmount. See below. - useLayoutEffect(() => { - const state = stateRef.current; - - return () => { - if (stateRef.current.isFocused) { - stateRef.current.isFocused = false; - } - if (stateRef.current.target) { - stateRef.current.target.removeEventListener('focusout', onBlurHandler); - stateRef.current.target = null; - } - if (state.disabledObserver) { - state.disabledObserver.disconnect(); - state.disabledObserver = null; - } - domNodeRemovedObserver.removeListener(domNodeRemovedHandler); - }; - }, [domNodeRemovedHandler]); - // This function is called during a React onFocus event. return useCallback((e: ReactFocusEvent) => { let target = e.target; - stateRef.current.isFocused = true; - - if (stateRef.current.target && target !== stateRef.current.target) { - stateRef.current.target.removeEventListener('focusout', onBlurHandler); - target.addEventListener('focusout', onBlurHandler, {once: true}); - } else if (!stateRef.current.target || e.target !== stateRef.current.target) { - e.target.addEventListener('focusout', onBlurHandler, {once: true}); - } - stateRef.current.target = target as HTMLElement; - // React does not fire onBlur when an element is disabled. https://github.com/facebook/react/issues/9142 // Most browsers fire a native focusout event in this case, except for Firefox. In that case, we use a // MutationObserver to watch for the disabled attribute, and dispatch these events ourselves. @@ -162,52 +131,46 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { e.target instanceof HTMLTextAreaElement || e.target instanceof HTMLSelectElement ) { + stateRef.current.isFocused = true; - stateRef.current.disabledObserver = new MutationObserver(() => { + target.addEventListener('focusout', onBlurHandler, {once: true}); + + stateRef.current.observer = new MutationObserver(() => { if (stateRef.current.isFocused && (target as any).disabled) { - stateRef.current.disabledObserver.disconnect(); + stateRef.current.observer.disconnect(); target.dispatchEvent(new FocusEvent('blur')); target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); } }); - stateRef.current.disabledObserver.observe(target, {attributes: true, attributeFilter: ['disabled']}); + stateRef.current.observer.observe(target, {attributes: true, attributeFilter: ['disabled']}); } // Some browsers do not fire onBlur when the document.activeElement is removed from the dom. // Firefox has had a bug open about it for 13 years https://bugzilla.mozilla.org/show_bug.cgi?id=559561 // A Safari bug has been logged for it as well https://bugs.webkit.org/show_bug.cgi?id=243749 - domNodeRemovedObserver.addListener(domNodeRemovedHandler, target); - }, []); -} - - -class DOMNodeRemovedObserver { - private observer; - public listeners = new Map(); - constructor() { - this.observer = new MutationObserver((mutationList) => { - for (let [listener, target] of this.listeners) { - if (listener(mutationList, target)) { - // early return if we find a place to call blur - return; + stateRef.current.removalObserver = new MutationObserver((mutationList) => { + if (stateRef.current.isFocused) { + for (const mutation of mutationList) { + for (const node of mutation.removedNodes) { + if (node?.contains?.(target)) { + // this can be null because mutation observers fire async, so we might've disconnected and set to null before + // this fires, but it's still been scheduled by an event that happened before we disconnected + if (stateRef.current.removalObserver) { + stateRef.current.removalObserver.disconnect(); + stateRef.current.removalObserver = null; + // fire blur to cleanup any other synthetic blur handlers + target.dispatchEvent(new FocusEvent('blur')); + target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); + // early return so we don't look at the rest of the DOM + return; + } + } + } } } }); - } - addListener(listener, target) { - if (this.listeners.size <= 0) { - this.observer.observe(document, {childList: true, subtree: true, attributes: false, characterData: false}); - } - this.listeners.set(listener, target); - } - removeListener(listener) { - if (this.listeners.has(listener)) { - this.listeners.delete(listener); - } - if (this.listeners.size <= 0) { - this.observer.disconnect(); - } - } + + stateRef.current.removalObserver.observe(document, {childList: true, subtree: true, attributes: false, characterData: false}); + }, []); } -let domNodeRemovedObserver = new DOMNodeRemovedObserver(); diff --git a/packages/@react-spectrum/datepicker/test/DatePicker.test.js b/packages/@react-spectrum/datepicker/test/DatePicker.test.js index 1e564317499..57ae559db68 100644 --- a/packages/@react-spectrum/datepicker/test/DatePicker.test.js +++ b/packages/@react-spectrum/datepicker/test/DatePicker.test.js @@ -55,7 +55,7 @@ function render(el) { describe('DatePicker', function () { beforeAll(() => { - jest.useFakeTimers(); + jest.useFakeTimers('legacy'); }); afterEach(() => { act(() => { @@ -186,7 +186,7 @@ describe('DatePicker', function () { }); describe('calendar popover', function () { - it('should emit onChange when selecting a date in the calendar in controlled mode', async function () { + it('should emit onChange when selecting a date in the calendar in controlled mode', function () { let onChange = jest.fn(); let {getByRole, getAllByRole, queryByLabelText} = render( @@ -215,10 +215,9 @@ describe('DatePicker', function () { expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenCalledWith(new CalendarDate(2019, 2, 4)); expect(getTextValue(combobox)).toBe('2/3/2019'); // controlled - await act(async () => Promise.resolve()); }); - it('should emit onChange when selecting a date in the calendar in uncontrolled mode', async function () { + it('should emit onChange when selecting a date in the calendar in uncontrolled mode', function () { let onChange = jest.fn(); let {getByRole, getAllByRole} = render( @@ -245,7 +244,6 @@ describe('DatePicker', function () { expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenCalledWith(new CalendarDate(2019, 2, 4)); expect(getTextValue(combobox)).toBe('2/4/2019'); // uncontrolled - await act(async () => Promise.resolve()); }); it('should display a time field when a CalendarDateTime value is used', function () { @@ -373,7 +371,7 @@ describe('DatePicker', function () { expectPlaceholder(combobox, formatter.format(value.toDate(getLocalTimeZone()))); }); - it('should confirm time placeholder on blur if date is selected', async function () { + it('should confirm time placeholder on blur if date is selected', function () { let onChange = jest.fn(); let {getByRole, getAllByRole} = render( @@ -400,7 +398,6 @@ describe('DatePicker', function () { userEvent.click(document.body); act(() => jest.runAllTimers()); - await act(async () => Promise.resolve()); expect(dialog).not.toBeInTheDocument(); @@ -410,7 +407,7 @@ describe('DatePicker', function () { expectPlaceholder(combobox, formatter.format(value.toDate(getLocalTimeZone()))); }); - it('should not confirm on blur if date is not selected', async function () { + it('should not confirm on blur if date is not selected', function () { let onChange = jest.fn(); let {getByRole, getAllByRole, getAllByLabelText} = render( @@ -443,7 +440,6 @@ describe('DatePicker', function () { userEvent.click(document.body); act(() => jest.runAllTimers()); - await act(async () => Promise.resolve()); expect(dialog).not.toBeInTheDocument(); expect(onChange).not.toHaveBeenCalled(); diff --git a/packages/@react-spectrum/datepicker/test/DateRangePicker.test.js b/packages/@react-spectrum/datepicker/test/DateRangePicker.test.js index 5343512d5c9..0237a47607a 100644 --- a/packages/@react-spectrum/datepicker/test/DateRangePicker.test.js +++ b/packages/@react-spectrum/datepicker/test/DateRangePicker.test.js @@ -264,7 +264,7 @@ describe('DateRangePicker', function () { }); describe('calendar popover', function () { - it('should emit onChange when selecting a date range in the calendar in uncontrolled mode', async function () { + it('should emit onChange when selecting a date range in the calendar in uncontrolled mode', function () { let onChange = jest.fn(); let {getByRole, getByTestId, getAllByRole, getByLabelText} = render( @@ -295,7 +295,6 @@ describe('DateRangePicker', function () { expect(onChange).toHaveBeenCalledWith({start: new CalendarDate(2019, 2, 10), end: new CalendarDate(2019, 2, 17)}); expect(getTextValue(startDate)).toBe('2/10/2019'); // uncontrolled expect(getTextValue(endDate)).toBe('2/17/2019'); - await act(async () => Promise.resolve()); }); it('should display time fields when a CalendarDateTime value is used', function () { @@ -458,7 +457,7 @@ describe('DateRangePicker', function () { expect(getTextValue(endDate)).toBe(formatter.format(endValue.toDate(getLocalTimeZone()))); }); - it('should confirm time placeholders on blur if date range is selected', async function () { + it('should confirm time placeholders on blur if date range is selected', function () { let onChange = jest.fn(); let {getByRole, getAllByRole, getByTestId} = render( @@ -487,7 +486,6 @@ describe('DateRangePicker', function () { userEvent.click(document.body); act(() => jest.runAllTimers()); - await act(async () => Promise.resolve()); expect(dialog).not.toBeInTheDocument(); @@ -499,7 +497,7 @@ describe('DateRangePicker', function () { expect(getTextValue(endDate)).toBe(formatter.format(endValue.toDate(getLocalTimeZone()))); }); - it('should not confirm on blur if date range is not selected', async function () { + it('should not confirm on blur if date range is not selected', function () { let onChange = jest.fn(); let {getByRole, getAllByLabelText, getByTestId} = render( @@ -534,7 +532,6 @@ describe('DateRangePicker', function () { userEvent.click(document.body); act(() => jest.runAllTimers()); - await act(async () => Promise.resolve()); expect(dialog).not.toBeInTheDocument(); expect(onChange).not.toHaveBeenCalled(); diff --git a/packages/@react-spectrum/list/test/ListView.test.js b/packages/@react-spectrum/list/test/ListView.test.js index 859e8760e89..3875e23c49b 100644 --- a/packages/@react-spectrum/list/test/ListView.test.js +++ b/packages/@react-spectrum/list/test/ListView.test.js @@ -1265,7 +1265,7 @@ describe('ListView', function () { }); describe('scrolling', function () { - it('should scroll to a row when it is focused', async function () { + it('should scroll to a row when it is focused', function () { let tree = render( Promise.resolve()); }); it('should scroll to a row when it is focused', function () { diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index 854259adcbe..74438b6a5b5 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -121,8 +121,7 @@ describe('TableView', function () { offsetHeight.mockReset(); }); - afterEach(async () => { - await act(async () => Promise.resolve()); + afterEach(() => { act(() => {jest.runAllTimers();}); }); @@ -1566,7 +1565,7 @@ describe('TableView', function () { expect(body.scrollTop).toBe(24); }); - it('should scroll to a cell when it is focused off screen', async function () { + it('should scroll to a cell when it is focused off screen', function () { let tree = renderManyColumns(); let body = tree.getByRole('grid').childNodes[1]; @@ -1575,7 +1574,7 @@ describe('TableView', function () { expect(document.activeElement).toBe(cell); expect(body.scrollTop).toBe(0); - // When scrolling the focused item out of view, focus should remain on the item, + // When scrolling the focused item out of view, focus should remaind on the item, // virtualizer keeps focused items from being reused body.scrollTop = 1000; body.scrollLeft = 1000; @@ -1599,8 +1598,6 @@ describe('TableView', function () { moveFocus('ArrowLeft'); expect(body.scrollTop).toBe(164); expect(document.activeElement).toBe(getCell(tree, 'Foo 5 4')); - act(() => {jest.runAllTimers();}); - await act(async () => Promise.resolve()); }); it('should not scroll when a column header receives focus', function () { @@ -2277,7 +2274,7 @@ describe('TableView', function () { checkRowSelection(rows.slice(1), true); }); - it('manually selecting all should not auto select new items', async function () { + it('manually selecting all should not auto select new items', function () { let onSelectionChange = jest.fn(); let tree = renderTable({onSelectionChange}, items); @@ -2298,7 +2295,6 @@ describe('TableView', function () { ])); act(() => jest.runAllTimers()); - await act(async () => Promise.resolve()); rows = tree.getAllByRole('row'); expect(getCell(tree, 'Foo 0')).toBeVisible(); @@ -2326,7 +2322,7 @@ describe('TableView', function () { }); }); - describe('announcements', function () { + describe('annoucements', function () { it('should announce the selected or deselected row', function () { let onSelectionChange = jest.fn(); let tree = renderTable({onSelectionChange}); @@ -2463,7 +2459,7 @@ describe('TableView', function () { expect(announce).toHaveBeenCalledTimes(2); }); - it('updates even if not focused', async () => { + it('updates even if not focused', () => { let tree = render(); let link = tree.getAllByRole('link')[1]; @@ -2500,7 +2496,6 @@ describe('TableView', function () { // TableWithBreadcrumbs has a setTimeout to load the results of the link navigation on Folder A jest.runAllTimers(); }); - await act(async () => Promise.resolve()); expect(announce).toHaveBeenCalledTimes(3); expect(announce).toHaveBeenLastCalledWith('No items selected.'); @@ -3449,7 +3444,7 @@ describe('TableView', function () { expect(checkbox.checked).toBe(false); }); - it('can edit items', async function () { + it('can edit items', function () { let tree = render(); let table = tree.getByRole('grid'); @@ -3480,7 +3475,6 @@ describe('TableView', function () { expect(dialog).not.toBeInTheDocument(); act(() => {jest.runAllTimers();}); - await act(async () => Promise.resolve()); let rowHeaders = within(rows[2]).getAllByRole('rowheader'); expect(rowHeaders[0]).toHaveTextContent('Jessica'); @@ -3577,7 +3571,7 @@ describe('TableView', function () { expect(document.activeElement).toBe(within(menu).getAllByRole('menuitem').pop()); }); - it('menu keyboard navigation does not affect table', async function () { + it('menu keyboard navigation does not affect table', function () { let tree = render(); let table = tree.getByRole('grid'); @@ -3603,7 +3597,6 @@ describe('TableView', function () { fireEvent.keyUp(document.activeElement, {key: 'Escape'}); act(() => jest.runAllTimers()); - await act(async () => Promise.resolve()); expect(menu).not.toBeInTheDocument(); expect(document.activeElement).toBe(within(rows[1]).getByRole('button')); From a8449f3ad756bebf1143eb76cf7cfb862e470345 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 17 Aug 2022 09:47:04 -0700 Subject: [PATCH 11/18] clean up remaining event listeners --- .../utils/src/useSyntheticBlurEvent.ts | 12 +++++++--- .../datepicker/test/DatePicker.test.js | 14 +++++++---- .../datepicker/test/DateRangePicker.test.js | 9 +++++--- .../list/test/ListView.test.js | 3 ++- .../@react-spectrum/table/test/Table.test.js | 23 ++++++++++++------- 5 files changed, 41 insertions(+), 20 deletions(-) diff --git a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts index 3cf0de07254..90d8eeef98c 100644 --- a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts +++ b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts @@ -120,6 +120,15 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { return useCallback((e: ReactFocusEvent) => { let target = e.target; + stateRef.current.isFocused = true; + + if (stateRef.current.target && target !== stateRef.current.target) { + stateRef.current.target.removeEventListener('focusout', onBlurHandler); + target.addEventListener('focusout', onBlurHandler, {once: true}); + } else if (!stateRef.current.target || e.target !== stateRef.current.target) { + target.addEventListener('focusout', onBlurHandler, {once: true}); + } + stateRef.current.target = target as HTMLElement; // React does not fire onBlur when an element is disabled. https://github.com/facebook/react/issues/9142 // Most browsers fire a native focusout event in this case, except for Firefox. In that case, we use a @@ -131,9 +140,6 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { e.target instanceof HTMLTextAreaElement || e.target instanceof HTMLSelectElement ) { - stateRef.current.isFocused = true; - - target.addEventListener('focusout', onBlurHandler, {once: true}); stateRef.current.observer = new MutationObserver(() => { if (stateRef.current.isFocused && (target as any).disabled) { diff --git a/packages/@react-spectrum/datepicker/test/DatePicker.test.js b/packages/@react-spectrum/datepicker/test/DatePicker.test.js index 57ae559db68..1e564317499 100644 --- a/packages/@react-spectrum/datepicker/test/DatePicker.test.js +++ b/packages/@react-spectrum/datepicker/test/DatePicker.test.js @@ -55,7 +55,7 @@ function render(el) { describe('DatePicker', function () { beforeAll(() => { - jest.useFakeTimers('legacy'); + jest.useFakeTimers(); }); afterEach(() => { act(() => { @@ -186,7 +186,7 @@ describe('DatePicker', function () { }); describe('calendar popover', function () { - it('should emit onChange when selecting a date in the calendar in controlled mode', function () { + it('should emit onChange when selecting a date in the calendar in controlled mode', async function () { let onChange = jest.fn(); let {getByRole, getAllByRole, queryByLabelText} = render( @@ -215,9 +215,10 @@ describe('DatePicker', function () { expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenCalledWith(new CalendarDate(2019, 2, 4)); expect(getTextValue(combobox)).toBe('2/3/2019'); // controlled + await act(async () => Promise.resolve()); }); - it('should emit onChange when selecting a date in the calendar in uncontrolled mode', function () { + it('should emit onChange when selecting a date in the calendar in uncontrolled mode', async function () { let onChange = jest.fn(); let {getByRole, getAllByRole} = render( @@ -244,6 +245,7 @@ describe('DatePicker', function () { expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenCalledWith(new CalendarDate(2019, 2, 4)); expect(getTextValue(combobox)).toBe('2/4/2019'); // uncontrolled + await act(async () => Promise.resolve()); }); it('should display a time field when a CalendarDateTime value is used', function () { @@ -371,7 +373,7 @@ describe('DatePicker', function () { expectPlaceholder(combobox, formatter.format(value.toDate(getLocalTimeZone()))); }); - it('should confirm time placeholder on blur if date is selected', function () { + it('should confirm time placeholder on blur if date is selected', async function () { let onChange = jest.fn(); let {getByRole, getAllByRole} = render( @@ -398,6 +400,7 @@ describe('DatePicker', function () { userEvent.click(document.body); act(() => jest.runAllTimers()); + await act(async () => Promise.resolve()); expect(dialog).not.toBeInTheDocument(); @@ -407,7 +410,7 @@ describe('DatePicker', function () { expectPlaceholder(combobox, formatter.format(value.toDate(getLocalTimeZone()))); }); - it('should not confirm on blur if date is not selected', function () { + it('should not confirm on blur if date is not selected', async function () { let onChange = jest.fn(); let {getByRole, getAllByRole, getAllByLabelText} = render( @@ -440,6 +443,7 @@ describe('DatePicker', function () { userEvent.click(document.body); act(() => jest.runAllTimers()); + await act(async () => Promise.resolve()); expect(dialog).not.toBeInTheDocument(); expect(onChange).not.toHaveBeenCalled(); diff --git a/packages/@react-spectrum/datepicker/test/DateRangePicker.test.js b/packages/@react-spectrum/datepicker/test/DateRangePicker.test.js index 0237a47607a..ab07411ac6e 100644 --- a/packages/@react-spectrum/datepicker/test/DateRangePicker.test.js +++ b/packages/@react-spectrum/datepicker/test/DateRangePicker.test.js @@ -264,7 +264,7 @@ describe('DateRangePicker', function () { }); describe('calendar popover', function () { - it('should emit onChange when selecting a date range in the calendar in uncontrolled mode', function () { + it('should emit onChange when selecting a date range in the calendar in uncontrolled mode', async function () { let onChange = jest.fn(); let {getByRole, getByTestId, getAllByRole, getByLabelText} = render( @@ -289,6 +289,7 @@ describe('DateRangePicker', function () { triggerPress(getByLabelText('Sunday, February 10, 2019 selected')); triggerPress(getByLabelText('Sunday, February 17, 2019')); + await act(async () => Promise.resolve()); expect(dialog).not.toBeInTheDocument(); expect(onChange).toHaveBeenCalledTimes(1); @@ -457,7 +458,7 @@ describe('DateRangePicker', function () { expect(getTextValue(endDate)).toBe(formatter.format(endValue.toDate(getLocalTimeZone()))); }); - it('should confirm time placeholders on blur if date range is selected', function () { + it('should confirm time placeholders on blur if date range is selected', async function () { let onChange = jest.fn(); let {getByRole, getAllByRole, getByTestId} = render( @@ -486,6 +487,7 @@ describe('DateRangePicker', function () { userEvent.click(document.body); act(() => jest.runAllTimers()); + await act(async () => Promise.resolve()); expect(dialog).not.toBeInTheDocument(); @@ -497,7 +499,7 @@ describe('DateRangePicker', function () { expect(getTextValue(endDate)).toBe(formatter.format(endValue.toDate(getLocalTimeZone()))); }); - it('should not confirm on blur if date range is not selected', function () { + it('should not confirm on blur if date range is not selected', async function () { let onChange = jest.fn(); let {getByRole, getAllByLabelText, getByTestId} = render( @@ -532,6 +534,7 @@ describe('DateRangePicker', function () { userEvent.click(document.body); act(() => jest.runAllTimers()); + await act(async () => Promise.resolve()); expect(dialog).not.toBeInTheDocument(); expect(onChange).not.toHaveBeenCalled(); diff --git a/packages/@react-spectrum/list/test/ListView.test.js b/packages/@react-spectrum/list/test/ListView.test.js index 3875e23c49b..859e8760e89 100644 --- a/packages/@react-spectrum/list/test/ListView.test.js +++ b/packages/@react-spectrum/list/test/ListView.test.js @@ -1265,7 +1265,7 @@ describe('ListView', function () { }); describe('scrolling', function () { - it('should scroll to a row when it is focused', function () { + it('should scroll to a row when it is focused', async function () { let tree = render( Promise.resolve()); }); it('should scroll to a row when it is focused', function () { diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index 74438b6a5b5..854259adcbe 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -121,7 +121,8 @@ describe('TableView', function () { offsetHeight.mockReset(); }); - afterEach(() => { + afterEach(async () => { + await act(async () => Promise.resolve()); act(() => {jest.runAllTimers();}); }); @@ -1565,7 +1566,7 @@ describe('TableView', function () { expect(body.scrollTop).toBe(24); }); - it('should scroll to a cell when it is focused off screen', function () { + it('should scroll to a cell when it is focused off screen', async function () { let tree = renderManyColumns(); let body = tree.getByRole('grid').childNodes[1]; @@ -1574,7 +1575,7 @@ describe('TableView', function () { expect(document.activeElement).toBe(cell); expect(body.scrollTop).toBe(0); - // When scrolling the focused item out of view, focus should remaind on the item, + // When scrolling the focused item out of view, focus should remain on the item, // virtualizer keeps focused items from being reused body.scrollTop = 1000; body.scrollLeft = 1000; @@ -1598,6 +1599,8 @@ describe('TableView', function () { moveFocus('ArrowLeft'); expect(body.scrollTop).toBe(164); expect(document.activeElement).toBe(getCell(tree, 'Foo 5 4')); + act(() => {jest.runAllTimers();}); + await act(async () => Promise.resolve()); }); it('should not scroll when a column header receives focus', function () { @@ -2274,7 +2277,7 @@ describe('TableView', function () { checkRowSelection(rows.slice(1), true); }); - it('manually selecting all should not auto select new items', function () { + it('manually selecting all should not auto select new items', async function () { let onSelectionChange = jest.fn(); let tree = renderTable({onSelectionChange}, items); @@ -2295,6 +2298,7 @@ describe('TableView', function () { ])); act(() => jest.runAllTimers()); + await act(async () => Promise.resolve()); rows = tree.getAllByRole('row'); expect(getCell(tree, 'Foo 0')).toBeVisible(); @@ -2322,7 +2326,7 @@ describe('TableView', function () { }); }); - describe('annoucements', function () { + describe('announcements', function () { it('should announce the selected or deselected row', function () { let onSelectionChange = jest.fn(); let tree = renderTable({onSelectionChange}); @@ -2459,7 +2463,7 @@ describe('TableView', function () { expect(announce).toHaveBeenCalledTimes(2); }); - it('updates even if not focused', () => { + it('updates even if not focused', async () => { let tree = render(); let link = tree.getAllByRole('link')[1]; @@ -2496,6 +2500,7 @@ describe('TableView', function () { // TableWithBreadcrumbs has a setTimeout to load the results of the link navigation on Folder A jest.runAllTimers(); }); + await act(async () => Promise.resolve()); expect(announce).toHaveBeenCalledTimes(3); expect(announce).toHaveBeenLastCalledWith('No items selected.'); @@ -3444,7 +3449,7 @@ describe('TableView', function () { expect(checkbox.checked).toBe(false); }); - it('can edit items', function () { + it('can edit items', async function () { let tree = render(); let table = tree.getByRole('grid'); @@ -3475,6 +3480,7 @@ describe('TableView', function () { expect(dialog).not.toBeInTheDocument(); act(() => {jest.runAllTimers();}); + await act(async () => Promise.resolve()); let rowHeaders = within(rows[2]).getAllByRole('rowheader'); expect(rowHeaders[0]).toHaveTextContent('Jessica'); @@ -3571,7 +3577,7 @@ describe('TableView', function () { expect(document.activeElement).toBe(within(menu).getAllByRole('menuitem').pop()); }); - it('menu keyboard navigation does not affect table', function () { + it('menu keyboard navigation does not affect table', async function () { let tree = render(); let table = tree.getByRole('grid'); @@ -3597,6 +3603,7 @@ describe('TableView', function () { fireEvent.keyUp(document.activeElement, {key: 'Escape'}); act(() => jest.runAllTimers()); + await act(async () => Promise.resolve()); expect(menu).not.toBeInTheDocument(); expect(document.activeElement).toBe(within(rows[1]).getByRole('button')); From 96feabf161faa469bfe38bf2ab9f898959c1ef2f Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 17 Aug 2022 15:19:51 -0700 Subject: [PATCH 12/18] Single node removed observer --- .../utils/src/useSyntheticBlurEvent.ts | 71 ++++++++++++------- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts index 90d8eeef98c..68199ae76ed 100644 --- a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts +++ b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts @@ -83,10 +83,6 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { state.observer.disconnect(); state.observer = null; } - if (state.removalObserver) { - state.removalObserver.disconnect(); - state.removalObserver = null; - } }; }, []); @@ -108,12 +104,6 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { stateRef.current.observer.disconnect(); stateRef.current.observer = null; } - - // We no longer need the MutationObserver once the target is blurred. - if (stateRef.current.removalObserver) { - stateRef.current.removalObserver.disconnect(); - stateRef.current.removalObserver = null; - } }, [stateRef]); // This function is called during a React onFocus event. @@ -124,6 +114,7 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { if (stateRef.current.target && target !== stateRef.current.target) { stateRef.current.target.removeEventListener('focusout', onBlurHandler); + domNodeRemovedObserver.removeTarget(stateRef.current.target); target.addEventListener('focusout', onBlurHandler, {once: true}); } else if (!stateRef.current.target || e.target !== stateRef.current.target) { target.addEventListener('focusout', onBlurHandler, {once: true}); @@ -155,28 +146,56 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { // Some browsers do not fire onBlur when the document.activeElement is removed from the dom. // Firefox has had a bug open about it for 13 years https://bugzilla.mozilla.org/show_bug.cgi?id=559561 // A Safari bug has been logged for it as well https://bugs.webkit.org/show_bug.cgi?id=243749 - stateRef.current.removalObserver = new MutationObserver((mutationList) => { - if (stateRef.current.isFocused) { + domNodeRemovedObserver.setTarget(target); + }, []); +} + + +class DOMNodeRemovedObserver { + private observer; + private target: HTMLElement; + + constructor() { + this.observer = new MutationObserver((mutationList) => { + if (this.target) { for (const mutation of mutationList) { for (const node of mutation.removedNodes) { - if (node?.contains?.(target)) { - // this can be null because mutation observers fire async, so we might've disconnected and set to null before - // this fires, but it's still been scheduled by an event that happened before we disconnected - if (stateRef.current.removalObserver) { - stateRef.current.removalObserver.disconnect(); - stateRef.current.removalObserver = null; - // fire blur to cleanup any other synthetic blur handlers - target.dispatchEvent(new FocusEvent('blur')); - target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); - // early return so we don't look at the rest of the DOM - return; - } + if (node?.contains?.(this.target)) { + // fire blur to cleanup any other synthetic blur handlers + this.target.dispatchEvent(new FocusEvent('blur')); + this.target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); + // early return so we don't look at the rest of the DOM + return; } } } } }); + this.onBlurHandler = this.onBlurHandler.bind(this); + } - stateRef.current.removalObserver.observe(document, {childList: true, subtree: true, attributes: false, characterData: false}); - }, []); + onBlurHandler() { + this.target = null; + this.observer.disconnect(); + } + setTarget(target: HTMLElement) { + if (!this.target && target) { + this.observer.observe(document, {childList: true, subtree: true, attributes: false, characterData: false}); + } + if (this.target !== target) { + if (this.target) { + this.target.removeEventListener('focusout', this.onBlurHandler); + } + target.addEventListener('focusout', this.onBlurHandler, {once: true}); + this.target = target; + } + } + removeTarget(target) { + if (this.target === target) { + this.onBlurHandler(); + } else if (target) { + target.removeEventListener('focusout', this.onBlurHandler); + } + } } +let domNodeRemovedObserver = new DOMNodeRemovedObserver(); From 4123cb56e01524a4059e0c813dec786df2320028 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 17 Aug 2022 16:36:30 -0700 Subject: [PATCH 13/18] fix lint --- packages/@react-aria/utils/src/useSyntheticBlurEvent.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts index 68199ae76ed..7f821e6d0ed 100644 --- a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts +++ b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts @@ -109,7 +109,7 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { // This function is called during a React onFocus event. return useCallback((e: ReactFocusEvent) => { - let target = e.target; + let target = e.target as HTMLElement; stateRef.current.isFocused = true; if (stateRef.current.target && target !== stateRef.current.target) { @@ -120,7 +120,7 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { target.addEventListener('focusout', onBlurHandler, {once: true}); } - stateRef.current.target = target as HTMLElement; + stateRef.current.target = target; // React does not fire onBlur when an element is disabled. https://github.com/facebook/react/issues/9142 // Most browsers fire a native focusout event in this case, except for Firefox. In that case, we use a // MutationObserver to watch for the disabled attribute, and dispatch these events ourselves. From 74eeb89e7fee3ef46cd1b1618a3fa2a91d977fe2 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 17 Aug 2022 16:56:36 -0700 Subject: [PATCH 14/18] fix ssr --- .../utils/src/useSyntheticBlurEvent.ts | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts index 7f821e6d0ed..549fe039cf4 100644 --- a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts +++ b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts @@ -156,30 +156,34 @@ class DOMNodeRemovedObserver { private target: HTMLElement; constructor() { - this.observer = new MutationObserver((mutationList) => { - if (this.target) { - for (const mutation of mutationList) { - for (const node of mutation.removedNodes) { - if (node?.contains?.(this.target)) { - // fire blur to cleanup any other synthetic blur handlers - this.target.dispatchEvent(new FocusEvent('blur')); - this.target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); - // early return so we don't look at the rest of the DOM - return; + if (typeof window !== 'undefined') { + this.observer = new MutationObserver((mutationList) => { + if (this.target) { + for (const mutation of mutationList) { + for (const node of mutation.removedNodes) { + if (node?.contains?.(this.target)) { + // fire blur to cleanup any other synthetic blur handlers + this.target.dispatchEvent(new FocusEvent('blur')); + this.target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); + // early return so we don't look at the rest of the DOM + return; + } } } } - } - }); + }); + } this.onBlurHandler = this.onBlurHandler.bind(this); } onBlurHandler() { this.target = null; - this.observer.disconnect(); + if (this.observer) { + this.observer.disconnect(); + } } setTarget(target: HTMLElement) { - if (!this.target && target) { + if (!this.target && target && this.observer) { this.observer.observe(document, {childList: true, subtree: true, attributes: false, characterData: false}); } if (this.target !== target) { From 7c835942db17d00354b07b90c86333cd66bbf62f Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 17 Aug 2022 17:01:00 -0700 Subject: [PATCH 15/18] remove dead code and fix comments/types --- .../@react-aria/utils/src/useSyntheticBlurEvent.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts index 549fe039cf4..17947a4ff5e 100644 --- a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts +++ b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts @@ -66,7 +66,6 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { isFocused: false, onBlur, observer: null as MutationObserver, - removalObserver: null as MutationObserver, target: null as HTMLElement }); stateRef.current.onBlur = onBlur; @@ -152,17 +151,17 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { class DOMNodeRemovedObserver { - private observer; - private target: HTMLElement; + private observer: MutationObserver | null; + private target: HTMLElement | null; constructor() { + // don't make an observer in SSR (MutationObserver is not supported in Node) if (typeof window !== 'undefined') { this.observer = new MutationObserver((mutationList) => { if (this.target) { for (const mutation of mutationList) { for (const node of mutation.removedNodes) { if (node?.contains?.(this.target)) { - // fire blur to cleanup any other synthetic blur handlers this.target.dispatchEvent(new FocusEvent('blur')); this.target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); // early return so we don't look at the rest of the DOM @@ -197,7 +196,8 @@ class DOMNodeRemovedObserver { removeTarget(target) { if (this.target === target) { this.onBlurHandler(); - } else if (target) { + } + if (target) { target.removeEventListener('focusout', this.onBlurHandler); } } From 3340ac4153df0d722921a5432e25efa7a6c03676 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 17 Aug 2022 17:16:13 -0700 Subject: [PATCH 16/18] remove extraneous code --- packages/@react-aria/focus/src/FocusScope.tsx | 25 ++----------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/packages/@react-aria/focus/src/FocusScope.tsx b/packages/@react-aria/focus/src/FocusScope.tsx index e224e959aea..0546ac0ea3c 100644 --- a/packages/@react-aria/focus/src/FocusScope.tsx +++ b/packages/@react-aria/focus/src/FocusScope.tsx @@ -331,34 +331,13 @@ function useFocusContainment(scopeRef: RefObject, contain: boolean) { } }; - let onFocusDocument = (e) => { - // If focusing an element in a child scope of the currently active scope, the child becomes active. - // Moving out of the active scope to an ancestor is not allowed. - if (!activeScope || isAncestorScope(activeScope, scopeRef)) { - activeScope = scopeRef; - focusedNode.current = e.target; - onSyntheticFocusDocument(e); - } else if (shouldContainFocus(scopeRef) && !isElementInChildScope(e.target, scopeRef)) { - // If a focus event occurs outside the active scope (e.g. user tabs from browser location bar), - // restore focus to the previously focused node or the first tabbable element in the active scope. - if (focusedNode.current) { - focusedNode.current.focus(); - } else if (activeScope) { - focusFirstInScope(activeScope.current); - } - } else if (shouldContainFocus(scopeRef)) { - focusedNode.current = e.target; - onSyntheticFocusDocument(e); - } - }; - document.addEventListener('keydown', onKeyDown, false); - document.addEventListener('focusin', onFocusDocument, false); + document.addEventListener('focusin', onFocus, false); scope.forEach(element => element.addEventListener('focusin', onFocus, false)); scope.forEach(element => element.addEventListener('focusout', onBlur, false)); return () => { document.removeEventListener('keydown', onKeyDown, false); - document.removeEventListener('focusin', onFocusDocument, false); + document.removeEventListener('focusin', onFocus, false); scope.forEach(element => element.removeEventListener('focusin', onFocus, false)); scope.forEach(element => element.removeEventListener('focusout', onBlur, false)); }; From 799f8c2019ef93247df5b320ae237eaa5b761ec8 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 30 Aug 2022 15:36:12 -0700 Subject: [PATCH 17/18] code review --- packages/@react-aria/focus/src/FocusScope.tsx | 3 +-- packages/@react-aria/focus/stories/FocusScope.stories.tsx | 2 +- packages/@react-aria/utils/src/useSyntheticBlurEvent.ts | 3 +-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/@react-aria/focus/src/FocusScope.tsx b/packages/@react-aria/focus/src/FocusScope.tsx index 0546ac0ea3c..5adeee582e2 100644 --- a/packages/@react-aria/focus/src/FocusScope.tsx +++ b/packages/@react-aria/focus/src/FocusScope.tsx @@ -272,7 +272,6 @@ function useFocusContainment(scopeRef: RefObject, contain: boolean) { }); }, [scopeRef, focusedNode]); let onSyntheticFocus = useSyntheticBlurEvent(onBlur); - let onSyntheticFocusDocument = useSyntheticBlurEvent(onBlur); useLayoutEffect(() => { let scope = scopeRef.current; if (!contain) { @@ -357,7 +356,7 @@ function useFocusContainment(scopeRef: RefObject, contain: boolean) { // and attach our blur fixer here if (shouldContainFocus(scopeRef) && isElementInChildScope(document.activeElement, scopeRef)) { let event = {target: document.activeElement}; - onSyntheticFocusDocument(event as React.FocusEvent); + onSyntheticFocus(event as React.FocusEvent); } }, []); } diff --git a/packages/@react-aria/focus/stories/FocusScope.stories.tsx b/packages/@react-aria/focus/stories/FocusScope.stories.tsx index 60f801fbc6f..a525967d5dc 100644 --- a/packages/@react-aria/focus/stories/FocusScope.stories.tsx +++ b/packages/@react-aria/focus/stories/FocusScope.stories.tsx @@ -113,7 +113,7 @@ function FocusableFirstInScopeExample() { return (

Dialog {index + 1}

- {index > 2 ? + {index >= 2 ? ( <>

The end of the road.

diff --git a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts index 17947a4ff5e..3e68cd58b10 100644 --- a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts +++ b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts @@ -71,7 +71,6 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { stateRef.current.onBlur = onBlur; // Clean up MutationObserver on unmount. See below. - // eslint-disable-next-line arrow-body-style useLayoutEffect(() => { const state = stateRef.current; return () => { @@ -93,7 +92,7 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { if (((target instanceof HTMLButtonElement || target instanceof HTMLInputElement || target instanceof HTMLTextAreaElement || - target instanceof HTMLSelectElement) && target.disabled) || !document.body.contains(e.target as HTMLElement)) { + target instanceof HTMLSelectElement) && target.disabled) || !document.body.contains(target as HTMLElement)) { // For backward compatibility, dispatch a (fake) React synthetic event. stateRef.current.onBlur?.(new SyntheticBlurEvent('blur', e)); } From 14dbc0e06795a209fe5b616cd7dadc67ad24376d Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Mon, 12 Sep 2022 13:03:46 -0700 Subject: [PATCH 18/18] small perf benefits --- .../utils/src/useSyntheticBlurEvent.ts | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts index 3e68cd58b10..63166cc2e09 100644 --- a/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts +++ b/packages/@react-aria/utils/src/useSyntheticBlurEvent.ts @@ -110,6 +110,10 @@ export function useSyntheticBlurEvent(onBlur: (e: ReactFocusEvent) => void) { let target = e.target as HTMLElement; stateRef.current.isFocused = true; + if (!domNodeRemovedObserver) { + domNodeRemovedObserver = new DOMNodeRemovedObserver(); + } + if (stateRef.current.target && target !== stateRef.current.target) { stateRef.current.target.removeEventListener('focusout', onBlurHandler); domNodeRemovedObserver.removeTarget(stateRef.current.target); @@ -156,18 +160,10 @@ class DOMNodeRemovedObserver { constructor() { // don't make an observer in SSR (MutationObserver is not supported in Node) if (typeof window !== 'undefined') { - this.observer = new MutationObserver((mutationList) => { - if (this.target) { - for (const mutation of mutationList) { - for (const node of mutation.removedNodes) { - if (node?.contains?.(this.target)) { - this.target.dispatchEvent(new FocusEvent('blur')); - this.target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); - // early return so we don't look at the rest of the DOM - return; - } - } - } + this.observer = new MutationObserver(() => { + if (this.target && !document.body.contains(this.target)) { + this.target.dispatchEvent(new FocusEvent('blur')); + this.target.dispatchEvent(new FocusEvent('focusout', {bubbles: true})); } }); } @@ -198,7 +194,8 @@ class DOMNodeRemovedObserver { } if (target) { target.removeEventListener('focusout', this.onBlurHandler); + this.target = null; } } } -let domNodeRemovedObserver = new DOMNodeRemovedObserver(); +let domNodeRemovedObserver;