Merge pull request #47991 from software-mansion-labs/@zfurtak/fix-modals

Fix bugs connected to changed modal logic
Expensify · Sep 2, 2024 · 46bd3f7 · 46bd3f7
2 parents 9aca655 + 68be6ff
commit 46bd3f7
Show file tree

Hide file tree

Showing 10 changed files with 63 additions and 47 deletions.
diff --git a/src/components/AttachmentModal.tsx b/src/components/AttachmentModal.tsx
@@ -448,6 +448,7 @@ function AttachmentModal({
                 onSelected: () => {
                     setIsDeleteReceiptConfirmModalVisible(true);
                 },
+                shouldCallAfterModalHide: true,
             });
         }
         return menuItems;

diff --git a/src/components/AvatarWithImagePicker.tsx b/src/components/AvatarWithImagePicker.tsx
@@ -11,7 +11,6 @@ import * as FileUtils from '@libs/fileDownload/FileUtils';
 import getImageResolution from '@libs/fileDownload/getImageResolution';
 import type {AvatarSource} from '@libs/UserUtils';
 import variables from '@styles/variables';
-import * as Modal from '@userActions/Modal';
 import CONST from '@src/CONST';
 import type {TranslationPaths} from '@src/languages/types';
 import type * as OnyxCommon from '@src/types/onyx/OnyxCommon';
