Skip to content

Commit

Permalink
lint and improve modal behavior, clarify comments
Browse files Browse the repository at this point in the history
  • Loading branch information
tgolen committed Sep 26, 2023
1 parent f84362e commit 6fbf067
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 11 deletions.
5 changes: 3 additions & 2 deletions src/components/DistanceRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)}
/>
</ScrollView>
);
Expand Down
1 change: 0 additions & 1 deletion src/libs/actions/TransactionEdit.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import Onyx from 'react-native-onyx';
import _ from 'underscore';
import ONYXKEYS from '../../ONYXKEYS';

/**
Expand Down
44 changes: 36 additions & 8 deletions src/pages/EditRequestDistancePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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]);

Check warning on line 63 in src/pages/EditRequestDistancePage.js

View workflow job for this annotation

GitHub Actions / lint

React Hook useEffect has a missing dependency: 'report.reportID'. Either include it or remove the dependency array

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
}, []);
Expand All @@ -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 (
Expand Down

0 comments on commit 6fbf067

Please sign in to comment.