[Odometer] Improvement to DiscardChangesConfirmation usage

src/hooks/useBeforeRemove.tsx

@@ -3,16 +3,13 @@ import type {EventListenerCallback, EventMapCore, NavigationState} from '@react-
 import {useEffect} from 'react';
 
 // beforeRemove have some limitations. When the react-navigation is upgraded to 7.x, update this to use usePreventRemove hook.
-const useBeforeRemove = (onBeforeRemove: EventListenerCallback<EventMapCore<NavigationState>, 'beforeRemove'>, isEnabled = true) => {
+const useBeforeRemove = (onBeforeRemove: EventListenerCallback<EventMapCore<NavigationState>, 'beforeRemove'>) => {
     const navigation = useNavigation();
 
     useEffect(() => {
-        if (!isEnabled) {
-            return undefined;
-        }
         const unsubscribe = navigation.addListener('beforeRemove', onBeforeRemove);
         return unsubscribe;
-    }, [navigation, onBeforeRemove, isEnabled]);
+    }, [navigation, onBeforeRemove]);
 };
 
 export default useBeforeRemove;

src/hooks/useDiscardChangesConfirmation/index.native.ts

@@ -1,20 +1,20 @@
 import type {NavigationAction} from '@react-navigation/native';
-import {useIsFocused, usePreventRemove} from '@react-navigation/native';
+import {usePreventRemove} from '@react-navigation/native';
 import {useCallback, useRef, useState} from 'react';
 import {ModalActions} from '@components/Modal/Global/ModalContext';
 import useConfirmModal from '@hooks/useConfirmModal';
 import useLocalize from '@hooks/useLocalize';
+import Log from '@libs/Log';
 import navigationRef from '@libs/Navigation/navigationRef';
 import type UseDiscardChangesConfirmationOptions from './types';
 
-function useDiscardChangesConfirmation({getHasUnsavedChanges, onVisibilityChange, isEnabled = true}: UseDiscardChangesConfirmationOptions) {
+function useDiscardChangesConfirmation({getHasUnsavedChanges, onCancel, onVisibilityChange, onConfirm}: UseDiscardChangesConfirmationOptions) {
     const {translate} = useLocalize();
-    const isFocused = useIsFocused();
     const {showConfirmModal} = useConfirmModal();
     const [shouldAllowNavigation, setShouldAllowNavigation] = useState(false);
     const blockedNavigationAction = useRef<NavigationAction | undefined>(undefined);
 
-    const shouldPrevent = isEnabled && isFocused && !shouldAllowNavigation;
+    const shouldPrevent = !shouldAllowNavigation;
 
     usePreventRemove(
         shouldPrevent,
@@ -36,18 +36,28 @@ function useDiscardChangesConfirmation({getHasUnsavedChanges, onVisibilityChange
                 }).then((result) => {
                     onVisibilityChange?.(false);
                     if (result.action !== ModalActions.CONFIRM) {
+                        onCancel?.();
                         return;
                     }
-                    setShouldAllowNavigation(true);
-                    if (blockedNavigationAction.current) {
-                        navigationRef.current?.dispatch(blockedNavigationAction.current);
-                        blockedNavigationAction.current = undefined;
-                    } else {
-                        navigationRef.current?.goBack();
-                    }
+                    const confirmNavigation = () => {
+                        setShouldAllowNavigation(true);
+                        if (blockedNavigationAction.current) {
+                            navigationRef.current?.dispatch(blockedNavigationAction.current);
+                            blockedNavigationAction.current = undefined;
+                        } else {
+                            navigationRef.current?.goBack();
+                        }
+                    };
+                    Promise.resolve()
+                        .then(() => onConfirm?.())
+                        .then(confirmNavigation)
+                        .catch((error: unknown) => {
+                            Log.warn('[useDiscardChangesConfirmation] Failed to run onConfirm callback', {error});
+                            blockedNavigationAction.current = undefined;
+                        });
                 });
             },
-            [getHasUnsavedChanges, onVisibilityChange, showConfirmModal, translate],
+            [getHasUnsavedChanges, onCancel, onVisibilityChange, onConfirm, showConfirmModal, translate],
         ),
     );
 }

