diff --git a/packages/react-native-web/src/exports/Modal/ModalContent.js b/packages/react-native-web/src/exports/Modal/ModalContent.js index 0ce3ee493c..f09888adb2 100644 --- a/packages/react-native-web/src/exports/Modal/ModalContent.js +++ b/packages/react-native-web/src/exports/Modal/ModalContent.js @@ -14,6 +14,7 @@ import * as React from 'react'; import View from '../View'; import StyleSheet from '../StyleSheet'; import canUseDOM from '../../modules/canUseDom'; +import useLayoutEffect from '../../modules/useLayoutEffect'; export type ModalContentProps = { ...ViewProps, @@ -29,7 +30,9 @@ const ModalContent: React.AbstractComponent< > = React.forwardRef((props, forwardedRef) => { const { active, children, onRequestClose, transparent, ...rest } = props; - React.useEffect(() => { + // A layout effect, so that Escape goes to the active modal from the commit + // that sets `role="dialog"`, not a task later. + useLayoutEffect(() => { if (canUseDOM) { const closeOnEscape = (e: KeyboardEvent) => { if (active && e.key === 'Escape') { diff --git a/packages/react-native-web/src/exports/Modal/ModalFocusTrap.js b/packages/react-native-web/src/exports/Modal/ModalFocusTrap.js index f596bf60d2..eb7a0f5018 100644 --- a/packages/react-native-web/src/exports/Modal/ModalFocusTrap.js +++ b/packages/react-native-web/src/exports/Modal/ModalFocusTrap.js @@ -14,6 +14,7 @@ import createElement from '../createElement'; import StyleSheet from '../StyleSheet'; import UIManager from '../UIManager'; import canUseDOM from '../../modules/canUseDom'; +import useLayoutEffect from '../../modules/useLayoutEffect'; /** * This Component is used to "wrap" the modal we're opening @@ -83,7 +84,9 @@ const ModalFocusTrap = ({ lastFocusedElement: null }); - React.useEffect(() => { + // A layout effect, so that focus is trapped from the commit that makes the + // modal active, not a task later. + useLayoutEffect(() => { if (canUseDOM) { const trapFocus = () => { // We should not trap focus if: diff --git a/packages/react-native-web/src/exports/Modal/__tests__/index-test.js b/packages/react-native-web/src/exports/Modal/__tests__/index-test.js index 9e82fcc5ba..f12af213b1 100644 --- a/packages/react-native-web/src/exports/Modal/__tests__/index-test.js +++ b/packages/react-native-web/src/exports/Modal/__tests__/index-test.js @@ -567,6 +567,86 @@ describe('components/Modal', () => { expect(document.activeElement).toBe(outsideElement); }); + test('focus is trapped in the same commit that makes the modal active', () => { + // Its layout effect runs in that commit, after the modal's own effects and + // before passive effects: what it sees is what an event arriving right + // after the commit would see. + function Probe({ active, onCommit }) { + React.useLayoutEffect(() => { + if (active) onCommit(); + }, [active, onCommit]); + return null; + } + + let seen; + const onCommit = () => { + const dialog = document.querySelector('[role="dialog"]'); + seen = dialog != null && dialog.contains(document.activeElement); + }; + + function Test({ visible }) { + const [active, setActive] = React.useState(false); + return ( + <> + + Outside + + setActive(true)} visible={visible}> + + Hello + + + + + ); + } + + const { rerender } = render(); + document.querySelector('[data-testid="outside"]').focus(); + rerender(); + + expect(seen).toBe(true); + }); + + test('escape goes to the new top modal in the same commit that makes it active', () => { + const spyA = jest.fn(); + const spyB = jest.fn(); + + // See the test above: an Escape right after the commit that makes modal B + // active. + function Probe({ active }) { + React.useLayoutEffect(() => { + if (active) { + document.dispatchEvent( + new KeyboardEvent('keyup', { bubbles: true, key: 'Escape' }) + ); + } + }, [active]); + return null; + } + + function Test({ visibleB }) { + const [activeB, setActiveB] = React.useState(false); + return ( + <> + + setActiveB(true)} + visible={visibleB} + /> + + + ); + } + + const { rerender } = render(); + rerender(); + + expect(spyA).toHaveBeenCalledTimes(0); + expect(spyB).toHaveBeenCalledTimes(1); + }); + test('creates portal outside of the react container', () => { const { container, baseElement } = render(