From 95e2d8429ac26f6551ea34f122353924047bbf6d Mon Sep 17 00:00:00 2001 From: Vincent Manapat Date: Fri, 25 Sep 2026 18:50:31 +0200 Subject: [PATCH] [fix] Modal: close only one modal per Escape key press When the app closed the top modal on Escape keydown, the keyup of the same key press closed the modal underneath as well. A modal now closes only for a key press that began while it was the active modal and whose keydown was not handled. --- .../src/exports/Modal/ModalContent.js | 20 ++- .../src/exports/Modal/__tests__/index-test.js | 149 +++++++++++++++++- 2 files changed, 165 insertions(+), 4 deletions(-) diff --git a/packages/react-native-web/src/exports/Modal/ModalContent.js b/packages/react-native-web/src/exports/Modal/ModalContent.js index 0ce3ee493c..07b94befee 100644 --- a/packages/react-native-web/src/exports/Modal/ModalContent.js +++ b/packages/react-native-web/src/exports/Modal/ModalContent.js @@ -29,18 +29,34 @@ const ModalContent: React.AbstractComponent< > = React.forwardRef((props, forwardedRef) => { const { active, children, onRequestClose, transparent, ...rest } = props; + const escapeKeyDownRef = React.useRef(null); + React.useEffect(() => { if (canUseDOM) { + const recordEscapeKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape' && !e.repeat) { + escapeKeyDownRef.current = active ? e : null; + } + }; const closeOnEscape = (e: KeyboardEvent) => { - if (active && e.key === 'Escape') { + if (e.key !== 'Escape') { + return; + } + const keyDown = escapeKeyDownRef.current; + escapeKeyDownRef.current = null; + if (active && keyDown != null && !keyDown.defaultPrevented) { e.stopPropagation(); if (onRequestClose) { onRequestClose(); } } }; + document.addEventListener('keydown', recordEscapeKeyDown, true); document.addEventListener('keyup', closeOnEscape, false); - return () => document.removeEventListener('keyup', closeOnEscape, false); + return () => { + document.removeEventListener('keydown', recordEscapeKeyDown, true); + document.removeEventListener('keyup', closeOnEscape, false); + }; } }, [active, onRequestClose]); 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..f781e6cc1a 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 @@ -7,8 +7,14 @@ import Modal from '..'; import React from 'react'; +import TextInput from '../../TextInput'; import { fireEvent, render } from '@testing-library/react'; +function pressEscape(target) { + fireEvent.keyDown(target, { key: 'Escape' }); + fireEvent.keyUp(target, { key: 'Escape' }); +} + describe('components/Modal', () => { test('visible by default', () => { const { getByTestId } = render( @@ -616,7 +622,7 @@ describe('components/Modal', () => { render(); - fireEvent.keyUp(document, { key: 'Escape' }); + pressEscape(document); expect(spy).toHaveBeenCalledTimes(1); }); @@ -632,7 +638,7 @@ describe('components/Modal', () => { ); - fireEvent.keyUp(document, { key: 'Escape' }); + pressEscape(document); expect(spyA).toHaveBeenCalledTimes(0); expect(spyB).toHaveBeenCalledTimes(1); @@ -675,9 +681,148 @@ describe('components/Modal', () => { fireEvent.animationEnd(animationAElement); fireEvent.animationEnd(animationBElement); + pressEscape(document); + + expect(spyA).toHaveBeenCalledTimes(0); + expect(spyB).toHaveBeenCalledTimes(1); + }); + + test('escape key fires onRequestClose when a TextInput is focused', () => { + const spy = jest.fn(); + + const { getByTestId } = render( + + + + ); + + pressEscape(getByTestId('input')); + + expect(spy).toHaveBeenCalledTimes(1); + }); + + test('escape key closes one modal per key press', () => { + const spyA = jest.fn(); + const spyB = jest.fn(); + + function TestComponent() { + const [visibleB, setVisibleB] = React.useState(true); + return ( + <> + + { + spyB(); + setVisibleB(false); + }} + visible={visibleB} + /> + + ); + } + + render(); + + pressEscape(document); + + expect(spyA).toHaveBeenCalledTimes(0); + expect(spyB).toHaveBeenCalledTimes(1); + }); + + test('escape key closes one modal per key press when the app handles it', () => { + const spyA = jest.fn(); + const spyB = jest.fn(); + + function TestComponent() { + const [visibleB, setVisibleB] = React.useState(true); + return ( + <> + + + { + if (e.key === 'Escape') { + e.preventDefault(); + setVisibleB(false); + } + }} + /> + + + ); + } + + const { getByTestId } = render(); + + fireEvent.keyDown(getByTestId('b'), { key: 'Escape' }); + fireEvent.keyUp(document, { key: 'Escape' }); + + expect(spyA).toHaveBeenCalledTimes(0); + expect(spyB).toHaveBeenCalledTimes(0); + }); + + test('holding escape closes one modal per key press', () => { + const spyA = jest.fn(); + const spyB = jest.fn(); + + function TestComponent() { + const [visibleB, setVisibleB] = React.useState(true); + return ( + <> + + { + spyB(); + setVisibleB(false); + }} + visible={visibleB} + /> + + ); + } + + render(); + + fireEvent.keyDown(document, { key: 'Escape' }); + fireEvent.keyDown(document, { key: 'Escape', repeat: true }); + fireEvent.keyDown(document, { key: 'Escape', repeat: true }); fireEvent.keyUp(document, { key: 'Escape' }); expect(spyA).toHaveBeenCalledTimes(0); expect(spyB).toHaveBeenCalledTimes(1); }); + + test('escape key does not fire onRequestClose when the keydown was handled', () => { + const spy = jest.fn(); + + const { getByTestId } = render( + + { + if (e.key === 'Escape') { + e.preventDefault(); + } + }} + /> + + ); + + pressEscape(getByTestId('a')); + + expect(spy).toHaveBeenCalledTimes(0); + }); + + test('escape key does not fire onRequestClose for a key press that began before the modal was active', () => { + const spy = jest.fn(); + + const { rerender } = render(); + + fireEvent.keyDown(document, { key: 'Escape' }); + rerender(); + fireEvent.keyUp(document, { key: 'Escape' }); + + expect(spy).toHaveBeenCalledTimes(0); + }); });