src/hooks/useDiscardChangesConfirmation/index.ts

@@ -1,25 +1,24 @@
 import type {NavigationAction} from '@react-navigation/native';
-import {useIsFocused, useNavigation} from '@react-navigation/native';
+import {useNavigation} from '@react-navigation/native';
 import {useCallback, useEffect, useRef} from 'react';
 import {ModalActions} from '@components/Modal/Global/ModalContext';
 import useBeforeRemove from '@hooks/useBeforeRemove';
 import useConfirmModal from '@hooks/useConfirmModal';
 import useLocalize from '@hooks/useLocalize';
+import Log from '@libs/Log';
 import setNavigationActionToMicrotaskQueue from '@libs/Navigation/helpers/setNavigationActionToMicrotaskQueue';
 import navigateAfterInteraction from '@libs/Navigation/navigateAfterInteraction';
 import navigationRef from '@libs/Navigation/navigationRef';
 import type {PlatformStackNavigationProp} from '@libs/Navigation/PlatformStackNavigation/types';
 import type {RootNavigatorParamList} from '@libs/Navigation/types';
 import type UseDiscardChangesConfirmationOptions from './types';
 
-function useDiscardChangesConfirmation({getHasUnsavedChanges, onCancel, onVisibilityChange, isEnabled = true}: UseDiscardChangesConfirmationOptions) {
+function useDiscardChangesConfirmation({getHasUnsavedChanges, onCancel, onVisibilityChange, onConfirm}: UseDiscardChangesConfirmationOptions) {
     const navigation = useNavigation<PlatformStackNavigationProp<RootNavigatorParamList>>();
-    const isFocused = useIsFocused();
     const {translate} = useLocalize();
-    const {showConfirmModal, closeModal} = useConfirmModal();
+    const {showConfirmModal} = useConfirmModal();
     const blockedNavigationAction = useRef<NavigationAction>(undefined);
     const shouldNavigateBack = useRef(false);
-    const isDiscardModalOpenRef = useRef(false);
 
     const navigateBack = useCallback(() => {
         if (blockedNavigationAction.current) {
@@ -34,7 +33,6 @@ function useDiscardChangesConfirmation({getHasUnsavedChanges, onCancel, onVisibi
 
     const showDiscardModal = useCallback(() => {
         onVisibilityChange?.(true);
-        isDiscardModalOpenRef.current = true;
         showConfirmModal({
             title: translate('discardChangesConfirmation.title'),
             prompt: translate('discardChangesConfirmation.body'),
@@ -43,43 +41,41 @@ function useDiscardChangesConfirmation({getHasUnsavedChanges, onCancel, onVisibi
             cancelText: translate('common.cancel'),
             shouldIgnoreBackHandlerDuringTransition: true,
         }).then((result) => {
-            isDiscardModalOpenRef.current = false;
             onVisibilityChange?.(false);
             if (result.action === ModalActions.CONFIRM) {
-                setNavigationActionToMicrotaskQueue(navigateBack);
+                Promise.resolve()
+                    .then(() => onConfirm?.())
+                    .then(() => {
+                        setNavigationActionToMicrotaskQueue(navigateBack);
+                    })
+                    .catch((error: unknown) => {
+                        Log.warn('[useDiscardChangesConfirmation] Failed to run onConfirm callback', {error});
+                        blockedNavigationAction.current = undefined;
+                        shouldNavigateBack.current = false;
+                    });
             } else {
                 blockedNavigationAction.current = undefined;
                 shouldNavigateBack.current = false;
                 onCancel?.();
             }
         });
-    }, [showConfirmModal, translate, navigateBack, onCancel, onVisibilityChange]);
+    }, [showConfirmModal, translate, navigateBack, onCancel, onConfirm, onVisibilityChange]);
 
-    useBeforeRemove(
-        useCallback(
-            (e) => {
-                if (!isEnabled || !isFocused || !getHasUnsavedChanges() || shouldNavigateBack.current) {
-                    return;
-                }
-
-                e.preventDefault();
-                blockedNavigationAction.current = e.data.action;
-                navigateAfterInteraction(showDiscardModal);
-            },
-            [getHasUnsavedChanges, isFocused, isEnabled, showDiscardModal],
-        ),
-        isEnabled && isFocused,
-    );
+    useBeforeRemove((e) => {
+        if (!getHasUnsavedChanges() || shouldNavigateBack.current) {
+            return;
+        }
+        e.preventDefault();
+        blockedNavigationAction.current = e.data.action;
+        navigateAfterInteraction(showDiscardModal);
+    });
 
     /**
      * We cannot programmatically stop the browser's back navigation like react-navigation's beforeRemove.
      * Events like popstate and transitionStart are triggered AFTER the back navigation has already completed.
      * So we need to go forward to get back to the current page.
      */
     useEffect(() => {
-        if (!isEnabled || !isFocused) {
-            return undefined;
-        }
         const unsubscribe = navigation.addListener('transitionStart', ({data: {closing}}) => {
             // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
             if (!getHasUnsavedChanges()) {
@@ -95,20 +91,7 @@ function useDiscardChangesConfirmation({getHasUnsavedChanges, onCancel, onVisibi
         });
 
         return unsubscribe;
-    }, [navigation, getHasUnsavedChanges, isFocused, isEnabled, showDiscardModal]);
-
-    /**
-     * When the screen loses focus (or is disabled) while the discard modal is open,
-     * close the modal and reset refs so we don't leave the modal visible or stale state.
-     */
-    useEffect(() => {
-        if ((isFocused && isEnabled) || !isDiscardModalOpenRef.current) {
-            return;
-        }
-        closeModal();
-        blockedNavigationAction.current = undefined;
-        shouldNavigateBack.current = false;
-    }, [isFocused, isEnabled, closeModal]);
+    }, [navigation, getHasUnsavedChanges, showDiscardModal]);
 }
 
 export default useDiscardChangesConfirmation;

src/hooks/useDiscardChangesConfirmation/types.ts

@@ -2,7 +2,7 @@ type UseDiscardChangesConfirmationOptions = {
     getHasUnsavedChanges: () => boolean;
     onCancel?: () => void;
     onVisibilityChange?: (visible: boolean) => void;
-    isEnabled?: boolean;
+    onConfirm?: () => void | Promise<void>;
 };
 
 export default UseDiscardChangesConfirmationOptions;

src/libs/actions/TransactionEdit.ts

@@ -54,12 +54,12 @@ function createBackupTransaction(transaction: OnyxEntry<Transaction>, isDraft: b
 /**
  * Removes a transaction from Onyx that was only used temporary in the edit flow
  */
-function removeBackupTransaction(transactionID: string | undefined) {
+function removeBackupTransaction(transactionID: string | undefined): Promise<void> {
     if (!transactionID) {
-        return;
+        return Promise.resolve();
     }
 
-    Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_BACKUP}${transactionID}`, null);
+    return Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_BACKUP}${transactionID}`, null);
 }
 
 function restoreOriginalTransactionFromBackup(transactionID: string | undefined, isDraft: boolean) {
@@ -187,7 +187,7 @@ function buildOptimisticTransactionAndCreateDraft({initialTransaction, currentUs
     return newTransaction;
 }
 
-function removeBackupTransactionWithImageCleanup(transactionID: string | undefined, isDraft: boolean, onComplete?: () => void) {
+function removeBackupTransactionWithImageCleanup(transactionID: string | undefined, isDraft: boolean) {
     if (!transactionID) {
         return;
     }
@@ -202,33 +202,36 @@ function removeBackupTransactionWithImageCleanup(transactionID: string | undefin
                     revokeOdometerImageUri(backupTransaction?.comment?.odometerStartImage, currentTransaction?.comment?.odometerStartImage);
                     revokeOdometerImageUri(backupTransaction?.comment?.odometerEndImage, currentTransaction?.comment?.odometerEndImage);
                     removeBackupTransaction(transactionID);
-                    onComplete?.();
                 },
             });
         },
     });
 }
 
-function restoreOriginalTransactionFromBackupWithImageCleanup(transactionID: string | undefined, isDraft: boolean, onComplete?: () => void) {
+function restoreOriginalTransactionFromBackupWithImageCleanup(transactionID: string | undefined, isDraft: boolean): Promise<void> {
     if (!transactionID) {
-        return;
+        return Promise.resolve();
     }
-    connection = Onyx.connectWithoutView({
-        key: `${ONYXKEYS.COLLECTION.TRANSACTION_BACKUP}${transactionID}`,
-        callback: (backupTransaction) => {
-            Onyx.disconnect(connection);
-            const currentConn = Onyx.connectWithoutView({
-                key: `${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
-                callback: (currentTransaction) => {
-                    Onyx.disconnect(currentConn);
-                    revokeOdometerImageUri(currentTransaction?.comment?.odometerStartImage, backupTransaction?.comment?.odometerStartImage);
-                    revokeOdometerImageUri(currentTransaction?.comment?.odometerEndImage, backupTransaction?.comment?.odometerEndImage);
-                    Onyx.set(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, backupTransaction ?? null);
-                    removeBackupTransaction(transactionID);
-                    onComplete?.();
-                },
-            });
-        },
+
+    return new Promise((resolve, reject) => {
+        connection = Onyx.connectWithoutView({
+            key: `${ONYXKEYS.COLLECTION.TRANSACTION_BACKUP}${transactionID}`,
+            callback: (backupTransaction) => {
+                Onyx.disconnect(connection);
+                const currentConn = Onyx.connectWithoutView({
+                    key: `${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
+                    callback: (currentTransaction) => {
+                        Onyx.disconnect(currentConn);
+                        revokeOdometerImageUri(currentTransaction?.comment?.odometerStartImage, backupTransaction?.comment?.odometerStartImage);
+                        revokeOdometerImageUri(currentTransaction?.comment?.odometerEndImage, backupTransaction?.comment?.odometerEndImage);
+                        Onyx.set(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, backupTransaction ?? null)
+                            .then(() => removeBackupTransaction(transactionID))
+                            .then(() => resolve())
+                            .catch((error) => reject(error));
+                    },
+                });
+            },
+        });
     });
 }
 

src/pages/MultifactorAuthentication/AuthorizeTransactionPage/index.tsx

@@ -76,18 +76,15 @@ function MultifactorAuthenticationScenarioAuthorizeTransactionPage({route}: Mult
         setConfirmModalVisibility(false);
     };
 
-    const onBeforeRemove: Parameters<typeof useBeforeRemove>[0] = useCallback(
-        (e) => {
-            if (allowNavigatingAwayRef.current) {
-                return;
-            }
-            e.preventDefault();
-            showConfirmModal();
-        },
-        [showConfirmModal],
-    );
+    const onBeforeRemove: Parameters<typeof useBeforeRemove>[0] = (e) => {
+        if (allowNavigatingAwayRef.current || !transaction || !!denyOutcomeScreen) {
+            return;
+        }
+        e.preventDefault();
+        showConfirmModal();
+    };
 
-    useBeforeRemove(onBeforeRemove, !!transaction && !denyOutcomeScreen);
+    useBeforeRemove(onBeforeRemove);
 
     const onApproveTransaction = () => {
         addBreadcrumb('Approve tapped', {transactionID});