From a660377050378d6ea0b727779c55b8cb68128d51 Mon Sep 17 00:00:00 2001
From: James Ward <james@notjam.es>
Date: Thu, 26 Nov 2020 02:56:49 -0500
Subject: [PATCH] [fix] Modal: fix onRequestClose target if using animations

Close #1824
Fix #1817
---
 .../src/exports/Modal/ModalAnimation.js       | 29 +++++---
 .../src/exports/Modal/__tests__/index.js      | 72 ++++++++++++++++++-
 2 files changed, 90 insertions(+), 11 deletions(-)

diff --git a/packages/react-native-web/src/exports/Modal/ModalAnimation.js b/packages/react-native-web/src/exports/Modal/ModalAnimation.js
index a7d9b16a..e028a9fa 100644
--- a/packages/react-native-web/src/exports/Modal/ModalAnimation.js
+++ b/packages/react-native-web/src/exports/Modal/ModalAnimation.js
@@ -40,18 +40,27 @@ function ModalAnimation(props: ModalAnimationProps) {
 
   const isAnimated = animationType && animationType !== 'none';
 
-  const animationEndCallback = useCallback(() => {
-    if (visible) {
-      if (onShow) {
-        onShow();
+  const animationEndCallback = useCallback(
+    (e: any) => {
+      if (e && e.currentTarget !== e.target) {
+        // If the event was generated for something NOT this element we
+        // should ignore it as it's not relevant to us
+        return;
       }
-    } else {
-      setIsRendering(false);
-      if (onDismiss) {
-        onDismiss();
+
+      if (visible) {
+        if (onShow) {
+          onShow();
+        }
+      } else {
+        setIsRendering(false);
+        if (onDismiss) {
+          onDismiss();
+        }
       }
-    }
-  }, [onDismiss, onShow, visible]);
+    },
+    [onDismiss, onShow, visible]
+  );
 
   useEffect(() => {
     if (visible) {
diff --git a/packages/react-native-web/src/exports/Modal/__tests__/index.js b/packages/react-native-web/src/exports/Modal/__tests__/index.js
index 8aeceb06..41c11bb2 100644
--- a/packages/react-native-web/src/exports/Modal/__tests__/index.js
+++ b/packages/react-native-web/src/exports/Modal/__tests__/index.js
@@ -2,7 +2,7 @@
 
 import Modal from '..';
 import React from 'react';
-import { render } from '@testing-library/react';
+import { fireEvent, render } from '@testing-library/react';
 
 describe('components/Modal', () => {
   test('visible by default', () => {
@@ -545,4 +545,74 @@ describe('components/Modal', () => {
     expect(spy).toHaveBeenNthCalledWith(1, 'ref');
     expect(spy).toHaveBeenNthCalledWith(2, 'mount');
   });
+
+  test('escape key fires onRequestClose', () => {
+    const spy = jest.fn();
+
+    render(<Modal onRequestClose={spy} visible={true} />);
+
+    fireEvent.keyUp(document, { key: 'Escape' });
+
+    expect(spy).toHaveBeenCalledTimes(1);
+  });
+
+  test('escape key fires onRequestClose for top modal only', () => {
+    const spyA = jest.fn();
+    const spyB = jest.fn();
+
+    render(
+      <>
+        <Modal onRequestClose={spyA} visible={true} />
+        <Modal onRequestClose={spyB} visible={true} />
+      </>
+    );
+
+    fireEvent.keyUp(document, { key: 'Escape' });
+
+    expect(spyA).toHaveBeenCalledTimes(0);
+    expect(spyB).toHaveBeenCalledTimes(1);
+  });
+
+  test('escape key fires onRequestClose for top modal only with animation', () => {
+    const spyA = jest.fn();
+    const spyB = jest.fn();
+
+    const { getByTestId, rerender } = render(
+      <>
+        <Modal animationType={'slide'} onRequestClose={spyA} visible={false}>
+          <a data-testid={'a'} />
+
+          <Modal animationType={'slide'} onRequestClose={spyB} visible={false}>
+            <a data-testid={'b'} />
+          </Modal>
+        </Modal>
+      </>
+    );
+
+    rerender(
+      <>
+        <Modal animationType={'slide'} onRequestClose={spyA} visible={true}>
+          <a data-testid={'a'} />
+
+          <Modal animationType={'slide'} onRequestClose={spyB} visible={true}>
+            <a data-testid={'b'} />
+          </Modal>
+        </Modal>
+      </>
+    );
+
+    // This is kind of ugly but I can't find a better way to target just the animation div
+    const animationAElement = getByTestId('a').parentElement.parentElement.parentElement
+      .parentElement;
+    const animationBElement = getByTestId('b').parentElement.parentElement.parentElement
+      .parentElement;
+
+    fireEvent.animationEnd(animationAElement);
+    fireEvent.animationEnd(animationBElement);
+
+    fireEvent.keyUp(document, { key: 'Escape' });
+
+    expect(spyA).toHaveBeenCalledTimes(0);
+    expect(spyB).toHaveBeenCalledTimes(1);
+  });
 });