From 6fbf067223b57174ec298d4be8dc8b84250ba153 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 26 Sep 2023 17:11:19 +0800 Subject: [PATCH] lint and improve modal behavior, clarify comments --- src/components/DistanceRequest.js | 5 ++-- src/libs/actions/TransactionEdit.js | 1 - src/pages/EditRequestDistancePage.js | 44 +++++++++++++++++++++++----- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/components/DistanceRequest.js b/src/components/DistanceRequest.js index 60e6207b1b42..30e5adfc62b0 100644 --- a/src/components/DistanceRequest.js +++ b/src/components/DistanceRequest.js @@ -103,6 +103,7 @@ function DistanceRequest({transactionID, report, transaction, mapboxAccessToken, const lastWaypointIndex = numberOfWaypoints - 1; const isLoadingRoute = lodashGet(transaction, 'comment.isLoading', false); + const isLoading = lodashGet(transaction, 'isLoading', false); const hasRouteError = !!lodashGet(transaction, 'errorFields.route'); const hasRoute = TransactionUtils.hasRoute(transaction); const validatedWaypoints = TransactionUtils.getValidWaypoints(waypoints); @@ -289,9 +290,9 @@ function DistanceRequest({transactionID, report, transaction, mapboxAccessToken, success style={[styles.w100, styles.mb4, styles.ph4, styles.flexShrink0]} onPress={() => onSubmit(waypoints)} - isDisabled={_.size(validatedWaypoints) < 2 || hasRouteError || isLoadingRoute} + isDisabled={!isOffline && (_.size(validatedWaypoints) < 2 || hasRouteError || isLoadingRoute || isLoading)} text={translate(isEditingRequest ? 'common.save' : 'common.next')} - isLoading={isLoadingRoute || shouldFetchRoute} + isLoading={!isOffline && (isLoadingRoute || shouldFetchRoute || isLoading)} /> ); diff --git a/src/libs/actions/TransactionEdit.js b/src/libs/actions/TransactionEdit.js index aba534a6de9e..44b489b72c43 100644 --- a/src/libs/actions/TransactionEdit.js +++ b/src/libs/actions/TransactionEdit.js @@ -1,5 +1,4 @@ import Onyx from 'react-native-onyx'; -import _ from 'underscore'; import ONYXKEYS from '../../ONYXKEYS'; /** diff --git a/src/pages/EditRequestDistancePage.js b/src/pages/EditRequestDistancePage.js index 96d8f3123a85..96aaf233b4df 100644 --- a/src/pages/EditRequestDistancePage.js +++ b/src/pages/EditRequestDistancePage.js @@ -14,6 +14,8 @@ import reportPropTypes from './reportPropTypes'; import * as IOU from '../libs/actions/IOU'; import transactionPropTypes from '../components/transactionPropTypes'; import * as TransactionEdit from '../libs/actions/TransactionEdit'; +import useNetwork from '../hooks/useNetwork'; +import usePrevious from '../hooks/usePrevious'; const propTypes = { /** The transactionID we're currently editing */ @@ -44,30 +46,51 @@ const defaultProps = { }; function EditRequestDistancePage({report, route, transaction}) { + const {isOffline} = useNetwork(); const {translate} = useLocalize(); const transactionWasSaved = useRef(false); const hasWaypointError = useRef(false); + const prevIsLoading = usePrevious(transaction.isLoading); useEffect(() => { hasWaypointError.current = Boolean(lodashGet(transaction, 'errorFields.route') || lodashGet(transaction, 'errorFields.waypoints')); - }, [transaction]); + + // When the loading goes from true to false, then we know the transaction has just been + // saved to the server. Check for errors. If there are no errors, then the modal can be closed. + if (prevIsLoading && !transaction.isLoading && !hasWaypointError.current) { + Navigation.dismissModal(report.reportID); + } + }, [transaction, prevIsLoading]); useEffect(() => { - // When the component is mounted, make a backup copy of the original transaction that is stored in onyx + // This effect runs when the component is mounted and unmounted. It's purpose is to be able to properly + // discard changes if the user cancels out of making any changes. This is accomplished by backing up the + // original transaction, letting the user modify the current transaction, and then if the user ever + // cancels out of the modal without saving changes, the original transaction is restored from the backup. + + // On mount, create the backup transaction. TransactionEdit.createBackupTransaction(transaction); return () => { - // When the component is unmounted - // If the transaction was saved without errors - // Then remove the backup transaction because it is no longer needed + // Unmounting happens when: + // 1. The transaction was saved offline or with no server errors + // 2. The user cancels out of the modal + + // If 1 happened, then only the backup transaction needs to be removed because it is no longer needed if (transactionWasSaved.current && !hasWaypointError.current) { TransactionEdit.removeBackupTransaction(transaction.transactionID); return; } - // If the transaction had errors, or wasn't saved, then the original transaction - // needs to be restored or else errors and edited fields will remain in the UI + // If 2 happened, then the original transaction needs to be restored so that no user changes are stored to onyx permanently. + // This also also deletes the backup. TransactionEdit.restoreOriginalTransactionFromBackup(transaction.transactionID); + + // You might be asking yourself "What about the case where the user saves the transaction, but there are server errors?" + // When this happens, the modal remains open so the user can fix the errors. Therefore, this component is never unmounted and this + // logic doesn't run. Once the user fixes the errors, they can: + // - Cancel out of the modal (triggering flow 2 above) + // - Fix the errors and save the transaction (triggering flow 1 above) }; // eslint-disable-next-line react-hooks/exhaustive-deps }, []); @@ -79,7 +102,12 @@ function EditRequestDistancePage({report, route, transaction}) { const saveTransaction = (waypoints) => { transactionWasSaved.current = true; IOU.updateDistanceRequest(transaction.transactionID, report.reportID, {waypoints}); - Navigation.dismissModal(report.reportID); + + // If the client is offline, then the modal can be closed as well (because there are no errors or other feedback to show them + // until they come online again and sync with the server). + if (isOffline) { + Navigation.dismissModal(report.reportID); + } }; return (