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.

Sign up for GitHub

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

Fix split amount input does not animate smoothly #42241

Merged
merged 5 commits into from
May 29, 2024
Merged

Fix split amount input does not animate smoothly #42241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
70584ff
make the split amount input a fixed width
bernhardoj May 16, 2024
2522b60
refactor formatting logic to be a props
bernhardoj May 20, 2024
3f7b486
move the amount chceking outside the function
bernhardoj May 20, 2024
d1aa802
pass currency
bernhardoj May 20, 2024
f784e54
Merge branch 'main' into fix/41751-split-amount-doesnt-animate-smoothly
bernhardoj May 28, 2024
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
21 changes: 17 additions & 4 deletions src/components/MoneyRequestAmountInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ type MoneyRequestAmountInputProps = {
/** IOU amount saved in Onyx */
amount?: number;

/** A callback to format the amount number */
onFormatAmount?: (amount: number, currency?: string) => string;

/** Currency chosen by user or saved in Onyx */
currency?: string;

Expand Down Expand Up @@ -74,6 +77,11 @@ type MoneyRequestAmountInputProps = {

/** Hide the focus styles on TextInput */
hideFocusedState?: boolean;

/**
* Autogrow input container length based on the entered text.
*/
autoGrow?: boolean;
};

type Selection = {
Expand All @@ -89,6 +97,8 @@ const getNewSelection = (oldSelection: Selection, prevLength: number, newLength:
return {start: cursorPosition, end: cursorPosition};
};

const defaultOnFormatAmount = (amount: number) => CurrencyUtils.convertToFrontendAmount(amount).toString();

function MoneyRequestAmountInput(
{
amount = 0,
Expand All @@ -101,9 +111,11 @@ function MoneyRequestAmountInput(
shouldUpdateSelection = true,
moneyRequestAmountInputRef,
disableKeyboard = true,
onFormatAmount = defaultOnFormatAmount,
formatAmountOnBlur,
maxLength,
hideFocusedState = true,
autoGrow = true,
...props
}: MoneyRequestAmountInputProps,
forwardedRef: ForwardedRef<BaseTextInputRef>,
Expand All @@ -113,7 +125,7 @@ function MoneyRequestAmountInput(
const textInput = useRef<BaseTextInputRef | null>(null);

const decimals = CurrencyUtils.getCurrencyDecimals(currency);
const selectedAmountAsString = amount ? CurrencyUtils.convertToFrontendAmount(amount).toString() : '';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's been there because this is an old logic. selectedAmountAsString is used as the initial state for the amount and we don't use the correct formatting logic for our case.

Copy link
Contributor

@youssef-lr youssef-lr May 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah okay, that makes sense, thanks for catching this!

const selectedAmountAsString = amount ? onFormatAmount(amount, currency) : '';

const [currentAmount, setCurrentAmount] = useState(selectedAmountAsString);

Expand Down Expand Up @@ -182,7 +194,7 @@ function MoneyRequestAmountInput(
if (!currency || typeof amount !== 'number' || (formatAmountOnBlur && textInput.current?.isFocused())) {
return;
}
const frontendAmount = formatAmountOnBlur ? CurrencyUtils.convertToDisplayStringWithoutCurrency(amount, currency) : CurrencyUtils.convertToFrontendAmount(amount).toString();
const frontendAmount = onFormatAmount(amount, currency);
setCurrentAmount(frontendAmount);

// Only update selection if the amount prop was changed from the outside and is not the same as the current amount we just computed
Expand Down Expand Up @@ -233,7 +245,7 @@ function MoneyRequestAmountInput(
if (!formatAmountOnBlur) {
return;
}
const formattedAmount = CurrencyUtils.convertToDisplayStringWithoutCurrency(amount, currency);
const formattedAmount = onFormatAmount(amount, currency);
if (maxLength && formattedAmount.length > maxLength) {
return;
}
Expand All @@ -242,12 +254,13 @@ function MoneyRequestAmountInput(
start: formattedAmount.length,
end: formattedAmount.length,
});
}, [amount, currency, formatAmountOnBlur, maxLength]);
}, [amount, currency, onFormatAmount, formatAmountOnBlur, maxLength]);

const formattedAmount = MoneyRequestUtils.replaceAllDigits(currentAmount, toLocaleDigit);

return (
<TextInputWithCurrencySymbol
autoGrow={autoGrow}
disableKeyboard={disableKeyboard}
formattedAmount={formattedAmount}
onChangeAmount={setNewAmount}
Expand Down
4 changes: 3 additions & 1 deletion src/components/MoneyRequestConfirmationList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,7 @@ function MoneyRequestConfirmationList({
isSelected: false,
rightElement: (
<MoneyRequestAmountInput
autoGrow={false}
amount={transaction?.splitShares?.[participantOption.accountID ?? 0]?.amount}
currency={iouCurrencyCode}
prefixCharacter={currencySymbol}
Expand All @@ -507,8 +508,9 @@ function MoneyRequestConfirmationList({
formatAmountOnBlur
prefixContainerStyle={[styles.pv0]}
inputStyle={[styles.optionRowAmountInput, amountWidth] as TextStyle[]}
containerStyle={[styles.textInputContainer]}
containerStyle={[styles.textInputContainer, amountWidth]}
touchableInputWrapperStyle={[styles.ml3]}
onFormatAmount={CurrencyUtils.convertToDisplayStringWithoutCurrency}
onAmountChange={(value: string) => onSplitShareChange(participantOption.accountID ?? 0, Number(value))}
maxLength={formattedTotalAmount.length}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/components/TextInputWithCurrencySymbol/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,6 @@ type TextInputWithCurrencySymbolProps = {

/** Hide the focus styles on TextInput */
hideFocusedState?: boolean;
} & Pick<BaseTextInputProps, 'autoFocus'>;
} & Pick<BaseTextInputProps, 'autoFocus' | 'autoGrow'>;

export default TextInputWithCurrencySymbolProps;
Loading