Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: User navigates to many pages when quickly press them #34861

Merged
merged 8 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/components/MenuItem.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import ExpensiMark from 'expensify-common/lib/ExpensiMark';
import type {ImageContentFit} from 'expo-image';
import type {ForwardedRef, ReactNode} from 'react';
import React, {forwardRef, useEffect, useMemo, useRef, useState} from 'react';
import React, {forwardRef, useContext, useEffect, useMemo, useRef, useState} from 'react';
import type {GestureResponderEvent, StyleProp, TextStyle, ViewStyle} from 'react-native';
import {View} from 'react-native';
import type {AnimatedStyle} from 'react-native-reanimated';
Expand Down Expand Up @@ -29,6 +29,7 @@ import Hoverable from './Hoverable';
import Icon from './Icon';
import * as Expensicons from './Icon/Expensicons';
import * as defaultWorkspaceAvatars from './Icon/WorkspaceDefaultAvatars';
import {MenuItemGroupContext} from './MenuItemGroup';
import MultipleAvatars from './MultipleAvatars';
import PressableWithSecondaryInteraction from './PressableWithSecondaryInteraction';
import RenderHTML from './RenderHTML';
Expand Down Expand Up @@ -295,6 +296,7 @@ function MenuItem(
const {isSmallScreenWidth} = useWindowDimensions();
const [html, setHtml] = useState('');
const titleRef = useRef('');
const {isExecuting, singleExecution, waitForNavigate} = useContext(MenuItemGroupContext) ?? {};

const isDeleted = style && Array.isArray(style) ? style.includes(styles.offlineFeedback.deleted) : false;
const descriptionVerticalMargin = shouldShowDescriptionOnTop ? styles.mb1 : styles.mt1;
Expand Down Expand Up @@ -370,7 +372,11 @@ function MenuItem(
}

if (onPress && event) {
onPress(event);
if (!singleExecution || !waitForNavigate) {
onPress(event);
return;
}
singleExecution(waitForNavigate(() => onPress(event)))();
}
};

Expand All @@ -394,7 +400,7 @@ function MenuItem(
shouldGreyOutWhenDisabled && disabled && styles.buttonOpacityDisabled,
] as StyleProp<ViewStyle>
}
disabled={disabled}
disabled={disabled || isExecuting}
ref={ref}
role={CONST.ROLE.MENUITEM}
accessibilityLabel={title ? title.toString() : ''}
Expand Down
35 changes: 35 additions & 0 deletions src/components/MenuItemGroup.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import React, {createContext, useMemo} from 'react';
import useSingleExecution from '@hooks/useSingleExecution';
import type {Action} from '@hooks/useSingleExecution';
import useWaitForNavigation from '@hooks/useWaitForNavigation';

type MenuItemGroupContextProps = {
isExecuting: boolean;
singleExecution: <T extends unknown[]>(action?: Action<T> | undefined) => (...params: T) => void;
waitForNavigate: ReturnType<typeof useWaitForNavigation>;
};

const MenuItemGroupContext = createContext<MenuItemGroupContextProps | undefined>(undefined);

type MenuItemGroupProps = {
/* Actual content wrapped by this component */
children: React.ReactNode;

/** Whether or not to use the single execution hook */
shouldUseSingleExecution?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does this ever get set to false?

Copy link
Contributor Author

@tienifr tienifr Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just like what we did in MenuItemList here. I can remove it without any problem.

};

function MenuItemGroup({children, shouldUseSingleExecution = true}: MenuItemGroupProps) {
const {isExecuting, singleExecution} = useSingleExecution();
const waitForNavigate = useWaitForNavigation();

const value = useMemo(
() => (shouldUseSingleExecution ? {isExecuting, singleExecution, waitForNavigate} : undefined),
[shouldUseSingleExecution, isExecuting, singleExecution, waitForNavigate],
);

return <MenuItemGroupContext.Provider value={value}>{children}</MenuItemGroupContext.Provider>;
}

export {MenuItemGroupContext};
export default MenuItemGroup;
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {ScrollView, View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import MenuItemGroup from '@components/MenuItemGroup';
import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription';
import {withNetwork} from '@components/OnyxProvider';
import ScreenWrapper from '@components/ScreenWrapper';
Expand Down Expand Up @@ -77,25 +78,27 @@ function PersonalDetailsInitialPage(props) {
<View style={[styles.ph5, styles.mb5]}>
<Text>{props.translate('privatePersonalDetails.privateDataMessage')}</Text>
</View>
<MenuItemWithTopDescription
title={legalName}
description={props.translate('privatePersonalDetails.legalName')}
shouldShowRightIcon
onPress={() => Navigation.navigate(ROUTES.SETTINGS_PERSONAL_DETAILS_LEGAL_NAME)}
/>
<MenuItemWithTopDescription
title={privateDetails.dob || ''}
description={props.translate('common.dob')}
shouldShowRightIcon
onPress={() => Navigation.navigate(ROUTES.SETTINGS_PERSONAL_DETAILS_DATE_OF_BIRTH)}
titleStyle={[styles.flex1]}
/>
<MenuItemWithTopDescription
title={PersonalDetailsUtils.getFormattedAddress(props.privatePersonalDetails)}
description={props.translate('privatePersonalDetails.address')}
shouldShowRightIcon
onPress={() => Navigation.navigate(ROUTES.SETTINGS_PERSONAL_DETAILS_ADDRESS)}
/>
<MenuItemGroup>
<MenuItemWithTopDescription
title={legalName}
description={props.translate('privatePersonalDetails.legalName')}
shouldShowRightIcon
onPress={() => Navigation.navigate(ROUTES.SETTINGS_PERSONAL_DETAILS_LEGAL_NAME)}
/>
<MenuItemWithTopDescription
title={privateDetails.dob || ''}
description={props.translate('common.dob')}
shouldShowRightIcon
onPress={() => Navigation.navigate(ROUTES.SETTINGS_PERSONAL_DETAILS_DATE_OF_BIRTH)}
titleStyle={[styles.flex1]}
/>
<MenuItemWithTopDescription
title={PersonalDetailsUtils.getFormattedAddress(props.privatePersonalDetails)}
description={props.translate('privatePersonalDetails.address')}
shouldShowRightIcon
onPress={() => Navigation.navigate(ROUTES.SETTINGS_PERSONAL_DETAILS_ADDRESS)}
/>
</MenuItemGroup>
</View>
</ScrollView>
)}
Expand Down
Loading