Skip to content

Commit

Permalink
Merge pull request #24274 from koko57/refactor/23149-follow-up-money-…
Browse files Browse the repository at this point in the history
…request-page

refactor: refactor money request form
  • Loading branch information
marcaaron authored Aug 15, 2023
2 parents 38ed41f + cabcce4 commit 2e8e699
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 30 deletions.
15 changes: 14 additions & 1 deletion src/components/TextInputWithCurrencySymbol.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import PropTypes from 'prop-types';
import AmountTextInput from './AmountTextInput';
import CurrencySymbolButton from './CurrencySymbolButton';
import * as CurrencyUtils from '../libs/CurrencyUtils';
import useLocalize from '../hooks/useLocalize';
import * as MoneyRequestUtils from '../libs/MoneyRequestUtils';

const propTypes = {
/** A ref to forward to amount text input */
Expand Down Expand Up @@ -46,6 +48,7 @@ const defaultProps = {
};

function TextInputWithCurrencySymbol(props) {
const {fromLocaleDigit} = useLocalize();
const currencySymbol = CurrencyUtils.getLocalizedCurrencySymbol(props.selectedCurrencyCode);
const isCurrencySymbolLTR = CurrencyUtils.isCurrencySymbolLTR(props.selectedCurrencyCode);

Expand All @@ -63,10 +66,20 @@ function TextInputWithCurrencySymbol(props) {
/>
);

/**
* Set a new amount value properly formatted
*
* @param {String} text - Changed text from user input
*/
const setFormattedAmount = (text) => {
const newAmount = MoneyRequestUtils.addLeadingZero(MoneyRequestUtils.replaceAllDigits(text, fromLocaleDigit));
props.onChangeAmount(newAmount);
};

const amountTextInput = (
<AmountTextInput
formattedAmount={props.formattedAmount}
onChangeAmount={props.onChangeAmount}
onChangeAmount={setFormattedAmount}
placeholder={props.placeholder}
ref={props.forwardedRef}
selection={props.selection}
Expand Down
34 changes: 6 additions & 28 deletions src/pages/iou/steps/MoneyRequestAmountForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const NUM_PAD_CONTAINER_VIEW_ID = 'numPadContainerView';
const NUM_PAD_VIEW_ID = 'numPadView';

function MoneyRequestAmountForm({amount, currency, isEditing, forwardedRef, onCurrencyButtonPress, onSubmitButtonPress, disableCurrency}) {
const {translate, toLocaleDigit, fromLocaleDigit, numberFormat} = useLocalize();
const {translate, toLocaleDigit, numberFormat} = useLocalize();

const textInput = useRef(null);

Expand Down Expand Up @@ -96,32 +96,22 @@ function MoneyRequestAmountForm({amount, currency, isEditing, forwardedRef, onCu
}
};

/**
* Convert amount to whole unit and update selection
*
* @param {String} currencyCode
* @param {Number} amountInCurrencyUnits
*/
const saveAmountToState = (currencyCode, amountInCurrencyUnits) => {
if (!currencyCode || !amountInCurrencyUnits) {
useEffect(() => {
if (!currency || !amount) {
return;
}
const amountAsStringForState = CurrencyUtils.convertToWholeUnit(currencyCode, amountInCurrencyUnits).toString();
const amountAsStringForState = CurrencyUtils.convertToWholeUnit(currency, amount).toString();
setCurrentAmount(amountAsStringForState);
setSelection({
start: amountAsStringForState.length,
end: amountAsStringForState.length,
});
};

useEffect(() => {
saveAmountToState(currency, amount);
// we want to update the state only when the amount is changed
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [amount]);

/**
* Sets the state according to amount that is passed
* Sets the selection and the amount accordingly to the value passed to the input
* @param {String} newAmount - Changed amount from user input
*/
const setNewAmount = (newAmount) => {
Expand All @@ -131,7 +121,6 @@ function MoneyRequestAmountForm({amount, currency, isEditing, forwardedRef, onCu
// Use a shallow copy of selection to trigger setSelection
// More info: https://github.com/Expensify/App/issues/16385
if (!MoneyRequestUtils.validateAmount(newAmountWithoutSpaces)) {
setCurrentAmount((prevAmount) => prevAmount);
setSelection((prevSelection) => ({...prevSelection}));
return;
}
Expand Down Expand Up @@ -179,17 +168,6 @@ function MoneyRequestAmountForm({amount, currency, isEditing, forwardedRef, onCu
}
}, []);

/**
* Update amount on amount change
* Validate new amount with decimal number regex up to 6 digits and 2 decimal digit
*
* @param {String} text - Changed text from user input
*/
const updateAmount = (text) => {
const newAmount = MoneyRequestUtils.addLeadingZero(MoneyRequestUtils.replaceAllDigits(text, fromLocaleDigit));
setNewAmount(newAmount);
};

/**
* Submit amount and navigate to a proper page
*/
Expand All @@ -209,7 +187,7 @@ function MoneyRequestAmountForm({amount, currency, isEditing, forwardedRef, onCu
>
<TextInputWithCurrencySymbol
formattedAmount={formattedAmount}
onChangeAmount={updateAmount}
onChangeAmount={setNewAmount}
onCurrencyButtonPress={onCurrencyButtonPress}
placeholder={numberFormat(0)}
ref={(ref) => {
Expand Down
4 changes: 3 additions & 1 deletion src/pages/iou/steps/NewRequestAmountPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,11 @@ function NewRequestAmountPage({route, iou, report}) {
Navigation.dismissModal(reportID);
}, [report, reportID]);

// Because we use Onyx to store iou info, when we try to make two different money requests from different tabs, it can result in many bugs.
// Because we use Onyx to store IOU info, when we try to make two different money requests from different tabs,
// it can result in an IOU sent with improper values. In such cases we want to reset the flow and redirect the user to the first step of the IOU.
useEffect(() => {
if (isEditing) {
// ID in Onyx could change by initiating a new request in a separate browser tab or completing a request
if (prevMoneyRequestID.current !== iou.id) {
// The ID is cleared on completing a request. In that case, we will do nothing.
if (!iou.id) {
Expand Down

0 comments on commit 2e8e699

Please sign in to comment.