Skip to content

Commit

Permalink
fix: improve amount input performance (#1869)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrstph-dvx authored Aug 30, 2024
1 parent 3cf03cf commit 9282843
Show file tree
Hide file tree
Showing 10 changed files with 150 additions and 112 deletions.
1 change: 0 additions & 1 deletion packages/arb-token-bridge-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@
"@typescript-eslint/eslint-plugin": "^5.57.0",
"@typescript-eslint/parser": "^6.13.2",
"autoprefixer": "^10.4.13",
"cypress-recurse": "^1.35.2",
"cypress-terminal-report": "^5.3.10",
"cypress-wait-until": "^1.7.2",
"env-cmd": "^10.1.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { useCallback, useEffect } from 'react'
import { ChangeEventHandler, useCallback, useEffect, useMemo } from 'react'

import { isTeleport } from '@/token-bridge-sdk/teleport'
import { getNetworkName } from '../../../util/networks'
import {
NetworkButton,
Expand Down Expand Up @@ -39,6 +38,7 @@ import { useSetInputAmount } from '../../../hooks/TransferPanel/useSetInputAmoun
import { useDialog } from '../../common/Dialog'
import { useTransferReadiness } from '../useTransferReadiness'
import { useIsBatchTransferSupported } from '../../../hooks/TransferPanel/useIsBatchTransferSupported'
import { useSelectedTokenDecimals } from '../../../hooks/TransferPanel/useSelectedTokenDecimals'

export function SourceNetworkBox({
customFeeTokenBalances,
Expand All @@ -64,7 +64,7 @@ export function SourceNetworkBox({
const [sourceNetworkSelectionDialogProps, openSourceNetworkSelectionDialog] =
useDialog()
const isBatchTransferSupported = useIsBatchTransferSupported()

const decimals = useSelectedTokenDecimals()
const { errorMessages } = useTransferReadiness()

const isMaxAmount = amount === AmountQueryParamEnum.MAX
Expand Down Expand Up @@ -95,6 +95,25 @@ export function SourceNetworkBox({
}
}, [maxAmount2, setAmount2])

const handleAmountChange: ChangeEventHandler<HTMLInputElement> = useCallback(
e => setAmount(e.target.value),
[setAmount]
)
const handleAmount2Change: ChangeEventHandler<HTMLInputElement> = useCallback(
e => {
setAmount2(e.target.value)
},
[setAmount2]
)

const tokenButtonOptionsAmount2 = useMemo(
() => ({
symbol: nativeCurrency.symbol,
disabled: true
}),
[nativeCurrency.symbol]
)

return (
<>
<NetworkContainer bgLogoHeight={138} network={networks.sourceChain}>
Expand Down Expand Up @@ -159,19 +178,22 @@ export function SourceNetworkBox({
maxButtonOnClick={maxButtonOnClick}
errorMessage={errorMessages?.inputAmount1}
value={isMaxAmount ? '' : amount}
onChange={e => setAmount(e.target.value)}
onChange={handleAmountChange}
maxAmount={maxAmount}
isMaxAmount={isMaxAmount}
decimals={decimals}
/>

{isBatchTransferSupported && (
<TransferPanelMainInput
maxButtonOnClick={amount2MaxButtonOnClick}
errorMessage={errorMessages?.inputAmount2}
value={amount2}
onChange={e => setAmount2(e.target.value)}
tokenButtonOptions={{
symbol: nativeCurrency.symbol,
disabled: true
}}
onChange={handleAmount2Change}
tokenButtonOptions={tokenButtonOptionsAmount2}
maxAmount={maxAmount2}
isMaxAmount={isMaxAmount2}
decimals={nativeCurrency.decimals}
/>
)}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
import React, {
ChangeEventHandler,
useCallback,
useEffect,
useState
} from 'react'
import { twMerge } from 'tailwind-merge'
import { useMemo } from 'react'

Expand All @@ -10,6 +16,8 @@ import { useBalances } from '../../hooks/useBalances'
import { TransferReadinessRichErrorMessage } from './useTransferReadinessUtils'
import { ExternalLink } from '../common/ExternalLink'
import { useTransferDisabledDialogStore } from './TransferDisabledDialog'
import { sanitizeAmountQueryParam } from '../../hooks/useArbQueryParams'
import { truncateExtraDecimals } from '../../util/NumberUtils'

function MaxButton(props: React.ButtonHTMLAttributes<HTMLButtonElement>) {
const { className = '', ...rest } = props
Expand Down Expand Up @@ -68,22 +76,21 @@ function MaxButton(props: React.ButtonHTMLAttributes<HTMLButtonElement>) {
)
}

function TransferPanelInputField(
props: React.InputHTMLAttributes<HTMLInputElement>
) {
const { value = '', ...rest } = props
const TransferPanelInputField = React.memo(
(props: React.InputHTMLAttributes<HTMLInputElement>) => {
return (
<input
type="text"
inputMode="decimal"
placeholder="Enter amount"
className="h-full w-full bg-transparent px-3 text-xl font-light placeholder:text-gray-dark sm:text-3xl"
{...props}
/>
)
}
)

return (
<input
type="text"
inputMode="decimal"
placeholder="Enter amount"
className="h-full w-full bg-transparent px-3 text-xl font-light placeholder:text-gray-dark sm:text-3xl"
value={value}
{...rest}
/>
)
}
TransferPanelInputField.displayName = 'TransferPanelInputField'

function ErrorMessage({
errorMessage
Expand Down Expand Up @@ -140,34 +147,90 @@ export type TransferPanelMainInputProps =
maxButtonOnClick: React.ButtonHTMLAttributes<HTMLButtonElement>['onClick']
value: string
tokenButtonOptions?: TokenButtonOptions
maxAmount: string | undefined
isMaxAmount: boolean
decimals: number
}

export function TransferPanelMainInput(props: TransferPanelMainInputProps) {
const { errorMessage, maxButtonOnClick, tokenButtonOptions, ...rest } = props
export const TransferPanelMainInput = React.memo(
({
errorMessage,
maxButtonOnClick,
tokenButtonOptions,
onChange,
maxAmount,
value,
isMaxAmount,
decimals,
...rest
}: TransferPanelMainInputProps) => {
const [localValue, setLocalValue] = useState(value)

useEffect(() => {
if (!isMaxAmount || !maxAmount) {
return
}

return (
<>
<div
className={twMerge(
'flex flex-row rounded border bg-black/40 shadow-2',
errorMessage
? 'border-brick text-brick'
: 'border-white/30 text-white'
)}
>
<TokenButton options={tokenButtonOptions} />
/**
* On first render, maxAmount is not defined, once we receive max amount value, we set the localValue
* If user types anything before we receive the amount, isMaxAmount is set to false in the parent
*/
setLocalValue(maxAmount)
}, [isMaxAmount, maxAmount])

const handleMaxButtonClick: React.MouseEventHandler<HTMLButtonElement> =
useCallback(
e => {
maxButtonOnClick?.(e)
if (maxAmount) {
setLocalValue(maxAmount)
}
},
[maxAmount, maxButtonOnClick]
)

const handleInputChange: ChangeEventHandler<HTMLInputElement> = useCallback(
e => {
setLocalValue(
sanitizeAmountQueryParam(
truncateExtraDecimals(e.target.value, decimals)
)
)
onChange?.(e)
},
[decimals, onChange]
)

return (
<>
<div
className={twMerge(
'flex grow flex-row items-center justify-center border-l',
errorMessage ? 'border-brick' : 'border-white/30'
'flex flex-row rounded border bg-black/40 shadow-2',
errorMessage
? 'border-brick text-brick'
: 'border-white/30 text-white'
)}
>
<TransferPanelInputField {...rest} />
<MaxButton onClick={maxButtonOnClick} />
<TokenButton options={tokenButtonOptions} />
<div
className={twMerge(
'flex grow flex-row items-center justify-center border-l',
errorMessage ? 'border-brick' : 'border-white/30'
)}
>
<TransferPanelInputField
{...rest}
value={localValue}
onChange={handleInputChange}
/>
<MaxButton onClick={handleMaxButtonClick} />
</div>
</div>
</div>

<ErrorMessage errorMessage={errorMessage} />
</>
)
}
<ErrorMessage errorMessage={errorMessage} />
</>
)
}
)

TransferPanelMainInput.displayName = 'TransferPanelMainInput'
5 changes: 3 additions & 2 deletions packages/arb-token-bridge-ui/src/hooks/useArbQueryParams.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const isMax = (amount: string | undefined) =>
* @param amount - transfer amount value from the input field or from the URL
* @returns sanitised value
*/
const sanitizeAmountQueryParam = (amount: string) => {
export const sanitizeAmountQueryParam = (amount: string) => {
// no need to process empty string
if (amount.length === 0) {
return amount
Expand Down Expand Up @@ -175,7 +175,8 @@ export function ArbQueryParamProvider({
searchStringToObject: queryString.parse,
objectToSearchString: queryString.stringify,
updateType: 'replaceIn', // replace just a single parameter when updating query-state, leaving the rest as is
removeDefaultsFromUrl: true
removeDefaultsFromUrl: true,
enableBatching: true
}}
>
{children}
Expand Down
1 change: 0 additions & 1 deletion packages/arb-token-bridge-ui/tests/e2e/cypress.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ declare global {
resetCctpAllowance: typeof resetCctpAllowance
fundUserUsdcTestnet: typeof fundUserUsdcTestnet
fundUserWalletEth: typeof fundUserWalletEth
typeRecursively(text: string): Chainable<JQuery<HTMLElement>>
searchAndSelectToken({
tokenName,
tokenAddress
Expand Down
16 changes: 6 additions & 10 deletions packages/arb-token-bridge-ui/tests/e2e/specs/importToken.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ describe('Import token', () => {
// Select the UNI token
cy.findByPlaceholderText(/Search by token name/i)
.should('be.visible')
.typeRecursively('UNI')
.type('UNI')

// flaky test can load data too slowly here
cy.findByText('Uniswap', { timeout: 5_000 }).click()
Expand All @@ -133,10 +133,7 @@ describe('Import token', () => {

context('Add button is grayed', () => {
it('should disable Add button if address is too long/short', () => {
const moveToEnd = ERC20TokenAddressL1.substring(
0,
ERC20TokenAddressL1.length - 1
)
const addressWithoutLastChar = ERC20TokenAddressL1.slice(0, -1) // Remove the last character

cy.login({ networkType: 'parentChain' })
cy.findSelectTokenButton('ETH').click()
Expand All @@ -145,7 +142,7 @@ describe('Import token', () => {
cy.findByPlaceholderText(/Search by token name/i)
.as('searchInput')
.should('be.visible')
.typeRecursively(ERC20TokenAddressL1.slice(0, -1))
.type(addressWithoutLastChar)

// Add button should be disabled
cy.findByRole('button', { name: 'Add New Token' })
Expand All @@ -154,14 +151,13 @@ describe('Import token', () => {
.as('addButton')

// Add last character
cy.get('@searchInput').typeRecursively(
`${moveToEnd}${ERC20TokenAddressL1.slice(-1)}`
)
cy.get('@searchInput').type(ERC20TokenAddressL1.slice(-1))

// Add button should be enabled
cy.get('@addButton').should('be.enabled')

// Add one more character to make the address invalid
cy.get('@searchInput').typeRecursively(`${moveToEnd}x`)
cy.get('@searchInput').type('x')
// Add button should be disabled
cy.get('@addButton').should('be.disabled')
})
Expand Down
30 changes: 3 additions & 27 deletions packages/arb-token-bridge-ui/tests/support/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
// ***********************************************

import '@testing-library/cypress/add-commands'
import { recurse } from 'cypress-recurse'
import {
NetworkType,
NetworkName,
Expand Down Expand Up @@ -80,27 +79,6 @@ export function login({
})
}

Cypress.Commands.add(
'typeRecursively',
{ prevSubject: true },
(subject, text: string) => {
recurse(
// the commands to repeat, and they yield the input element
() => cy.wrap(subject).clear().type(text),
// the predicate takes the output of the above commands
// and returns a boolean. If it returns true, the recursion stops
$input => $input.val() === text,
{
log: false,
timeout: 180_000
}
)
// the recursion yields whatever the command function yields
// and we can confirm that the text was entered correctly
.should('have.value', text)
}
)

// once all assertions are run, before test exit, make sure web-app is reset to original
export const logout = () => {
cy.disconnectMetamaskWalletFromAllDapps()
Expand Down Expand Up @@ -242,7 +220,7 @@ export const searchAndSelectToken = ({

// open the Select Token popup
cy.findByPlaceholderText(/Search by token name/i)
.typeRecursively(tokenAddress)
.type(tokenAddress)
.should('be.visible')

// Click on the Add new token button
Expand All @@ -268,15 +246,13 @@ export const fillCustomDestinationAddress = () => {

cy.findByPlaceholderText(Cypress.env('ADDRESS'))
.should('be.visible')
.typeRecursively(Cypress.env('CUSTOM_DESTINATION_ADDRESS'))
.type(Cypress.env('CUSTOM_DESTINATION_ADDRESS'))
}

export function typeAmount(
amount: string | number
): Cypress.Chainable<JQuery<HTMLElement>> {
return cy
.findByPlaceholderText(/enter amount/i)
.typeRecursively(String(amount))
return cy.findByPlaceholderText(/enter amount/i).type(String(amount))
}

export function findSourceChainButton(
Expand Down
Loading

0 comments on commit 9282843

Please sign in to comment.