From 9282843a2246929d27ada42894f7dbb736b976a0 Mon Sep 17 00:00:00 2001 From: Christophe Deveaux Date: Fri, 30 Aug 2024 16:16:16 +0200 Subject: [PATCH] fix: improve amount input performance (#1869) --- packages/arb-token-bridge-ui/package.json | 1 - .../TransferPanelMain/SourceNetworkBox.tsx | 40 +++-- .../TransferPanel/TransferPanelMainInput.tsx | 137 +++++++++++++----- .../src/hooks/useArbQueryParams.tsx | 5 +- .../tests/e2e/cypress.d.ts | 1 - .../tests/e2e/specs/importToken.cy.ts | 16 +- .../tests/support/commands.ts | 30 +--- .../tests/support/common.ts | 15 +- .../tests/support/index.ts | 5 - yarn.lock | 12 -- 10 files changed, 150 insertions(+), 112 deletions(-) diff --git a/packages/arb-token-bridge-ui/package.json b/packages/arb-token-bridge-ui/package.json index c4c0e8636d..b174e5b584 100644 --- a/packages/arb-token-bridge-ui/package.json +++ b/packages/arb-token-bridge-ui/package.json @@ -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", diff --git a/packages/arb-token-bridge-ui/src/components/TransferPanel/TransferPanelMain/SourceNetworkBox.tsx b/packages/arb-token-bridge-ui/src/components/TransferPanel/TransferPanelMain/SourceNetworkBox.tsx index 237c001d13..cf0ba5a22a 100644 --- a/packages/arb-token-bridge-ui/src/components/TransferPanel/TransferPanelMain/SourceNetworkBox.tsx +++ b/packages/arb-token-bridge-ui/src/components/TransferPanel/TransferPanelMain/SourceNetworkBox.tsx @@ -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, @@ -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, @@ -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 @@ -95,6 +95,25 @@ export function SourceNetworkBox({ } }, [maxAmount2, setAmount2]) + const handleAmountChange: ChangeEventHandler = useCallback( + e => setAmount(e.target.value), + [setAmount] + ) + const handleAmount2Change: ChangeEventHandler = useCallback( + e => { + setAmount2(e.target.value) + }, + [setAmount2] + ) + + const tokenButtonOptionsAmount2 = useMemo( + () => ({ + symbol: nativeCurrency.symbol, + disabled: true + }), + [nativeCurrency.symbol] + ) + return ( <> @@ -159,7 +178,10 @@ 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 && ( @@ -167,11 +189,11 @@ export function SourceNetworkBox({ 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} /> )} diff --git a/packages/arb-token-bridge-ui/src/components/TransferPanel/TransferPanelMainInput.tsx b/packages/arb-token-bridge-ui/src/components/TransferPanel/TransferPanelMainInput.tsx index e05d7e4919..ad3efc5e98 100644 --- a/packages/arb-token-bridge-ui/src/components/TransferPanel/TransferPanelMainInput.tsx +++ b/packages/arb-token-bridge-ui/src/components/TransferPanel/TransferPanelMainInput.tsx @@ -1,3 +1,9 @@ +import React, { + ChangeEventHandler, + useCallback, + useEffect, + useState +} from 'react' import { twMerge } from 'tailwind-merge' import { useMemo } from 'react' @@ -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) { const { className = '', ...rest } = props @@ -68,22 +76,21 @@ function MaxButton(props: React.ButtonHTMLAttributes) { ) } -function TransferPanelInputField( - props: React.InputHTMLAttributes -) { - const { value = '', ...rest } = props +const TransferPanelInputField = React.memo( + (props: React.InputHTMLAttributes) => { + return ( + + ) + } +) - return ( - - ) -} +TransferPanelInputField.displayName = 'TransferPanelInputField' function ErrorMessage({ errorMessage @@ -140,34 +147,90 @@ export type TransferPanelMainInputProps = maxButtonOnClick: React.ButtonHTMLAttributes['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 ( - <> -
- + /** + * 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 = + useCallback( + e => { + maxButtonOnClick?.(e) + if (maxAmount) { + setLocalValue(maxAmount) + } + }, + [maxAmount, maxButtonOnClick] + ) + + const handleInputChange: ChangeEventHandler = useCallback( + e => { + setLocalValue( + sanitizeAmountQueryParam( + truncateExtraDecimals(e.target.value, decimals) + ) + ) + onChange?.(e) + }, + [decimals, onChange] + ) + + return ( + <>
- - + +
+ + +
-
- - - ) -} + + + ) + } +) + +TransferPanelMainInput.displayName = 'TransferPanelMainInput' diff --git a/packages/arb-token-bridge-ui/src/hooks/useArbQueryParams.tsx b/packages/arb-token-bridge-ui/src/hooks/useArbQueryParams.tsx index 18e6e38738..e7590b23e7 100644 --- a/packages/arb-token-bridge-ui/src/hooks/useArbQueryParams.tsx +++ b/packages/arb-token-bridge-ui/src/hooks/useArbQueryParams.tsx @@ -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 @@ -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} diff --git a/packages/arb-token-bridge-ui/tests/e2e/cypress.d.ts b/packages/arb-token-bridge-ui/tests/e2e/cypress.d.ts index 8669de0cad..1e1761833c 100644 --- a/packages/arb-token-bridge-ui/tests/e2e/cypress.d.ts +++ b/packages/arb-token-bridge-ui/tests/e2e/cypress.d.ts @@ -47,7 +47,6 @@ declare global { resetCctpAllowance: typeof resetCctpAllowance fundUserUsdcTestnet: typeof fundUserUsdcTestnet fundUserWalletEth: typeof fundUserWalletEth - typeRecursively(text: string): Chainable> searchAndSelectToken({ tokenName, tokenAddress diff --git a/packages/arb-token-bridge-ui/tests/e2e/specs/importToken.cy.ts b/packages/arb-token-bridge-ui/tests/e2e/specs/importToken.cy.ts index 8ccf4194ee..3946254dbf 100644 --- a/packages/arb-token-bridge-ui/tests/e2e/specs/importToken.cy.ts +++ b/packages/arb-token-bridge-ui/tests/e2e/specs/importToken.cy.ts @@ -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() @@ -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() @@ -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' }) @@ -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') }) diff --git a/packages/arb-token-bridge-ui/tests/support/commands.ts b/packages/arb-token-bridge-ui/tests/support/commands.ts index 4d4a7d0666..425398add7 100644 --- a/packages/arb-token-bridge-ui/tests/support/commands.ts +++ b/packages/arb-token-bridge-ui/tests/support/commands.ts @@ -8,7 +8,6 @@ // *********************************************** import '@testing-library/cypress/add-commands' -import { recurse } from 'cypress-recurse' import { NetworkType, NetworkName, @@ -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() @@ -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 @@ -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> { - return cy - .findByPlaceholderText(/enter amount/i) - .typeRecursively(String(amount)) + return cy.findByPlaceholderText(/enter amount/i).type(String(amount)) } export function findSourceChainButton( diff --git a/packages/arb-token-bridge-ui/tests/support/common.ts b/packages/arb-token-bridge-ui/tests/support/common.ts index 9228c6e1dc..d17eab02db 100644 --- a/packages/arb-token-bridge-ui/tests/support/common.ts +++ b/packages/arb-token-bridge-ui/tests/support/common.ts @@ -101,16 +101,15 @@ export const importTokenThroughUI = (address: string) => { cy.findSelectTokenButton('ETH').click() // open the Select Token popup + cy.findByPlaceholderText(/Search by token name/i) + .should('be.visible') + .type(address) + + // Click on the Add new token button return cy - .findByPlaceholderText(/Search by token name/i) + .findByRole('button', { name: 'Add New Token' }) .should('be.visible') - .typeRecursively(address) - .then(() => { - // Click on the Add new token button - cy.findByRole('button', { name: 'Add New Token' }) - .should('be.visible') - .click() - }) + .click() } export async function getInitialETHBalance( diff --git a/packages/arb-token-bridge-ui/tests/support/index.ts b/packages/arb-token-bridge-ui/tests/support/index.ts index c446f45741..7d992ca6e9 100644 --- a/packages/arb-token-bridge-ui/tests/support/index.ts +++ b/packages/arb-token-bridge-ui/tests/support/index.ts @@ -8,11 +8,6 @@ import { getL2TestnetNetworkConfig } from './common' -Cypress.Keyboard.defaults({ - // tests are flaky in CI with low keystroke delay - keystrokeDelay: 150 -}) - logCollector({ collectTypes: [ 'cy:command', diff --git a/yarn.lock b/yarn.lock index 26035c50d7..13d50115e1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5005,13 +5005,6 @@ csstype@^3.0.2, csstype@^3.0.6, csstype@^3.0.7: resolved "https://registry.yarnpkg.com/csstype/-/csstype-3.1.2.tgz#1d4bf9d572f11c14031f0436e1c10bc1f571f50b" integrity sha512-I7K1Uu0MBPzaFKg4nI5Q7Vs2t+3gWWW648spaF+Rg7pI9ds18Ugn+lvg4SHczUdKlHI5LWBXyqfS8+DufyBsgQ== -cypress-recurse@^1.35.2: - version "1.35.2" - resolved "https://registry.yarnpkg.com/cypress-recurse/-/cypress-recurse-1.35.2.tgz#3f49db173beb117196cad82c5484e638e2dc3478" - integrity sha512-G6HfxP90xa7phw8oeOX4uabxcI9gE1ktkKHShcA3nCByrkMLs56+GIJVn0A+ws1tI0PGRKBz6+V9DHS5WnZX4A== - dependencies: - humanize-duration "^3.27.3" - cypress-terminal-report@^5.3.10: version "5.3.10" resolved "https://registry.yarnpkg.com/cypress-terminal-report/-/cypress-terminal-report-5.3.10.tgz#62d59cf9ecc9a9a9befe629d73824eb65240738a" @@ -7668,11 +7661,6 @@ human-signals@^5.0.0: resolved "https://registry.yarnpkg.com/human-signals/-/human-signals-5.0.0.tgz#42665a284f9ae0dade3ba41ebc37eb4b852f3a28" integrity sha512-AXcZb6vzzrFAUE61HnN4mpLqd/cSIwNQjtNWR0euPm6y0iqx3G4gOXaIDdtdDwZmhwe82LA6+zinmW4UBWVePQ== -humanize-duration@^3.27.3: - version "3.28.0" - resolved "https://registry.yarnpkg.com/humanize-duration/-/humanize-duration-3.28.0.tgz#f79770c0bec34d3bfd4899338cc40643bc04df72" - integrity sha512-jMAxraOOmHuPbffLVDKkEKi/NeG8dMqP8lGRd6Tbf7JgAeG33jjgPWDbXXU7ypCI0o+oNKJFgbSB9FKVdWNI2A== - humanize-ms@^1.2.1: version "1.2.1" resolved "https://registry.yarnpkg.com/humanize-ms/-/humanize-ms-1.2.1.tgz#c46e3159a293f6b896da29316d8b6fe8bb79bbed"