@@ -44,6 +43,7 @@ type MenuItem = {
     icon: IconAsset;
     text: string;
     onSelected: () => void;
+    shouldCallAfterModalHide?: boolean;
 };
 
 type AvatarWithImagePickerProps = {
@@ -260,19 +260,19 @@ function AvatarWithImagePicker({
      * Create menu items list for avatar menu
      */
     const createMenuItems = (openPicker: OpenPicker): MenuItem[] => {
-        const menuItems = [
+        const menuItems: MenuItem[] = [
             {
                 icon: Expensicons.Upload,
                 text: translate('avatarWithImagePicker.uploadPhoto'),
-                onSelected: () =>
-                    Modal.close(() => {
-                        if (Browser.isSafari()) {
-                            return;
-                        }
-                        openPicker({
-                            onPicked: showAvatarCropModal,
-                        });
-                    }),
+                onSelected: () => {
+                    if (Browser.isSafari()) {
+                        return;
+                    }
+                    openPicker({
+                        onPicked: showAvatarCropModal,
+                    });
+                },
+                shouldCallAfterModalHide: true,
             },
         ];
 
@@ -344,14 +344,14 @@ function AvatarWithImagePicker({
                                     menuItems.push({
                                         icon: Expensicons.Eye,
                                         text: translate('avatarWithImagePicker.viewPhoto'),
-                                        onSelected: () =>
-                                            Modal.close(() => {
-                                                if (typeof onViewPhotoPress !== 'function') {
-                                                    show();
-                                                    return;
-                                                }
-                                                onViewPhotoPress();
-                                            }),
+                                        onSelected: () => {
+                                            if (typeof onViewPhotoPress !== 'function') {
+                                                show();
+                                                return;
+                                            }
+                                            onViewPhotoPress();
+                                        },
+                                        shouldCallAfterModalHide: true,
                                     });
                                 }
 

diff --git a/src/components/ButtonWithDropdownMenu/index.tsx b/src/components/ButtonWithDropdownMenu/index.tsx
@@ -11,7 +11,6 @@ import useTheme from '@hooks/useTheme';
 import useThemeStyles from '@hooks/useThemeStyles';
 import useWindowDimensions from '@hooks/useWindowDimensions';
 import mergeRefs from '@libs/mergeRefs';
-import * as Modal from '@userActions/Modal';
 import CONST from '@src/CONST';
 import type {AnchorPosition} from '@src/styles';
 import type {ButtonWithDropdownMenuProps} from './types';
@@ -178,11 +177,12 @@ function ButtonWithDropdownMenu<IValueType>({
                     menuItems={options.map((item, index) => ({
                         ...item,
                         onSelected: item.onSelected
-                            ? () => Modal.close(() => item.onSelected?.())
+                            ? () => item.onSelected?.()
                             : () => {
                                   onOptionSelected?.(item);
                                   setSelectedItemIndex(index);
                               },
+                        shouldCallAfterModalHide: true,
                     }))}
                 />
             )}

diff --git a/src/components/PopoverMenu.tsx b/src/components/PopoverMenu.tsx
@@ -8,6 +8,8 @@ import useKeyboardShortcut from '@hooks/useKeyboardShortcut';
 import useResponsiveLayout from '@hooks/useResponsiveLayout';
 import useTheme from '@hooks/useTheme';
 import useThemeStyles from '@hooks/useThemeStyles';
+import * as Browser from '@libs/Browser';
+import * as Modal from '@userActions/Modal';
 import CONST from '@src/CONST';
 import type {AnchorPosition} from '@src/styles';
 import type AnchorAlignment from '@src/types/utils/AnchorAlignment';
@@ -35,6 +37,11 @@ type PopoverMenuItem = MenuItemProps & {
 
     /** Determines whether the menu item is disabled or not */
     disabled?: boolean;
+
+    /** Determines whether the menu item's onSelected() function is called after the modal is hidden
+     *  It is meant to be used in situations where, after clicking on the modal, another one is opened.
+     */
+    shouldCallAfterModalHide?: boolean;
 };
 
 type PopoverModalProps = Pick<ModalProps, 'animationIn' | 'animationOut' | 'animationInTiming'>;
@@ -128,6 +135,11 @@ function PopoverMenu({
             setEnteredSubMenuIndexes([...enteredSubMenuIndexes, index]);
             const selectedSubMenuItemIndex = selectedItem?.subMenuItems.findIndex((option) => option.isSelected);
             setFocusedIndex(selectedSubMenuItemIndex);
+        } else if (selectedItem.shouldCallAfterModalHide && !Browser.isSafari()) {
+            Modal.close(() => {
+                onItemSelected(selectedItem, index);
+                selectedItem.onSelected?.();
+            });
         } else {
             onItemSelected(selectedItem, index);
             selectedItem.onSelected?.();

diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.tsx b/src/libs/Navigation/AppNavigator/AuthScreens.tsx
@@ -339,7 +339,11 @@ function AuthScreens({session, lastOpenedPublicRoomID, initialLastUpdateIDApplie
         const unsubscribeSearchShortcut = KeyboardShortcut.subscribe(
             searchShortcutConfig.shortcutKey,
             () => {
-                Modal.close(Session.checkIfActionIsAllowed(() => Navigation.navigate(ROUTES.CHAT_FINDER)));
+                Modal.close(
+                    Session.checkIfActionIsAllowed(() => Navigation.navigate(ROUTES.CHAT_FINDER)),
+                    true,
+                    true,
+                );
             },
             shortcutsOverviewShortcutConfig.descriptionKey,
             shortcutsOverviewShortcutConfig.modifiers,

diff --git a/src/libs/actions/Modal.ts b/src/libs/actions/Modal.ts
@@ -5,6 +5,7 @@ const closeModals: Array<(isNavigating?: boolean) => void> = [];
 
 let onModalClose: null | (() => void);
 let isNavigate: undefined | boolean;
+let shouldCloseAll: boolean | undefined;
 
 /**
  * Allows other parts of the app to call modal close function
@@ -39,12 +40,13 @@ function closeTop() {
 /**
  * Close modal in other parts of the app
  */
-function close(onModalCloseCallback: () => void, isNavigating = true) {
+function close(onModalCloseCallback: () => void, isNavigating = true, shouldCloseAllModals = false) {
     if (closeModals.length === 0) {
         onModalCloseCallback();
         return;
     }
     onModalClose = onModalCloseCallback;
+    shouldCloseAll = shouldCloseAllModals;
     isNavigate = isNavigating;
     closeTop();
 }
@@ -53,7 +55,7 @@ function onModalDidClose() {
     if (!onModalClose) {
         return;
     }
-    if (closeModals.length) {
+    if (closeModals.length && shouldCloseAll) {
         closeTop();
         return;
     }

diff --git a/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.tsx b/src/pages/home/report/ReportActionCompose/AttachmentPickerWithMenuItems.tsx
@@ -23,7 +23,6 @@ import Navigation from '@libs/Navigation/Navigation';
 import * as ReportUtils from '@libs/ReportUtils';
 import * as SubscriptionUtils from '@libs/SubscriptionUtils';
 import * as IOU from '@userActions/IOU';
-import * as Modal from '@userActions/Modal';
 import * as Report from '@userActions/Report';
 import * as Task from '@userActions/Task';
 import type {IOUType} from '@src/CONST';
@@ -225,13 +224,13 @@ function AttachmentPickerWithMenuItems({
                     {
                         icon: Expensicons.Paperclip,
                         text: translate('reportActionCompose.addAttachment'),
-                        onSelected: () =>
-                            Modal.close(() => {
-                                if (Browser.isSafari()) {
-                                    return;
-                                }
-                                triggerAttachmentPicker();
-                            }),
+                        onSelected: () => {
+                            if (Browser.isSafari()) {
+                                return;
+                            }
+                            triggerAttachmentPicker();
+                        },
+                        shouldCallAfterModalHide: true,
                     },
                 ];
                 return (

diff --git a/src/pages/iou/request/step/IOURequestStepWaypoint.tsx b/src/pages/iou/request/step/IOURequestStepWaypoint.tsx
@@ -24,7 +24,6 @@ import * as ErrorUtils from '@libs/ErrorUtils';
 import * as IOUUtils from '@libs/IOUUtils';
 import Navigation from '@libs/Navigation/Navigation';
 import * as ValidationUtils from '@libs/ValidationUtils';
-import * as Modal from '@userActions/Modal';
 import * as Transaction from '@userActions/Transaction';
 import CONST from '@src/CONST';
 import type {TranslationPaths} from '@src/languages/types';
@@ -180,11 +179,10 @@ function IOURequestStepWaypoint({
                             icon: Expensicons.Trashcan,
                             text: translate('distance.deleteWaypoint'),
                             onSelected: () => {
-                                Modal.close(() => {
-                                    setRestoreFocusType(undefined);
-                                    setIsDeleteStopModalOpen(true);
-                                });
+                                setRestoreFocusType(undefined);
+                                setIsDeleteStopModalOpen(true);
                             },
+                            shouldCallAfterModalHide: true,
                         },
                     ]}
                 />

diff --git a/src/pages/workspace/WorkspacesListPage.tsx b/src/pages/workspace/WorkspacesListPage.tsx
@@ -32,7 +32,6 @@ import * as PolicyUtils from '@libs/PolicyUtils';
 import * as ReportUtils from '@libs/ReportUtils';
 import type {AvatarSource} from '@libs/UserUtils';
 import * as App from '@userActions/App';
-import * as Modal from '@userActions/Modal';
 import * as Policy from '@userActions/Policy/Policy';
 import * as Session from '@userActions/Session';
 import CONST from '@src/CONST';
@@ -162,12 +161,12 @@ function WorkspacesListPage({policies, reimbursementAccount, reports, session}:
                 threeDotsMenuItems.push({
                     icon: Expensicons.Trashcan,
                     text: translate('workspace.common.delete'),
-                    onSelected: () =>
-                        Modal.close(() => {
-                            setPolicyIDToDelete(item.policyID ?? '-1');
-                            setPolicyNameToDelete(item.title);
-                            setIsDeleteModalOpen(true);
-                        }),
+                    onSelected: () => {
+                        setPolicyIDToDelete(item.policyID ?? '-1');
+                        setPolicyNameToDelete(item.title);
+                        setIsDeleteModalOpen(true);
+                    },
+                    shouldCallAfterModalHide: true,
                 });
             }
 

diff --git a/src/pages/workspace/accounting/PolicyAccountingPage.tsx b/src/pages/workspace/accounting/PolicyAccountingPage.tsx
@@ -38,7 +38,6 @@ import Navigation from '@navigation/Navigation';
 import AccessOrNotFoundWrapper from '@pages/workspace/AccessOrNotFoundWrapper';
 import withPolicyConnections from '@pages/workspace/withPolicyConnections';
 import type {AnchorPosition} from '@styles/index';
-import * as Modal from '@userActions/Modal';
 import CONST from '@src/CONST';
 import ONYXKEYS from '@src/ONYXKEYS';
 import ROUTES from '@src/ROUTES';
@@ -98,7 +97,8 @@ function PolicyAccountingPage({policy}: PolicyAccountingPageProps) {
                       {
                           icon: Expensicons.Key,
                           text: translate('workspace.accounting.enterCredentials'),
-                          onSelected: () => Modal.close(() => startIntegrationFlow({name: connectedIntegration})),
+                          onSelected: () => startIntegrationFlow({name: connectedIntegration}),
+                          shouldCallAfterModalHide: true,
                           disabled: isOffline,
                           iconRight: Expensicons.NewWindow,
                           shouldShowRightIcon: true,
@@ -115,7 +115,8 @@ function PolicyAccountingPage({policy}: PolicyAccountingPageProps) {
             {
                 icon: Expensicons.Trashcan,
                 text: translate('workspace.accounting.disconnect'),
-                onSelected: () => Modal.close(() => setIsDisconnectModalOpen(true)),
+                onSelected: () => setIsDisconnectModalOpen(true),
+                shouldCallAfterModalHide: true,
             },
         ],
         [shouldShowEnterCredentials, translate, isOffline, policyID, connectedIntegration, startIntegrationFlow],