feat(TokenEnterAmount): add new flow to EnterAmount.tsx #6245
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -32,6 +32,7 @@ | |||||
| export const APPROX_SYMBOL = '≈' | ||||||
|
|
||||||
| const BORDER_RADIUS = 12 | ||||||
| export const FETCH_UPDATED_TRANSACTIONS_DEBOUNCE_TIME_MS = 250 | ||||||
|
|
||||||
| /** | ||||||
| * This function formats numbers in a "1234.5678" format into the correct format according to the | ||||||
|
|
@@ -48,6 +49,16 @@ | |||||
| .replaceAll('_', groupingSeparator) | ||||||
| } | ||||||
|
|
||||||
| export function unformatNumberForProcessing(value: string) { | ||||||
| const { decimalSeparator, groupingSeparator } = getNumberFormatSettings() | ||||||
| return value.replaceAll(groupingSeparator, '').replaceAll(decimalSeparator, '.') | ||||||
| } | ||||||
|
|
||||||
| export function roundLocalNumber(value: BigNumber | null) { | ||||||
|
There was a problem hiding this comment.
Suggested change
There was a problem hiding this comment. side thought, no need to address now but i wonder if this will be handy to have as a general function that can be used in other places like the transaction feed.... There was a problem hiding this comment. @kathaypacific I think it definitely will be. Considering that it is currently only used in this file - there's no point in moving it to a general utility yet file but we should keep this in mind for when its gonna be useful. |
||||||
| if (!value) return '' | ||||||
| return value.isLessThan(0.01) ? value.toPrecision(1) : value.toFixed(2) | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * This function returns complete formatted value for a token (crypto) amount when it is being a | ||||||
| * converted value (in "ExchangeAmount" element). | ||||||
|
|
@@ -83,7 +94,7 @@ | |||||
| return `<${localCurrencySymbol}0${decimalSeparator}000001` | ||||||
| } | ||||||
|
|
||||||
| const roundedAmount = roundLocalNumber(bignum) | ||||||
| const formattedAmount = formatNumber(roundedAmount.toString()) | ||||||
| return `${localCurrencySymbol}${formattedAmount}` | ||||||
| } | ||||||
|
|
@@ -95,62 +106,53 @@ | |||||
| export function useEnterAmount(props: { | ||||||
| token: TokenBalance | ||||||
| inputRef: React.RefObject<RNTextInput> | ||||||
| onHandleAmountInputChange?(value: string): void | ||||||
| }) { | ||||||
| const { decimalSeparator, groupingSeparator } = getNumberFormatSettings() | ||||||
|
|
||||||
| /** | ||||||
| * This field is formatted for processing purpose. It is a lot easier to process a number formatted | ||||||
| * in a single format, rather than writing different logic for various combinations of decimal | ||||||
| * and grouping separators. The format is "1234.5678". | ||||||
| */ | ||||||
| const [amount, setAmount] = useState('') | ||||||
| const [amountType, setAmountType] = useState<AmountEnteredIn>('token') | ||||||
|
|
||||||
| // This should never be null, just adding a default to make TS happy | ||||||
| const localCurrencySymbol = useSelector(getLocalCurrencySymbol) ?? LocalCurrencySymbol.USD | ||||||
| const usdToLocalRate = useSelector(usdToLocalCurrencyRateSelector) | ||||||
|
|
||||||
| /** | ||||||
| * This field includes all the necessary processed derived state. It is recalculated once whenever | ||||||
| * the input amount changes. Please, add new processed/calculated values to this variable. | ||||||
| * | ||||||
| * This field consists of two values: token and local. Both values represent calculated values for the | ||||||
| * corresponding amount type. Whenever we change token amount - we need to recalculate both token and | ||||||
| * local amounts. Each object consists of: | ||||||
| * - `bignum` - this is a BigNumber representation of the "amount" field. Necessary for easier | ||||||
| * condition checks and various processing things. | ||||||
| * - `displayAmount` - this is a read-only component-friendly value that contains all of the necessary | ||||||
| * formatting, including: grouping, decimals, token symbol/fiat sign, small amounts format. This | ||||||
| * value is only necessary to be passed to TokenEnterAmount component fields as: | ||||||
| * - `token.displayAmount` -> `tokenDisplayAmount` | ||||||
| * - `local.displayAmount` -> `localDisplayAmount` | ||||||
| */ | ||||||
| const processedAmounts = useMemo(() => { | ||||||
| if (amountType === 'token') { | ||||||
| const parsedTokenAmount = amount === '' ? null : parseInputAmount(amount) | ||||||
|
|
||||||
| const tokenToLocal = convertTokenToLocalAmount({ | ||||||
| tokenAmount: parsedTokenAmount, | ||||||
| tokenInfo: props.token, | ||||||
| usdToLocalRate, | ||||||
| }) | ||||||
|
|
||||||
| return { | ||||||
| token: { | ||||||
| bignum: parsedTokenAmount, | ||||||
| displayAmount: getDisplayTokenAmount(parsedTokenAmount, props.token), | ||||||
| }, | ||||||
| local: { | ||||||
| bignum: tokenToLocal, | ||||||
| displayAmount: getDisplayLocalAmount(tokenToLocal, localCurrencySymbol), | ||||||
| }, | ||||||
|
|
@@ -160,7 +162,7 @@ | |||||
| /** | ||||||
| * At this point, we can be sure that we are processing local (fiat) input. | ||||||
| */ | ||||||
| const parsedLocalAmount = amount === '' ? null : parseInputAmount(amount) | ||||||
|
|
||||||
| const localToToken = convertLocalToTokenAmount({ | ||||||
| localAmount: parsedLocalAmount, | ||||||
|
|
@@ -177,66 +179,94 @@ | |||||
| : '' | ||||||
|
|
||||||
| const parsedTokenAmount = parseInputAmount(convertedLocalToToken, decimalSeparator) | ||||||
| const balanceInLocal = convertTokenToLocalAmount({ | ||||||
| tokenAmount: props.token.balance, | ||||||
| tokenInfo: props.token, | ||||||
| usdToLocalRate, | ||||||
| })?.decimalPlaces(2) | ||||||
|
|
||||||
| const isMaxLocalAmount = | ||||||
| parsedLocalAmount && balanceInLocal && balanceInLocal.eq(parsedLocalAmount) | ||||||
| const tokenAmount = isMaxLocalAmount ? props.token.balance : parsedTokenAmount | ||||||
|
|
||||||
| return { | ||||||
| token: { | ||||||
| bignum: tokenAmount, | ||||||
| displayAmount: getDisplayTokenAmount(tokenAmount, props.token), | ||||||
| }, | ||||||
| local: { | ||||||
| bignum: parsedLocalAmount, | ||||||
| displayAmount: getDisplayLocalAmount(parsedLocalAmount, localCurrencySymbol), | ||||||
| }, | ||||||
| } | ||||||
| }, [amount, amountType, localCurrencySymbol]) | ||||||
|
|
||||||
| function handleToggleAmountType() { | ||||||
| const newAmountType = amountType === 'local' ? 'token' : 'local' | ||||||
|
There was a problem hiding this comment. this is an existing issue and i haven't been able to think of a better approach that doesn't introduce a bunch of issues, but setting state using the existing value of a state variable like this is unreliable because of the batched state updates 🙈 probably low risk for this use case, unless someone is toggling the amounts many times really quickly. maybe this component would have been a good one for There was a problem hiding this comment. @kathaypacific considering that the state setting is indeed batched since React 18 and the whole scope of And according to React docs: So even if user clicks multiple time - we will be sure that both states will always be in sync with one another as it will only execute everything in scope as a single update. P.S. I do remember that they also whether had this stated at the react docs previously or it was just some Abramov's post but there was a statement that using state setter without callback is completely fine if the use-case is simple enough and it will benefit from batching (which I believe applies in this case). |
||||||
| setAmountType(newAmountType) | ||||||
| setAmount( | ||||||
| newAmountType === 'local' | ||||||
| ? processedAmounts.local.bignum?.toFixed(2) || '' | ||||||
| : processedAmounts.token.bignum?.decimalPlaces(6).toString() || '' | ||||||
| ) | ||||||
| props.inputRef.current?.blur() | ||||||
| } | ||||||
|
|
||||||
| function handleAmountInputChange(val: string) { | ||||||
| let value = val.replaceAll(groupingSeparator, '').replaceAll(decimalSeparator, '.') | ||||||
| value = value.startsWith('.') ? `0${value}` : value | ||||||
|
|
||||||
| if (!value) { | ||||||
| setAmount('') | ||||||
| props.onHandleAmountInputChange?.('') | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| // only allow numbers, one decimal separator and 2 decimals | ||||||
| const localAmountRegex = new RegExp(`^(\\d+([.])?\\d{0,2}|[.]\\d{0,2}|[.])$`) | ||||||
| // only allow numbers, one decimal separator and amount of decimals equal to token.decimals | ||||||
| const tokenAmountRegex = new RegExp( | ||||||
| `^(?:\\d+[.]?\\d{0,${props.token.decimals}}|[.]\\d{0,${props.token.decimals}}|[.])$` | ||||||
| ) | ||||||
|
|
||||||
| if ( | ||||||
| (amountType === 'token' && value.match(tokenAmountRegex)) || | ||||||
| (amountType === 'local' && value.match(localAmountRegex)) | ||||||
| ) { | ||||||
| setAmount(value) | ||||||
| props.onHandleAmountInputChange?.(value) | ||||||
| return | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| function handlePercentage(percentage: number) { | ||||||
| if (percentage <= 0 || percentage > 1) return | ||||||
|
|
||||||
| const percentageAmount = props.token.balance.multipliedBy(percentage) | ||||||
| const newAmount = | ||||||
| amountType === 'token' | ||||||
| ? percentageAmount.decimalPlaces(6).toString() | ||||||
| : roundLocalNumber( | ||||||
| convertTokenToLocalAmount({ | ||||||
| tokenAmount: percentageAmount, | ||||||
| tokenInfo: props.token, | ||||||
| usdToLocalRate, | ||||||
| }) | ||||||
| ) | ||||||
|
|
||||||
| if (!newAmount) return | ||||||
|
|
||||||
| setAmount(newAmount) | ||||||
| } | ||||||
|
|
||||||
| return { | ||||||
| amount, | ||||||
| amountType, | ||||||
| processedAmounts, | ||||||
| setAmount, | ||||||
| handleToggleAmountType, | ||||||
| handleAmountInputChange, | ||||||
| handlePercentage, | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -276,9 +306,14 @@ | |||||
| const [startPosition, setStartPosition] = useState<number | undefined>(0) | ||||||
| // this should never be null, just adding a default to make TS happy | ||||||
| const localCurrencySymbol = useSelector(getLocalCurrencySymbol) ?? LocalCurrencySymbol.USD | ||||||
|
|
||||||
| const placeholder = useMemo(() => { | ||||||
| const { decimalSeparator } = getNumberFormatSettings() | ||||||
| return { | ||||||
| token: new BigNumber(0).toFormat(2), | ||||||
| local: `${localCurrencySymbol}${new BigNumber(0).toFormat(2).replaceAll('.', decimalSeparator)}`, | ||||||
| } | ||||||
| }, [localCurrencySymbol]) | ||||||
|
|
||||||
| const formattedInputValue = useMemo(() => { | ||||||
| const formattedNumber = formatNumber(inputValue) | ||||||
|
|
@@ -358,7 +393,7 @@ | |||||
| }} | ||||||
| value={formattedInputValue} | ||||||
| placeholderTextColor={Colors.gray3} | ||||||
| placeholder={amountType === 'token' ? placeholder.token : placeholder.local} | ||||||
| keyboardType="decimal-pad" | ||||||
| // Work around for RN issue with Samsung keyboards | ||||||
| // https://github.com/facebook/react-native/issues/22005 | ||||||
|
|
@@ -406,12 +441,12 @@ | |||||
|
|
||||||
| <Text | ||||||
| numberOfLines={1} | ||||||
| style={[styles.secondaryAmountText, { flexShrink: 0, maxWidth: '45%' }]} | ||||||
| testID={`${testID}/ExchangeAmount`} | ||||||
| > | ||||||
| {amountType === 'token' | ||||||
| ? `${APPROX_SYMBOL} ${localAmount ? localAmount : placeholder.local}` | ||||||
| : `${APPROX_SYMBOL} ${tokenAmount ? tokenAmount : placeholder.token}`} | ||||||
| </Text> | ||||||
| </> | ||||||
| ) : ( | ||||||
|
|
||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or
parseNumberToStandardFormatorapplyUSNumberFormatting?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kathaypacific hmm, I'm still unsure on this one. It is only used in a single place so far to actually un-format already formatted number that comes from the input and format it for the processing purposes. I think it makes a more clear distinction for the specific purpose of the function:
I've only made is a separate function to simplify description as otherwise I'd need to write a comment explaining what it does and why, which feels more concise in a form of a clear function name.
In case of
formatNumberToStandard/parseNumberToStandardFormat- we need to define what we mean by "standard". And in case ofapplyUSNumberFormatting- we need to understand why we specifically useUSnumber formatting (even though I'm not sure it's US specific as up to this point I thought this was a general format that everyone uses and was always using it myself 😄)At least, that's my train of though. What do you think about it?