Merge pull request #28338 from ishpaul777/fix/quickly-pressing-differ…

…ent-menuitems fix: ignore other actions if navigation in progress in menu Items
Expensify · Oct 25, 2023 · 0d6fa8b · 0d6fa8b
2 parents a8482e2 + 38e6e69
commit 0d6fa8b
Show file tree

Hide file tree

Showing 8 changed files with 156 additions and 106 deletions.
diff --git a/src/components/HeaderWithBackButton/headerWithBackButtonPropTypes.js b/src/components/HeaderWithBackButton/headerWithBackButtonPropTypes.js
@@ -31,12 +31,18 @@ const propTypes = {
     /** Whether we should show a get assistance (question mark) button */
     shouldShowGetAssistanceButton: PropTypes.bool,
 
+    /** Whether we should disable the get assistance button */
+    shouldDisableGetAssistanceButton: PropTypes.bool,
+
     /** Whether we should show a pin button */
     shouldShowPinButton: PropTypes.bool,
 
     /** Whether we should show a more options (threedots) button */
     shouldShowThreeDotsButton: PropTypes.bool,
 
+    /** Whether we should disable threedots button */
+    shouldDisableThreeDotsButton: PropTypes.bool,
+
     /** List of menu items for more(three dots) menu */
     threeDotsMenuItems: ThreeDotsMenuItemPropTypes,
 
@@ -84,6 +90,9 @@ const propTypes = {
 
     /** Children to wrap in Header */
     children: PropTypes.node,
+
+    /** Single execution function to prevent concurrent navigation actions */
+    singleExecution: PropTypes.func,
 };
 
 export default propTypes;
diff --git a/src/components/HeaderWithBackButton/index.js b/src/components/HeaderWithBackButton/index.js
@@ -18,6 +18,7 @@ import headerWithBackButtonPropTypes from './headerWithBackButtonPropTypes';
 import useThrottledButtonState from '../../hooks/useThrottledButtonState';
 import useLocalize from '../../hooks/useLocalize';
 import useKeyboardState from '../../hooks/useKeyboardState';
+import useWaitForNavigation from '../../hooks/useWaitForNavigation';
 
 function HeaderWithBackButton({
     iconFill = undefined,
@@ -35,8 +36,10 @@ function HeaderWithBackButton({
     shouldShowCloseButton = false,
     shouldShowDownloadButton = false,
     shouldShowGetAssistanceButton = false,
+    shouldDisableGetAssistanceButton = false,
     shouldShowPinButton = false,
     shouldShowThreeDotsButton = false,
+    shouldDisableThreeDotsButton = false,
     stepCounter = null,
     subtitle = '',
     title = '',
@@ -49,10 +52,12 @@ function HeaderWithBackButton({
     shouldEnableDetailPageNavigation = false,
     children = null,
     shouldOverlay = false,
+    singleExecution = (func) => func,
 }) {
     const [isDownloadButtonActive, temporarilyDisableDownloadButton] = useThrottledButtonState();
     const {translate} = useLocalize();
     const {isKeyboardShown} = useKeyboardState();
+    const waitForNavigate = useWaitForNavigation();
     return (
         <View
             // Hover on some part of close icons will not work on Electron if dragArea is true
@@ -126,7 +131,8 @@ function HeaderWithBackButton({
                     {shouldShowGetAssistanceButton && (
                         <Tooltip text={translate('getAssistancePage.questionMarkButtonTooltip')}>
                             <PressableWithoutFeedback
-                                onPress={() => Navigation.navigate(ROUTES.GET_ASSISTANCE.getRoute(guidesCallTaskID))}
+                                disabled={shouldDisableGetAssistanceButton}
+                                onPress={singleExecution(waitForNavigate(() => Navigation.navigate(ROUTES.GET_ASSISTANCE.getRoute(guidesCallTaskID))))}
                                 style={[styles.touchableButtonImage]}
                                 accessibilityRole="button"
                                 accessibilityLabel={translate('getAssistancePage.questionMarkButtonTooltip')}
@@ -141,6 +147,7 @@ function HeaderWithBackButton({
                     {shouldShowPinButton && <PinButton report={report} />}
                     {shouldShowThreeDotsButton && (
                         <ThreeDotsMenu
+                            disabled={shouldDisableThreeDotsButton}
                             menuItems={threeDotsMenuItems}
                             onIconPress={onThreeDotsButtonPress}
                             anchorPosition={threeDotsAnchorPosition}

diff --git a/src/components/MenuItemList.js b/src/components/MenuItemList.js
@@ -1,6 +1,7 @@
 import React from 'react';
 import _ from 'underscore';
 import PropTypes from 'prop-types';
+import useSingleExecution from '../hooks/useSingleExecution';
 import MenuItem from './MenuItem';
 import menuItemPropTypes from './menuItemPropTypes';
 import * as ReportActionContextMenu from '../pages/home/report/ContextMenu/ReportActionContextMenu';
@@ -9,13 +10,18 @@ import {CONTEXT_MENU_TYPES} from '../pages/home/report/ContextMenu/ContextMenuAc
 const propTypes = {
     /** An array of props that are pass to individual MenuItem components */
     menuItems: PropTypes.arrayOf(PropTypes.shape(menuItemPropTypes)),
+
+    /** Whether or not to use the single execution hook */
+    shouldUseSingleExecution: PropTypes.bool,
 };
 const defaultProps = {
     menuItems: [],
+    shouldUseSingleExecution: false,
 };
 
 function MenuItemList(props) {
     let popoverAnchor;
+    const {isExecuting, singleExecution} = useSingleExecution();
 
     /**
      * Handle the secondary interaction for a menu item.
@@ -41,6 +47,8 @@ function MenuItemList(props) {
                     shouldBlockSelection={Boolean(menuItemProps.link)}
                     // eslint-disable-next-line react/jsx-props-no-spreading
                     {...menuItemProps}
+                    disabled={menuItemProps.disabled || isExecuting}
+                    onPress={props.shouldUseSingleExecution ? singleExecution(menuItemProps.onPress) : menuItemProps.onPress}
                 />
             ))}
         </>

diff --git a/src/components/ThreeDotsMenu/index.js b/src/components/ThreeDotsMenu/index.js
@@ -50,12 +50,16 @@ const propTypes = {
     /** Whether the popover menu should overlay the current view */
     shouldOverlay: PropTypes.bool,
 
+    /** Whether the menu is disabled */
+    disabled: PropTypes.bool,
+
     /** Should we announce the Modal visibility changes? */
     shouldSetModalVisibility: PropTypes.bool,
 };
 
 const defaultProps = {
     iconTooltip: 'common.more',
+    disabled: false,
     iconFill: undefined,
     iconStyles: [],
     icon: Expensicons.ThreeDots,
@@ -68,7 +72,7 @@ const defaultProps = {
     shouldSetModalVisibility: true,
 };
 
-function ThreeDotsMenu({iconTooltip, icon, iconFill, iconStyles, onIconPress, menuItems, anchorPosition, anchorAlignment, shouldOverlay, shouldSetModalVisibility}) {
+function ThreeDotsMenu({iconTooltip, icon, iconFill, iconStyles, onIconPress, menuItems, anchorPosition, anchorAlignment, shouldOverlay, shouldSetModalVisibility, disabled}) {
     const [isPopupMenuVisible, setPopupMenuVisible] = useState(false);
     const buttonRef = useRef(null);
     const {translate} = useLocalize();
@@ -96,6 +100,7 @@ function ThreeDotsMenu({iconTooltip, icon, iconFill, iconStyles, onIconPress, me
                                 onIconPress();
                             }
                         }}
+                        disabled={disabled}
                         onMouseDown={(e) => {
                             /* Keep the focus state on mWeb like we did on the native apps. */
                             if (!Browser.isMobile()) {

diff --git a/src/pages/settings/AboutPage/AboutPage.js b/src/pages/settings/AboutPage/AboutPage.js
@@ -1,5 +1,5 @@
 import _ from 'underscore';
-import React from 'react';
+import React, {useRef, useMemo} from 'react';
 import {ScrollView, View} from 'react-native';
 import DeviceInfo from 'react-native-device-info';
 import HeaderWithBackButton from '../../../components/HeaderWithBackButton';
@@ -13,7 +13,6 @@ import * as Expensicons from '../../../components/Icon/Expensicons';
 import ScreenWrapper from '../../../components/ScreenWrapper';
 import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize';
 import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions';
-import MenuItem from '../../../components/MenuItem';
 import Logo from '../../../../assets/images/new-expensify.svg';
 import pkg from '../../../../package.json';
 import * as Report from '../../../libs/actions/Report';
@@ -22,6 +21,8 @@ import compose from '../../../libs/compose';
 import * as ReportActionContextMenu from '../../home/report/ContextMenu/ReportActionContextMenu';
 import {CONTEXT_MENU_TYPES} from '../../home/report/ContextMenu/ContextMenuActions';
 import * as Environment from '../../../libs/Environment/Environment';
+import MenuItemList from '../../../components/MenuItemList';
+import useWaitForNavigation from '../../../hooks/useWaitForNavigation';
 
 const propTypes = {
     ...withLocalizePropTypes,
@@ -40,46 +41,57 @@ function getFlavor() {
 }
 
 function AboutPage(props) {
-    let popoverAnchor;
-    const menuItems = [
-        {
-            translationKey: 'initialSettingsPage.aboutPage.appDownloadLinks',
-            icon: Expensicons.Link,
-            action: () => {
-                Navigation.navigate(ROUTES.SETTINGS_APP_DOWNLOAD_LINKS);
+    const {translate} = props;
+    const popoverAnchor = useRef(null);
+    const waitForNavigate = useWaitForNavigation();
+
+    const menuItems = useMemo(() => {
+        const baseMenuItems = [
+            {
+                translationKey: 'initialSettingsPage.aboutPage.appDownloadLinks',
+                icon: Expensicons.Link,
+                action: waitForNavigate(() => Navigation.navigate(ROUTES.SETTINGS_APP_DOWNLOAD_LINKS)),
+            },
+            {
+                translationKey: 'initialSettingsPage.aboutPage.viewKeyboardShortcuts',
+                icon: Expensicons.Keyboard,
+                action: waitForNavigate(() => Navigation.navigate(ROUTES.KEYBOARD_SHORTCUTS)),
             },
-        },
-        {
-            translationKey: 'initialSettingsPage.aboutPage.viewKeyboardShortcuts',
-            icon: Expensicons.Keyboard,
-            action: () => {
-                Navigation.navigate(ROUTES.KEYBOARD_SHORTCUTS);
+            {
+                translationKey: 'initialSettingsPage.aboutPage.viewTheCode',
+                icon: Expensicons.Eye,
+                iconRight: Expensicons.NewWindow,
+                action: () => {
+                    Link.openExternalLink(CONST.GITHUB_URL);
+                },
             },
-        },
-        {
-            translationKey: 'initialSettingsPage.aboutPage.viewTheCode',
-            icon: Expensicons.Eye,
-            iconRight: Expensicons.NewWindow,
-            action: () => {
-                Link.openExternalLink(CONST.GITHUB_URL);
+            {
+                translationKey: 'initialSettingsPage.aboutPage.viewOpenJobs',
+                icon: Expensicons.MoneyBag,
+                iconRight: Expensicons.NewWindow,
+                action: () => {
+                    Link.openExternalLink(CONST.UPWORK_URL);
+                },
+                link: CONST.UPWORK_URL,
             },
-            link: CONST.GITHUB_URL,
-        },
-        {
-            translationKey: 'initialSettingsPage.aboutPage.viewOpenJobs',
-            icon: Expensicons.MoneyBag,
-            iconRight: Expensicons.NewWindow,
-            action: () => {
-                Link.openExternalLink(CONST.UPWORK_URL);
+            {
+                translationKey: 'initialSettingsPage.aboutPage.reportABug',
+                icon: Expensicons.Bug,
+                action: waitForNavigate(Report.navigateToConciergeChat),
             },
-            link: CONST.UPWORK_URL,
-        },
-        {
-            translationKey: 'initialSettingsPage.aboutPage.reportABug',
-            icon: Expensicons.Bug,
-            action: Report.navigateToConciergeChat,
-        },
-    ];
+        ];
+        return _.map(baseMenuItems, (item) => ({
+            key: item.translationKey,
+            title: translate(item.translationKey),
+            icon: item.icon,
+            iconRight: item.iconRight,
+            onPress: item.action,
+            shouldShowRightIcon: true,
+            onSecondaryInteraction: !_.isEmpty(item.link) ? (e) => ReportActionContextMenu.showContextMenu(CONTEXT_MENU_TYPES.LINK, e, item.link, popoverAnchor) : undefined,
+            ref: popoverAnchor,
+            shouldBlockSelection: Boolean(item.link),
+        }));
+    }, [translate, waitForNavigate]);
 
     return (
         <ScreenWrapper
@@ -109,21 +121,10 @@ function AboutPage(props) {
                                     <Text style={[styles.baseFontStyle, styles.mv5]}>{props.translate('initialSettingsPage.aboutPage.description')}</Text>
                                 </View>
                             </View>
-                            {_.map(menuItems, (item) => (
-                                <MenuItem
-                                    key={item.translationKey}
-                                    title={props.translate(item.translationKey)}
-                                    icon={item.icon}
-                                    iconRight={item.iconRight}
-                                    onPress={() => item.action()}
-                                    shouldBlockSelection={Boolean(item.link)}
-                                    onSecondaryInteraction={
-                                        !_.isEmpty(item.link) ? (e) => ReportActionContextMenu.showContextMenu(CONTEXT_MENU_TYPES.LINK, e, item.link, popoverAnchor) : undefined
-                                    }
-                                    ref={(el) => (popoverAnchor = el)}
-                                    shouldShowRightIcon
-                                />
-                            ))}
+                            <MenuItemList
+                                menuItems={menuItems}
+                                shouldUseSingleExecution
+                            />
                         </View>
                         <View style={[styles.sidebarFooter]}>
                             <Text

diff --git a/src/pages/settings/InitialSettingsPage.js b/src/pages/settings/InitialSettingsPage.js
@@ -341,7 +341,8 @@ function InitialSettingsPage(props) {
                     <Tooltip text={translate('common.profile')}>
                         <PressableWithoutFeedback
                             style={styles.mb3}
-                            onPress={openProfileSettings}
+                            disabled={isExecuting}
+                            onPress={singleExecution(openProfileSettings)}
                             accessibilityLabel={translate('common.profile')}
                             accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON}
                         >
@@ -357,7 +358,8 @@ function InitialSettingsPage(props) {
                     </Tooltip>
                     <PressableWithoutFeedback
                         style={[styles.mt1, styles.mw100]}
-                        onPress={openProfileSettings}
+                        disabled={isExecuting}
+                        onPress={singleExecution(openProfileSettings)}
                         accessibilityLabel={translate('common.profile')}
                         accessibilityRole={CONST.ACCESSIBILITY_ROLE.LINK}
                     >