Skip to content
Draft
Show file tree
Hide file tree
Changes from 6 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
92 changes: 65 additions & 27 deletions app/components/UI/Notification/SwitchLoadingModal/LoaderModal.tsx
Original file line number Diff line number Diff line change
@@ -1,41 +1,79 @@
import React from 'react';
import { StyleSheet } from 'react-native';
import Modal from 'react-native-modal';
import { useTheme } from '../../../../util/theme';
import React, { useCallback, useEffect, useRef, useState } from 'react';
import {
BottomSheet,
type BottomSheetRef,
} from '@metamask/design-system-react-native';

export interface LoaderModalProps {
isVisible: boolean;
onCancel: () => void;
children: React.ReactNode;
}

const styles = StyleSheet.create({
bottomModal: {
justifyContent: 'flex-end',
marginHorizontal: 0,
},
});

const LoaderModal = (props: LoaderModalProps) => {
const { colors } = useTheme();
const { isVisible, onCancel, children } = props;
// Keep mounted while animating closed.
const [isMounted, setIsMounted] = useState(isVisible);
const sheetRef = useRef<BottomSheetRef>(null);
const closingDueToVisibilityRef = useRef(false);
// Track latest visibility to disambiguate stale close callbacks.
const isVisibleRef = useRef(isVisible);
useEffect(() => {
isVisibleRef.current = isVisible;
}, [isVisible]);

useEffect(() => {
if (isVisible) {
// Reset programmatic-close marker on explicit reopen.
closingDueToVisibilityRef.current = false;
setIsMounted(true);
// Ensure the sheet is opened in case a previous close finished.
sheetRef.current?.onOpenBottomSheet();
return;
}

if (isMounted) {
closingDueToVisibilityRef.current = true;
sheetRef.current?.onCloseBottomSheet(() => {
// If visibility flipped back to true while the close was animating,
// ignore this stale completion to avoid cancel/unmount flicker.
if (isVisibleRef.current) {
return;
}
setIsMounted(false);
closingDueToVisibilityRef.current = false;
});
}
}, [isMounted, isVisible]);

const handleSheetClosed = useCallback(() => {
// If the parent wants it visible again, ignore this stale close event.
if (isVisibleRef.current) {
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Dismiss blocked while visible

Medium Severity

The new isVisibleRef early return in handleSheetClosed skips onCancel whenever the parent still wants the sheet visible. That blocks intentional swipe/backdrop dismiss on an isInteractable sheet, so close events never sync parent state and the sheet can stay closed while isVisible remains true.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6020eff. Configure here.

setIsMounted(false);
if (!closingDueToVisibilityRef.current) {
onCancel();
} else {
// Programmatic close finished as intended; reset flag.
closingDueToVisibilityRef.current = false;
}
}, [onCancel]);
Comment thread
cursor[bot] marked this conversation as resolved.

if (!isMounted) {
return null;
}

return (
<Modal
isVisible={props.isVisible}
animationIn="slideInUp"
animationOut="slideOutDown"
style={styles.bottomModal}
backdropColor={colors.overlay.default}
backdropOpacity={1}
animationInTiming={600}
animationOutTiming={600}
onBackdropPress={props.onCancel}
onSwipeComplete={props.onCancel}
swipeDirection={'down'}
propagateSwipe
<BottomSheet
ref={sheetRef}
isInteractable
onClose={handleSheetClosed}
keyboardAvoidingViewEnabled
>
{props.children}
</Modal>
{/* Children can include their own card/container styling */}
{children}
</BottomSheet>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,6 @@ export const createStyles = (colors: Colors) =>
firstSetting: {
marginTop: 0,
},
modalView: {
alignItems: 'center',
flex: 1,
flexDirection: 'column',
justifyContent: 'center',
padding: 20,
},
modalTitle: {
textAlign: 'center',
marginBottom: 20,
},
picker: {
borderColor: colors.border.default,
borderRadius: 5,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import React from 'react';
import { fireEvent, act } from '@testing-library/react-native';
import renderWithProvider from '../../../../../util/test/renderWithProvider';
import { backgroundState } from '../../../../../util/test/initial-root-state';
import { strings } from '../../../../../../locales/i18n';
import { ResetAccountModal } from './ResetAccountModal';
import { AdvancedViewSelectorsIDs } from '../AdvancedView.testIds';
import { selectSelectedInternalAccountFormattedAddress } from '../../../../../selectors/accountsController';
import { selectChainId } from '../../../../../selectors/networkController';
import { wipeTransactions } from '../../../../../util/transaction-controller';
Expand Down Expand Up @@ -63,11 +63,6 @@ describe('ResetAccountModal', () => {
const defaultProps = {
resetModalVisible: true,
cancelResetAccount: jest.fn(),
styles: {
modalView: {},
modalTitle: {},
modalText: {},
},
};

beforeEach(() => {
Expand All @@ -79,13 +74,13 @@ describe('ResetAccountModal', () => {
});

it('calls wipeBridgeStatus, wipeTransactions, and wipeSmartTransactions when reset button is pressed', async () => {
const { getByText } = renderWithProvider(
const { getByTestId } = renderWithProvider(
<ResetAccountModal {...defaultProps} />,
{ state: initialState },
);

const confirmButton = getByText(
strings('app_settings.reset_account_confirm_button'),
const confirmButton = getByTestId(
AdvancedViewSelectorsIDs.RESET_ACCOUNT_CONFIRM_BUTTON,
);
fireEvent.press(confirmButton);

Expand All @@ -105,13 +100,13 @@ describe('ResetAccountModal', () => {
selectSelectedInternalAccountFormattedAddress as unknown as jest.Mock
).mockReturnValue(undefined);

const { getByText } = renderWithProvider(
const { getByTestId } = renderWithProvider(
<ResetAccountModal {...defaultProps} />,
{ state: initialState },
);

const confirmButton = getByText(
strings('app_settings.reset_account_confirm_button'),
const confirmButton = getByTestId(
AdvancedViewSelectorsIDs.RESET_ACCOUNT_CONFIRM_BUTTON,
);
fireEvent.press(confirmButton);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import React from 'react';
import { View } from 'react-native';
import Text, {
TextVariant,
} from '../../../../../component-library/components/Texts/Text';
import React, { useCallback, useRef } from 'react';
import { strings } from '../../../../../../locales/i18n';
import ActionModal from '../../../../UI/ActionModal';
import { wipeTransactions } from '../../../../../util/transaction-controller';
import { wipeSmartTransactions } from '../../../../../util/smart-transactions';
import { wipeBridgeStatus } from '../../../../UI/Bridge/utils';
Expand All @@ -15,26 +10,33 @@ import { selectSelectedInternalAccountFormattedAddress } from '../../../../../se
import { selectChainId } from '../../../../../selectors/networkController';
import { usePerpsFirstTimeUser } from '../../../../UI/Perps/hooks/usePerpsFirstTimeUser';
import { AdvancedViewSelectorsIDs } from '../AdvancedView.testIds';
import {
BottomSheet,
BottomSheetFooter,
BottomSheetHeader,
type BottomSheetRef,
Box,
Text,
TextVariant,
} from '@metamask/design-system-react-native';

export const ResetAccountModal = ({
resetModalVisible,
cancelResetAccount,
styles,
}: {
resetModalVisible: boolean;
cancelResetAccount: () => void;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
styles: any;
}) => {
const navigation = useNavigation<AppNavigationProp>();
const sheetRef = useRef<BottomSheetRef>(null);
const selectedAddress = useSelector(
selectSelectedInternalAccountFormattedAddress,
);
const chainId = useSelector(selectChainId);
const { resetFirstTimeUserState, clearPendingTransactionRequests } =
usePerpsFirstTimeUser();

const resetAccount = () => {
const resetAccount = useCallback(() => {
if (selectedAddress) {
wipeBridgeStatus(selectedAddress, chainId);
wipeSmartTransactions(selectedAddress);
Expand All @@ -45,26 +47,55 @@ export const ResetAccountModal = ({
// Clear any stuck pending Perps transactions
clearPendingTransactionRequests();
navigation.navigate('WalletView');
};
}, [
chainId,
clearPendingTransactionRequests,
navigation,
resetFirstTimeUserState,
selectedAddress,
]);

const handleRequestClose = useCallback(() => {
sheetRef.current?.onCloseBottomSheet();
}, []);

const handleConfirm = useCallback(() => {
sheetRef.current?.onCloseBottomSheet(() => {
resetAccount();
});
}, [resetAccount]);

if (!resetModalVisible) {
return null;
}

return (
<ActionModal
modalVisible={resetModalVisible}
confirmText={strings('app_settings.reset_account_confirm_button')}
cancelText={strings('app_settings.reset_account_cancel_button')}
confirmTestID={AdvancedViewSelectorsIDs.RESET_ACCOUNT_CONFIRM_BUTTON}
onCancelPress={cancelResetAccount}
onRequestClose={cancelResetAccount}
onConfirmPress={resetAccount}
<BottomSheet
ref={sheetRef}
isInteractable
onClose={cancelResetAccount}
keyboardAvoidingViewEnabled
>
<View style={styles.modalView}>
<Text style={styles.modalTitle} variant={TextVariant.HeadingMD}>
{strings('app_settings.reset_account_modal_title')}
</Text>
<Text style={styles.modalText}>
<BottomSheetHeader onClose={handleRequestClose}>
{strings('app_settings.reset_account_modal_title')}
</BottomSheetHeader>
<Box twClassName="px-4 pt-2 pb-6">
<Text variant={TextVariant.BodyMd} twClassName="text-center">
{strings('app_settings.reset_account_modal_message')}
</Text>
</View>
</ActionModal>
</Box>
<BottomSheetFooter
secondaryButtonProps={{
children: strings('app_settings.reset_account_cancel_button'),
onPress: handleRequestClose,
}}
primaryButtonProps={{
children: strings('app_settings.reset_account_confirm_button'),
onPress: handleConfirm,
isDanger: true,
testID: AdvancedViewSelectorsIDs.RESET_ACCOUNT_CONFIRM_BUTTON,
}}
/>
</BottomSheet>
);
};
9 changes: 4 additions & 5 deletions app/components/Views/Settings/AdvancedSettings/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,6 @@ const AdvancedSettings = ({
ref={scrollView}
>
<View style={styles.inner} testID={AdvancedViewSelectorsIDs.CONTAINER}>
<ResetAccountModal
resetModalVisible={resetModalVisible}
cancelResetAccount={cancelResetAccount}
styles={styles}
/>
<View style={[styles.setting, styles.firstSetting]}>
<Text variant={TextVariant.BodyMd} fontWeight={FontWeight.Medium}>
{strings('app_settings.reset_account')}
Expand Down Expand Up @@ -284,6 +279,10 @@ const AdvancedSettings = ({
</View>
</View>
</KeyboardAwareScrollView>
<ResetAccountModal
resetModalVisible={resetModalVisible}
cancelResetAccount={cancelResetAccount}
/>
</SafeAreaView>
);
};
Expand Down
Loading
Loading