Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: refactor money request form #24274

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
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