From 0984dbf6c1938bee0376a285a7213ccc9d2e320b Mon Sep 17 00:00:00 2001 From: zfurtak Date: Tue, 13 Aug 2024 17:15:28 +0200 Subject: [PATCH 1/3] Introduce new fix --- src/components/AttachmentModal.tsx | 1 + src/components/AvatarWithImagePicker.tsx | 38 +++++++++---------- .../ButtonWithDropdownMenu/index.tsx | 4 +- src/components/PopoverMenu.tsx | 10 +++++ .../Navigation/AppNavigator/AuthScreens.tsx | 6 ++- src/libs/actions/Modal.ts | 6 ++- .../AttachmentPickerWithMenuItems.tsx | 15 ++++---- .../request/step/IOURequestStepWaypoint.tsx | 8 ++-- src/pages/workspace/WorkspacesListPage.tsx | 13 +++---- .../accounting/PolicyAccountingPage.tsx | 4 +- 10 files changed, 59 insertions(+), 46 deletions(-) diff --git a/src/components/AttachmentModal.tsx b/src/components/AttachmentModal.tsx index 164da5922b98f..d9aaa17d0b6e7 100644 --- a/src/components/AttachmentModal.tsx +++ b/src/components/AttachmentModal.tsx @@ -429,6 +429,7 @@ function AttachmentModal({ onSelected: () => { setIsDeleteReceiptConfirmModalVisible(true); }, + shouldCallAfterModalHide: true, }); } return menuItems; diff --git a/src/components/AvatarWithImagePicker.tsx b/src/components/AvatarWithImagePicker.tsx index a1b8524dd2932..8a018101b63e1 100644 --- 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 index 943d6dbb5c166..74b38f515a066 100644 --- 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({ 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 index e9df68d43d06f..a38fedabe8cab 100644 --- 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,9 @@ 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 modal hide or not */ + shouldCallAfterModalHide?: boolean; }; type PopoverModalProps = Pick; @@ -128,6 +133,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 index 071b6d8270d7d..067f47494c24c 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.tsx +++ b/src/libs/Navigation/AppNavigator/AuthScreens.tsx @@ -305,7 +305,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 index 9cba7a3595377..01ac832336abf 100644 --- 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 index 5d7b5b1390c2d..6e3c3a48de74f 100644 --- 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 index 71c42acefdaaa..802967345fb65 100644 --- a/src/pages/iou/request/step/IOURequestStepWaypoint.tsx +++ b/src/pages/iou/request/step/IOURequestStepWaypoint.tsx @@ -23,7 +23,6 @@ import useWindowDimensions from '@hooks/useWindowDimensions'; import * as ErrorUtils from '@libs/ErrorUtils'; 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'; @@ -179,11 +178,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 index 94fb454c4a2d4..d4ff46e268f81 100755 --- 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 index ffde9cb25de04..7cd61a09e40e8 100644 --- a/src/pages/workspace/accounting/PolicyAccountingPage.tsx +++ b/src/pages/workspace/accounting/PolicyAccountingPage.tsx @@ -47,7 +47,6 @@ import type {WithPolicyConnectionsProps} from '@pages/workspace/withPolicyConnec import withPolicyConnections from '@pages/workspace/withPolicyConnections'; import type {AnchorPosition} from '@styles/index'; import {getTrackingCategories} from '@userActions/connections/ConnectToXero'; -import * as Modal from '@userActions/Modal'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; @@ -249,7 +248,8 @@ function PolicyAccountingPage({policy}: PolicyAccountingPageProps) { { icon: Expensicons.Trashcan, text: translate('workspace.accounting.disconnect'), - onSelected: () => Modal.close(() => setIsDisconnectModalOpen(true)), + onSelected: () => setIsDisconnectModalOpen(true), + shouldCallAfterModalHide: true, }, ], [translate, policyID, isOffline, connectedIntegration], From 25d93c7ac4c653ddcb81ed5468df87421c0c15ef Mon Sep 17 00:00:00 2001 From: zfurtak Date: Wed, 14 Aug 2024 16:21:21 +0200 Subject: [PATCH 2/3] Adjusted to review --- src/components/PopoverMenu.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/PopoverMenu.tsx b/src/components/PopoverMenu.tsx index a38fedabe8cab..788f1f84246a0 100644 --- a/src/components/PopoverMenu.tsx +++ b/src/components/PopoverMenu.tsx @@ -8,7 +8,6 @@ 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'; @@ -38,7 +37,9 @@ 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 modal hide or not */ + /** 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; }; @@ -133,7 +134,7 @@ function PopoverMenu({ setEnteredSubMenuIndexes([...enteredSubMenuIndexes, index]); const selectedSubMenuItemIndex = selectedItem?.subMenuItems.findIndex((option) => option.isSelected); setFocusedIndex(selectedSubMenuItemIndex); - } else if (selectedItem.shouldCallAfterModalHide && !Browser.isSafari()) { + } else if (selectedItem.shouldCallAfterModalHide) { Modal.close(() => { onItemSelected(selectedItem, index); selectedItem.onSelected?.(); From afeb37e206cbca5f261e461548f41fbe241d2aae Mon Sep 17 00:00:00 2001 From: war-in Date: Wed, 21 Aug 2024 08:28:17 +0200 Subject: [PATCH 3/3] fix lint --- src/pages/workspace/accounting/PolicyAccountingPage.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/pages/workspace/accounting/PolicyAccountingPage.tsx b/src/pages/workspace/accounting/PolicyAccountingPage.tsx index eab0a1cc32cb3..107f64dd2deed 100644 --- 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 {getTrackingCategories} from '@userActions/connections/ConnectToXero'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; @@ -99,8 +98,8 @@ function PolicyAccountingPage({policy}: PolicyAccountingPageProps) { icon: Expensicons.Key, text: translate('workspace.accounting.enterCredentials'), onSelected: () => startIntegrationFlow({name: connectedIntegration}), - shouldCallAfterModalHide: true, - disabled: isOffline, + shouldCallAfterModalHide: true, + disabled: isOffline, iconRight: Expensicons.NewWindow, shouldShowRightIcon: true, },