Merge pull request Expensify#46357 from dominictb/fix/44082-hold-edu-…

…modal fix: remove popstate listener in popover
brunovjk · Jul 29, 2024 · 6be6082 · 6be6082
2 parents aae253e + 36f8d8a
commit 6be6082
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 5 deletions.
diff --git a/src/components/Popover/index.tsx b/src/components/Popover/index.tsx
@@ -27,6 +27,7 @@ function Popover(props: PopoverProps) {
         anchorRef = () => {},
         animationIn = 'fadeIn',
         animationOut = 'fadeOut',
+        shouldCloseWhenBrowserNavigationChanged = true,
     } = props;
 
     const {isSmallScreenWidth} = useResponsiveLayout();
@@ -36,18 +37,20 @@ function Popover(props: PopoverProps) {
     // Not adding this inside the PopoverProvider
     // because this is an issue on smaller screens as well.
     React.useEffect(() => {
+        if (!shouldCloseWhenBrowserNavigationChanged) {
+            return;
+        }
         const listener = () => {
             if (!isVisible) {
                 return;
             }
-
             onClose();
         };
         window.addEventListener('popstate', listener);
         return () => {
             window.removeEventListener('popstate', listener);
         };
-    }, [onClose, isVisible]);
+    }, [onClose, isVisible, shouldCloseWhenBrowserNavigationChanged]);
 
     const onCloseWithPopoverContext = () => {
         if (popover && 'current' in anchorRef) {

diff --git a/src/components/Popover/types.ts b/src/components/Popover/types.ts
@@ -38,8 +38,8 @@ type PopoverProps = BaseModalProps &
         /** Whether we want to show the popover on the right side of the screen */
         fromSidebarMediumScreen?: boolean;
 
-        /** Whether handle navigation back when modal show. */
-        shouldHandleNavigationBack?: boolean;
+        /** Whether we should close when browser navigation change. This doesn't affect native platform */
+        shouldCloseWhenBrowserNavigationChanged?: boolean;
     };
 
 type PopoverWithWindowDimensionsProps = PopoverProps & WindowDimensionsProps;

diff --git a/src/components/ProcessMoneyRequestHoldMenu.tsx b/src/components/ProcessMoneyRequestHoldMenu.tsx
@@ -1,4 +1,5 @@
-import React, {useRef} from 'react';
+import {useNavigation} from '@react-navigation/native';
+import React, {useEffect, useRef} from 'react';
 import {View} from 'react-native';
 import useLocalize from '@hooks/useLocalize';
 import useThemeStyles from '@hooks/useThemeStyles';
@@ -31,6 +32,14 @@ function ProcessMoneyRequestHoldMenu({isVisible, onClose, onConfirm, anchorPosit
     const {translate} = useLocalize();
     const styles = useThemeStyles();
     const popoverRef = useRef(null);
+    const navigation = useNavigation();
+
+    useEffect(() => {
+        const unsub = navigation.addListener('beforeRemove', () => {
+            onClose();
+        });
+        return unsub;
+    }, [navigation, onClose]);
 
     return (
         <Popover
@@ -41,6 +50,7 @@ function ProcessMoneyRequestHoldMenu({isVisible, onClose, onConfirm, anchorPosit
             anchorAlignment={anchorAlignment}
             disableAnimation={false}
             withoutOverlay={false}
+            shouldCloseWhenBrowserNavigationChanged={false}
         >
             <View style={[styles.mh5, styles.mv5]}>
                 <View style={[styles.flexRow, styles.alignItemsCenter, styles.mb5]}>