From 7a92e2211186142694c76b7981b7aecde1b4fb5e Mon Sep 17 00:00:00 2001 From: Matthew Epps Date: Wed, 1 Dec 2021 09:25:09 -0700 Subject: [PATCH] Swaps optimizations (#12842) * Trigger Build * Trigger Build * Move swaps index variables to redux * all optimizations so far * Add better equality checks for selectors in swaps index and build quote * Clean up PR, remove extra code and logs * Clean up lavamoat file * Fixes for optimizations * Update tests and test snapshots * Remove unnecessary tests * Remove unnecessary console log * Trigger Build * Trigger Build * Add delay to account for remote call made by trezor keyring Co-authored-by: Dan Miller --- ui/ducks/swaps/swaps.js | 35 ++++++ ui/ducks/swaps/swaps.test.js | 10 +- ui/hooks/useGasFeeEstimates.js | 10 +- ui/hooks/useTokenFiatAmount.js | 7 +- ui/hooks/useTokenTracker.js | 4 +- ui/hooks/useTokensToSearch.js | 8 +- ui/pages/routes/routes.component.js | 52 +++------ ui/pages/routes/routes.container.js | 10 +- ui/pages/swaps/awaiting-swap/awaiting-swap.js | 10 +- .../__snapshots__/build-quote.test.js.snap | 15 +-- ui/pages/swaps/build-quote/build-quote.js | 100 +++++++++++------- .../swaps/build-quote/build-quote.test.js | 29 ----- ui/pages/swaps/index.js | 54 ++-------- 13 files changed, 161 insertions(+), 183 deletions(-) diff --git a/ui/ducks/swaps/swaps.js b/ui/ducks/swaps/swaps.js index fa128bb73..8923574fb 100644 --- a/ui/ducks/swaps/swaps.js +++ b/ui/ducks/swaps/swaps.js @@ -85,6 +85,10 @@ const initialState = { balanceError: false, fetchingQuotes: false, fromToken: null, + fromTokenInputValue: '', + fromTokenError: null, + isFeatureFlagLoaded: false, + maxSlippage: 3, quotesFetchStartTime: null, reviewSwapClickedTimestamp: null, topAssets: {}, @@ -128,6 +132,18 @@ const slice = createSlice({ setFromToken: (state, action) => { state.fromToken = action.payload; }, + setFromTokenInputValue: (state, action) => { + state.fromTokenInputValue = action.payload; + }, + setFromTokenError: (state, action) => { + state.fromTokenError = action.payload; + }, + setIsFeatureFlagLoaded: (state, action) => { + state.isFeatureFlagLoaded = action.payload; + }, + setMaxSlippage: (state, action) => { + state.maxSlippage = action.payload; + }, setQuotesFetchStartTime: (state, action) => { state.quotesFetchStartTime = action.payload; }, @@ -178,6 +194,16 @@ export const getBalanceError = (state) => state.swaps.balanceError; export const getFromToken = (state) => state.swaps.fromToken; +export const getFromTokenError = (state) => state.swaps.fromTokenError; + +export const getFromTokenInputValue = (state) => + state.swaps.fromTokenInputValue; + +export const getIsFeatureFlagLoaded = (state) => + state.swaps.isFeatureFlagLoaded; + +export const getMaxSlippage = (state) => state.swaps.maxSlippage; + export const getTopAssets = (state) => state.swaps.topAssets; export const getToToken = (state) => state.swaps.toToken; @@ -329,6 +355,10 @@ const { setBalanceError, setFetchingQuotes, setFromToken, + setFromTokenError, + setFromTokenInputValue, + setIsFeatureFlagLoaded, + setMaxSlippage, setQuotesFetchStartTime, setReviewSwapClickedTimestamp, setTopAssets, @@ -345,6 +375,10 @@ export { setBalanceError, setFetchingQuotes, setFromToken as setSwapsFromToken, + setFromTokenError, + setFromTokenInputValue, + setIsFeatureFlagLoaded, + setMaxSlippage, setQuotesFetchStartTime as setSwapQuotesFetchStartTime, setReviewSwapClickedTimestamp, setTopAssets, @@ -411,6 +445,7 @@ export const fetchSwapsLiveness = () => { log.error('Failed to fetch Swaps liveness, defaulting to false.', error); } await dispatch(setSwapsLiveness(swapsLivenessForNetwork)); + dispatch(setIsFeatureFlagLoaded(true)); return swapsLivenessForNetwork; }; }; diff --git a/ui/ducks/swaps/swaps.test.js b/ui/ducks/swaps/swaps.test.js index fd27f75e0..e766e484e 100644 --- a/ui/ducks/swaps/swaps.test.js +++ b/ui/ducks/swaps/swaps.test.js @@ -71,7 +71,7 @@ describe('Ducks - Swaps', () => { createGetState(), ); expect(featureFlagApiNock.isDone()).toBe(true); - expect(mockDispatch).toHaveBeenCalledTimes(1); + expect(mockDispatch).toHaveBeenCalledTimes(2); expect(setSwapsLiveness).toHaveBeenCalledWith(expectedSwapsLiveness); expect(swapsLiveness).toMatchObject(expectedSwapsLiveness); }); @@ -91,7 +91,7 @@ describe('Ducks - Swaps', () => { createGetState(), ); expect(featureFlagApiNock.isDone()).toBe(true); - expect(mockDispatch).toHaveBeenCalledTimes(1); + expect(mockDispatch).toHaveBeenCalledTimes(2); expect(setSwapsLiveness).toHaveBeenCalledWith(expectedSwapsLiveness); expect(swapsLiveness).toMatchObject(expectedSwapsLiveness); }); @@ -112,7 +112,7 @@ describe('Ducks - Swaps', () => { createGetState(), ); expect(featureFlagApiNock.isDone()).toBe(true); - expect(mockDispatch).toHaveBeenCalledTimes(1); + expect(mockDispatch).toHaveBeenCalledTimes(2); expect(setSwapsLiveness).toHaveBeenCalledWith(expectedSwapsLiveness); expect(swapsLiveness).toMatchObject(expectedSwapsLiveness); }); @@ -130,7 +130,7 @@ describe('Ducks - Swaps', () => { createGetState(), ); expect(featureFlagApiNock.isDone()).toBe(true); - expect(mockDispatch).toHaveBeenCalledTimes(1); + expect(mockDispatch).toHaveBeenCalledTimes(2); expect(setSwapsLiveness).toHaveBeenCalledWith(expectedSwapsLiveness); expect(swapsLiveness).toMatchObject(expectedSwapsLiveness); }); @@ -154,7 +154,7 @@ describe('Ducks - Swaps', () => { createGetState(), ); expect(featureFlagApiNock2.isDone()).toBe(false); // Second API call wasn't made, cache was used instead. - expect(mockDispatch).toHaveBeenCalledTimes(2); + expect(mockDispatch).toHaveBeenCalledTimes(4); expect(setSwapsLiveness).toHaveBeenCalledWith(expectedSwapsLiveness); expect(swapsLiveness).toMatchObject(expectedSwapsLiveness); }); diff --git a/ui/hooks/useGasFeeEstimates.js b/ui/hooks/useGasFeeEstimates.js index 7bbdd4fc0..92067e050 100644 --- a/ui/hooks/useGasFeeEstimates.js +++ b/ui/hooks/useGasFeeEstimates.js @@ -1,4 +1,5 @@ -import { useSelector } from 'react-redux'; +import isEqual from 'lodash/isEqual'; +import { shallowEqual, useSelector } from 'react-redux'; import { getEstimatedGasFeeTimeBounds, getGasEstimateType, @@ -31,8 +32,11 @@ import { useSafeGasEstimatePolling } from './useSafeGasEstimatePolling'; */ export function useGasFeeEstimates() { const gasEstimateType = useSelector(getGasEstimateType); - const gasFeeEstimates = useSelector(getGasFeeEstimates); - const estimatedGasFeeTimeBounds = useSelector(getEstimatedGasFeeTimeBounds); + const gasFeeEstimates = useSelector(getGasFeeEstimates, isEqual); + const estimatedGasFeeTimeBounds = useSelector( + getEstimatedGasFeeTimeBounds, + shallowEqual, + ); const isGasEstimatesLoading = useSelector(getIsGasEstimatesLoading); useSafeGasEstimatePolling(); diff --git a/ui/hooks/useTokenFiatAmount.js b/ui/hooks/useTokenFiatAmount.js index dc52a979a..8fe0cfc7c 100644 --- a/ui/hooks/useTokenFiatAmount.js +++ b/ui/hooks/useTokenFiatAmount.js @@ -1,5 +1,5 @@ import { useMemo } from 'react'; -import { useSelector } from 'react-redux'; +import { shallowEqual, useSelector } from 'react-redux'; import { getTokenExchangeRates, getCurrentCurrency, @@ -29,7 +29,10 @@ export function useTokenFiatAmount( overrides = {}, hideCurrencySymbol, ) { - const contractExchangeRates = useSelector(getTokenExchangeRates); + const contractExchangeRates = useSelector( + getTokenExchangeRates, + shallowEqual, + ); const conversionRate = useSelector(getConversionRate); const currentCurrency = useSelector(getCurrentCurrency); const userPrefersShownFiat = useSelector(getShouldShowFiat); diff --git a/ui/hooks/useTokenTracker.js b/ui/hooks/useTokenTracker.js index 5ad2fb5af..01b56f615 100644 --- a/ui/hooks/useTokenTracker.js +++ b/ui/hooks/useTokenTracker.js @@ -1,6 +1,6 @@ import { useState, useEffect, useRef, useCallback } from 'react'; import TokenTracker from '@metamask/eth-token-tracker'; -import { useSelector } from 'react-redux'; +import { shallowEqual, useSelector } from 'react-redux'; import { getCurrentChainId, getSelectedAddress } from '../selectors'; import { SECOND } from '../../shared/constants/time'; import { isEqualCaseInsensitive } from '../helpers/utils/util'; @@ -12,7 +12,7 @@ export function useTokenTracker( hideZeroBalanceTokens = false, ) { const chainId = useSelector(getCurrentChainId); - const userAddress = useSelector(getSelectedAddress); + const userAddress = useSelector(getSelectedAddress, shallowEqual); const [loading, setLoading] = useState(() => tokens?.length >= 0); const [tokensWithBalances, setTokensWithBalances] = useState([]); const [error, setError] = useState(null); diff --git a/ui/hooks/useTokensToSearch.js b/ui/hooks/useTokensToSearch.js index 50bc438f9..7140737a4 100644 --- a/ui/hooks/useTokensToSearch.js +++ b/ui/hooks/useTokensToSearch.js @@ -1,5 +1,5 @@ import { useMemo } from 'react'; -import { useSelector } from 'react-redux'; +import { shallowEqual, useSelector } from 'react-redux'; import contractMap from '@metamask/contract-metadata'; import BigNumber from 'bignumber.js'; import { isEqual, shuffle, uniqBy } from 'lodash'; @@ -94,8 +94,8 @@ export function useTokensToSearch({ const tokenConversionRates = useSelector(getTokenExchangeRates, isEqual); const conversionRate = useSelector(getConversionRate); const currentCurrency = useSelector(getCurrentCurrency); - const defaultSwapsToken = useSelector(getSwapsDefaultToken); - const tokenList = useSelector(getTokenList); + const defaultSwapsToken = useSelector(getSwapsDefaultToken, shallowEqual); + const tokenList = useSelector(getTokenList, isEqual); const useTokenDetection = useSelector(getUseTokenDetection); // token from dynamic api list is fetched when useTokenDetection is true const shuffledTokenList = useTokenDetection @@ -115,7 +115,7 @@ export function useTokensToSearch({ ); const memoizedDefaultToken = useEqualityCheck(defaultToken); - const swapsTokens = useSelector(getSwapsTokens) || []; + const swapsTokens = useSelector(getSwapsTokens, isEqual) || []; const tokensToSearch = swapsTokens.length ? swapsTokens diff --git a/ui/pages/routes/routes.component.js b/ui/pages/routes/routes.component.js index 5dbe37745..f83efe74d 100644 --- a/ui/pages/routes/routes.component.js +++ b/ui/pages/routes/routes.component.js @@ -77,8 +77,6 @@ export default class Routes extends Component { alertMessage: PropTypes.string, textDirection: PropTypes.string, isNetworkLoading: PropTypes.bool, - provider: PropTypes.object, - frequentRpcListDetail: PropTypes.array, alertOpen: PropTypes.bool, isUnlocked: PropTypes.bool, setLastActiveTime: PropTypes.func, @@ -88,10 +86,12 @@ export default class Routes extends Component { isMouseUser: PropTypes.bool, setMouseUserState: PropTypes.func, providerId: PropTypes.string, + providerType: PropTypes.string, autoLockTimeLimit: PropTypes.number, pageChanged: PropTypes.func.isRequired, prepareToLeaveSwaps: PropTypes.func, - browserEnvironment: PropTypes.object, + browserEnvironmentOs: PropTypes.string, + browserEnvironmentBrowser: PropTypes.string, }; static contextTypes = { @@ -287,6 +287,13 @@ export default class Routes extends Component { ); } + onAppHeaderClick = async () => { + const { prepareToLeaveSwaps } = this.props; + if (this.onSwapsPage()) { + await prepareToLeaveSwaps(); + } + }; + render() { const { isLoading, @@ -295,19 +302,16 @@ export default class Routes extends Component { textDirection, loadingMessage, isNetworkLoading, - provider, - frequentRpcListDetail, setMouseUserState, isMouseUser, - prepareToLeaveSwaps, - browserEnvironment, + browserEnvironmentOs: os, + browserEnvironmentBrowser: browser, } = this.props; const loadMessage = loadingMessage || isNetworkLoading ? this.getConnectingLabel(loadingMessage) : null; - const { os, browser } = browserEnvironment; return (
{ - if (this.onSwapsPage()) { - await prepareToLeaveSwaps(); - } - }} + onClick={this.onAppHeaderClick} disabled={ this.onConfirmPage() || (this.onSwapsPage() && !this.onSwapsBuildQuotePage()) @@ -344,10 +344,7 @@ export default class Routes extends Component { {process.env.ONBOARDING_V2 && this.showOnboardingHeader() && ( )} - +
{isLoading ? : null} @@ -377,9 +374,9 @@ export default class Routes extends Component { if (loadingMessage) { return loadingMessage; } - const { provider, providerId } = this.props; + const { providerType, providerId } = this.props; - switch (provider.type) { + switch (providerType) { case 'mainnet': return this.context.t('connectingToMainnet'); case 'ropsten': @@ -394,21 +391,4 @@ export default class Routes extends Component { return this.context.t('connectingTo', [providerId]); } } - - getNetworkName() { - switch (this.props.provider.type) { - case 'mainnet': - return this.context.t('mainnet'); - case 'ropsten': - return this.context.t('ropsten'); - case 'kovan': - return this.context.t('kovan'); - case 'rinkeby': - return this.context.t('rinkeby'); - case 'goerli': - return this.context.t('goerli'); - default: - return this.context.t('unknownNetwork'); - } - } } diff --git a/ui/pages/routes/routes.container.js b/ui/pages/routes/routes.container.js index a9f0d977a..e8f6ba6ad 100644 --- a/ui/pages/routes/routes.container.js +++ b/ui/pages/routes/routes.container.js @@ -5,7 +5,6 @@ import { getNetworkIdentifier, getPreferences, isNetworkLoading, - submittedPendingTransactionsSelector, } from '../../selectors'; import { lockMetamask, @@ -29,15 +28,14 @@ function mapStateToProps(state) { isLoading, loadingMessage, isUnlocked: state.metamask.isUnlocked, - submittedPendingTransactions: submittedPendingTransactionsSelector(state), isNetworkLoading: isNetworkLoading(state), - provider: state.metamask.provider, - frequentRpcListDetail: state.metamask.frequentRpcListDetail || [], currentCurrency: state.metamask.currentCurrency, isMouseUser: state.appState.isMouseUser, - providerId: getNetworkIdentifier(state), autoLockTimeLimit, - browserEnvironment: state.metamask.browserEnvironment, + browserEnvironmentOs: state.metamask.browserEnvironment?.os, + browserEnvironmentContainter: state.metamask.browserEnvironment?.browser, + providerId: getNetworkIdentifier(state), + providerType: state.metamask.provider?.type, }; } diff --git a/ui/pages/swaps/awaiting-swap/awaiting-swap.js b/ui/pages/swaps/awaiting-swap/awaiting-swap.js index 8fbeeab83..f3c1b5f5d 100644 --- a/ui/pages/swaps/awaiting-swap/awaiting-swap.js +++ b/ui/pages/swaps/awaiting-swap/awaiting-swap.js @@ -26,6 +26,8 @@ import { navigateBackToBuildQuote, prepareForRetryGetQuotes, prepareToLeaveSwaps, + getFromTokenInputValue, + getMaxSlippage, } from '../../../ducks/swaps/swaps'; import Mascot from '../../../components/ui/mascot'; import Box from '../../../components/ui/box'; @@ -58,8 +60,6 @@ export default function AwaitingSwap({ txHash, tokensReceived, submittingSwap, - inputValue, - maxSlippage, }) { const t = useContext(I18nContext); const metaMetricsEvent = useContext(MetaMetricsContext); @@ -69,6 +69,8 @@ export default function AwaitingSwap({ const fetchParams = useSelector(getFetchParams); const { destinationTokenInfo, sourceTokenInfo } = fetchParams?.metaData || {}; + const fromTokenInputValue = useSelector(getFromTokenInputValue); + const maxSlippage = useSelector(getMaxSlippage); const usedQuote = useSelector(getUsedQuote); const approveTxParams = useSelector(getApproveTxParams); const swapsGasPrice = useSelector(getUsedSwapsGasPrice); @@ -283,7 +285,7 @@ export default function AwaitingSwap({ await dispatch( fetchQuotesAndSetQuoteState( history, - inputValue, + fromTokenInputValue, maxSlippage, metaMetricsEvent, ), @@ -321,6 +323,4 @@ AwaitingSwap.propTypes = { CONTRACT_DATA_DISABLED_ERROR, ]), submittingSwap: PropTypes.bool, - inputValue: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), - maxSlippage: PropTypes.number, }; diff --git a/ui/pages/swaps/build-quote/__snapshots__/build-quote.test.js.snap b/ui/pages/swaps/build-quote/__snapshots__/build-quote.test.js.snap index 844710b12..4314aea3a 100644 --- a/ui/pages/swaps/build-quote/__snapshots__/build-quote.test.js.snap +++ b/ui/pages/swaps/build-quote/__snapshots__/build-quote.test.js.snap @@ -14,25 +14,20 @@ exports[`BuildQuote renders the component with initial props 1`] = ` 2%
`; diff --git a/ui/pages/swaps/build-quote/build-quote.js b/ui/pages/swaps/build-quote/build-quote.js index 32e25816d..04fb4553a 100644 --- a/ui/pages/swaps/build-quote/build-quote.js +++ b/ui/pages/swaps/build-quote/build-quote.js @@ -1,6 +1,7 @@ import React, { useContext, useEffect, useState, useCallback } from 'react'; +import BigNumber from 'bignumber.js'; import PropTypes from 'prop-types'; -import { useDispatch, useSelector } from 'react-redux'; +import { shallowEqual, useDispatch, useSelector } from 'react-redux'; import classnames from 'classnames'; import { uniqBy, isEqual } from 'lodash'; import { useHistory } from 'react-router-dom'; @@ -34,7 +35,15 @@ import { getTopAssets, getFetchParams, getQuotes, + setBalanceError, + setFromTokenInputValue, + setFromTokenError, + setMaxSlippage, setReviewSwapClickedTimestamp, + getFromTokenInputValue, + getFromTokenError, + getMaxSlippage, + getIsFeatureFlagLoaded, } from '../../../ducks/swaps/swaps'; import { getSwapsDefaultToken, @@ -77,6 +86,7 @@ import { stopPollingForQuotes, } from '../../../store/actions'; import { + countDecimals, fetchTokenPrice, fetchTokenBalance, shouldEnableDirectWrapping, @@ -94,14 +104,8 @@ const MAX_ALLOWED_SLIPPAGE = 15; let timeoutIdForQuotesPrefetching; export default function BuildQuote({ - inputValue, - onInputChange, ethBalance, - setMaxSlippage, - maxSlippage, selectedAccountAddress, - isFeatureFlagLoaded, - tokenFromError, shuffledTokensList, }) { const t = useContext(I18nContext); @@ -114,18 +118,22 @@ export default function BuildQuote({ ); const [verificationClicked, setVerificationClicked] = useState(false); + const isFeatureFlagLoaded = useSelector(getIsFeatureFlagLoaded); const balanceError = useSelector(getBalanceError); - const fetchParams = useSelector(getFetchParams); + const fetchParams = useSelector(getFetchParams, isEqual); const { sourceTokenInfo = {}, destinationTokenInfo = {} } = fetchParams?.metaData || {}; - const tokens = useSelector(getTokens); + const tokens = useSelector(getTokens, isEqual); const topAssets = useSelector(getTopAssets); - const fromToken = useSelector(getFromToken); + const fromToken = useSelector(getFromToken, isEqual); + const fromTokenInputValue = useSelector(getFromTokenInputValue); + const fromTokenError = useSelector(getFromTokenError); + const maxSlippage = useSelector(getMaxSlippage); const toToken = useSelector(getToToken) || destinationTokenInfo; - const defaultSwapsToken = useSelector(getSwapsDefaultToken); + const defaultSwapsToken = useSelector(getSwapsDefaultToken, isEqual); const chainId = useSelector(getCurrentChainId); - const rpcPrefs = useSelector(getRpcPrefsForCurrentProvider); - const tokenList = useSelector(getTokenList); + const rpcPrefs = useSelector(getRpcPrefsForCurrentProvider, shallowEqual); + const tokenList = useSelector(getTokenList, isEqual); const useTokenDetection = useSelector(getUseTokenDetection); const quotes = useSelector(getQuotes, isEqual); const areQuotesPresent = Object.keys(quotes).length > 0; @@ -198,7 +206,7 @@ export default function BuildQuote({ const swapFromTokenFiatValue = useTokenFiatAmount( fromTokenAddress, - inputValue || 0, + fromTokenInputValue || 0, fromTokenSymbol, { showFiat: true, @@ -206,7 +214,7 @@ export default function BuildQuote({ true, ); const swapFromEthFiatValue = useEthFiatAmount( - inputValue || 0, + fromTokenInputValue || 0, { showFiat: true }, true, ); @@ -214,6 +222,27 @@ export default function BuildQuote({ ? swapFromEthFiatValue : swapFromTokenFiatValue; + const onInputChange = useCallback( + (newInputValue, balance) => { + dispatch(setFromTokenInputValue(newInputValue)); + const newBalanceError = new BigNumber(newInputValue || 0).gt( + balance || 0, + ); + // "setBalanceError" is just a warning, a user can still click on the "Review Swap" button. + if (balanceError !== newBalanceError) { + dispatch(setBalanceError(newBalanceError)); + } + dispatch( + setFromTokenError( + fromToken && countDecimals(newInputValue) > fromToken.decimals + ? 'tooManyDecimals' + : null, + ), + ); + }, + [dispatch, fromToken, balanceError], + ); + const onFromSelect = (token) => { if ( token?.address && @@ -255,7 +284,7 @@ export default function BuildQuote({ } dispatch(setSwapsFromToken(token)); onInputChange( - token?.address ? inputValue : '', + token?.address ? fromTokenInputValue : '', token.string, token.decimals, ); @@ -364,9 +393,14 @@ export default function BuildQuote({ useEffect(() => { if (prevFromTokenBalance !== fromTokenBalance) { - onInputChange(inputValue, fromTokenBalance); + onInputChange(fromTokenInputValue, fromTokenBalance); } - }, [onInputChange, prevFromTokenBalance, inputValue, fromTokenBalance]); + }, [ + onInputChange, + prevFromTokenBalance, + fromTokenInputValue, + fromTokenBalance, + ]); useEffect(() => { dispatch(resetSwapsPostFetchState()); @@ -416,9 +450,9 @@ export default function BuildQuote({ selectedToToken.address, ); const isReviewSwapButtonDisabled = - tokenFromError || + fromTokenError || !isFeatureFlagLoaded || - !Number(inputValue) || + !Number(fromTokenInputValue) || !selectedToToken?.address || Number(maxSlippage) < 0 || Number(maxSlippage) > MAX_ALLOWED_SLIPPAGE || @@ -433,7 +467,7 @@ export default function BuildQuote({ await dispatch( fetchQuotesAndSetQuoteState( history, - inputValue, + fromTokenInputValue, maxSlippage, metaMetricsEvent, pageRedirectionDisabled, @@ -456,7 +490,7 @@ export default function BuildQuote({ maxSlippage, metaMetricsEvent, isReviewSwapButtonDisabled, - inputValue, + fromTokenInputValue, fromTokenAddress, toTokenAddress, ]); @@ -484,8 +518,8 @@ export default function BuildQuote({ onInputChange={(value) => { onInputChange(value, fromTokenBalance); }} - inputValue={inputValue} - leftValue={inputValue && swapFromFiatValue} + inputValue={fromTokenInputValue} + leftValue={fromTokenInputValue && swapFromFiatValue} selectedItem={selectedFromToken} maxListItems={30} loading={ @@ -504,14 +538,14 @@ export default function BuildQuote({
- {!tokenFromError && + {!fromTokenError && !balanceError && fromTokenSymbol && swapYourTokenBalance} - {!tokenFromError && balanceError && fromTokenSymbol && ( + {!fromTokenError && balanceError && fromTokenSymbol && (
{t('swapsNotEnoughForTx', [fromTokenSymbol])} @@ -521,7 +555,7 @@ export default function BuildQuote({
)} - {tokenFromError && ( + {fromTokenError && ( <>
{t('swapTooManyDecimalsError', [ @@ -645,7 +679,7 @@ export default function BuildQuote({
{ - setMaxSlippage(newSlippage); + dispatch(setMaxSlippage(newSlippage)); }} maxAllowedSlippage={MAX_ALLOWED_SLIPPAGE} currentSlippage={maxSlippage} @@ -664,7 +698,7 @@ export default function BuildQuote({ dispatch( fetchQuotesAndSetQuoteState( history, - inputValue, + fromTokenInputValue, maxSlippage, metaMetricsEvent, ), @@ -688,13 +722,7 @@ export default function BuildQuote({ } BuildQuote.propTypes = { - maxSlippage: PropTypes.number, - inputValue: PropTypes.string, - onInputChange: PropTypes.func, ethBalance: PropTypes.string, - setMaxSlippage: PropTypes.func, selectedAccountAddress: PropTypes.string, - isFeatureFlagLoaded: PropTypes.bool.isRequired, - tokenFromError: PropTypes.string, shuffledTokensList: PropTypes.array, }; diff --git a/ui/pages/swaps/build-quote/build-quote.test.js b/ui/pages/swaps/build-quote/build-quote.test.js index 759ebb9b2..12f1b27bc 100644 --- a/ui/pages/swaps/build-quote/build-quote.test.js +++ b/ui/pages/swaps/build-quote/build-quote.test.js @@ -1,7 +1,6 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; import thunk from 'redux-thunk'; -import { fireEvent } from '@testing-library/react'; import { renderWithProvider, @@ -13,11 +12,7 @@ import BuildQuote from '.'; const middleware = [thunk]; const createProps = (customProps = {}) => { return { - inputValue: '5', - onInputChange: jest.fn(), ethBalance: '0x8', - setMaxSlippage: jest.fn(), - maxSlippage: 15, selectedAccountAddress: 'selectedAccountAddress', isFeatureFlagLoaded: false, shuffledTokensList: [], @@ -49,28 +44,4 @@ describe('BuildQuote', () => { document.querySelector('.slippage-buttons__button-group'), ).toMatchSnapshot(); }); - - it('clicks on the max button', () => { - const store = configureMockStore(middleware)(createSwapsMockStore()); - const props = createProps(); - const { getByTestId } = renderWithProvider( - , - store, - ); - fireEvent.click(getByTestId('build-quote__max-button')); - expect(props.onInputChange).toHaveBeenCalled(); - }); - - it('types a number inside the input field', () => { - const store = configureMockStore(middleware)(createSwapsMockStore()); - const props = createProps(); - const { getByDisplayValue } = renderWithProvider( - , - store, - ); - fireEvent.change(getByDisplayValue('5'), { - target: { value: '8' }, - }); - expect(props.onInputChange).toHaveBeenCalled(); - }); }); diff --git a/ui/pages/swaps/index.js b/ui/pages/swaps/index.js index 474c0b766..4da5d6742 100644 --- a/ui/pages/swaps/index.js +++ b/ui/pages/swaps/index.js @@ -1,5 +1,5 @@ -import React, { useState, useEffect, useRef, useContext } from 'react'; -import { useDispatch, useSelector } from 'react-redux'; +import React, { useEffect, useRef, useContext } from 'react'; +import { shallowEqual, useDispatch, useSelector } from 'react-redux'; import { Switch, Route, @@ -7,8 +7,7 @@ import { useHistory, Redirect, } from 'react-router-dom'; -import BigNumber from 'bignumber.js'; -import { shuffle } from 'lodash'; +import { shuffle, isEqual } from 'lodash'; import { I18nContext } from '../../contexts/i18n'; import { getSelectedAccount, @@ -24,7 +23,6 @@ import { getTradeTxId, getApproveTxId, getFetchingQuotes, - setBalanceError, setTopAssets, getFetchParams, setAggregatorMetadata, @@ -35,7 +33,6 @@ import { prepareToLeaveSwaps, fetchAndSetSwapsGasPriceInfo, fetchSwapsLiveness, - getFromToken, getReviewSwapClickedTimestamp, } from '../../ducks/swaps/swaps'; import { @@ -77,7 +74,6 @@ import { fetchTopAssets, getSwapsTokensReceivedFromTxMeta, fetchAggregatorMetadata, - countDecimals, } from './swaps.util'; import AwaitingSignatures from './awaiting-signatures'; import AwaitingSwap from './awaiting-swap'; @@ -96,21 +92,16 @@ export default function Swap() { const isSwapsErrorRoute = pathname === SWAPS_ERROR_ROUTE; const isLoadingQuotesRoute = pathname === LOADING_QUOTES_ROUTE; - const fetchParams = useSelector(getFetchParams); + const fetchParams = useSelector(getFetchParams, isEqual); const { destinationTokenInfo = {} } = fetchParams?.metaData || {}; - const [inputValue, setInputValue] = useState(fetchParams?.value || ''); - const [maxSlippage, setMaxSlippage] = useState(fetchParams?.slippage || 3); - const [isFeatureFlagLoaded, setIsFeatureFlagLoaded] = useState(false); - const [tokenFromError, setTokenFromError] = useState(null); - const routeState = useSelector(getBackgroundSwapRouteState); - const selectedAccount = useSelector(getSelectedAccount); - const quotes = useSelector(getQuotes); - const txList = useSelector(currentNetworkTxListSelector); + const selectedAccount = useSelector(getSelectedAccount, shallowEqual); + const quotes = useSelector(getQuotes, isEqual); + const txList = useSelector(currentNetworkTxListSelector, shallowEqual); const tradeTxId = useSelector(getTradeTxId); const approveTxId = useSelector(getApproveTxId); - const aggregatorMetadata = useSelector(getAggregatorMetadata); + const aggregatorMetadata = useSelector(getAggregatorMetadata, shallowEqual); const fetchingQuotes = useSelector(getFetchingQuotes); let swapsErrorKey = useSelector(getSwapsErrorKey); const swapsEnabled = useSelector(getSwapsFeatureIsLive); @@ -119,8 +110,7 @@ export default function Swap() { const networkAndAccountSupports1559 = useSelector( checkNetworkAndAccountSupports1559, ); - const fromToken = useSelector(getFromToken); - const tokenList = useSelector(getTokenList); + const tokenList = useSelector(getTokenList, isEqual); const listTokenValues = shuffle(Object.values(tokenList)); const reviewSwapClickedTimestamp = useSelector(getReviewSwapClickedTimestamp); const reviewSwapClicked = Boolean(reviewSwapClickedTimestamp); @@ -237,7 +227,6 @@ export default function Swap() { useEffect(() => { const fetchSwapsLivenessWrapper = async () => { await dispatch(fetchSwapsLiveness()); - setIsFeatureFlagLoaded(true); }; fetchSwapsLivenessWrapper(); return () => { @@ -308,31 +297,10 @@ export default function Swap() { return ; } - const onInputChange = (newInputValue, balance) => { - setInputValue(newInputValue); - const balanceError = new BigNumber(newInputValue || 0).gt( - balance || 0, - ); - // "setBalanceError" is just a warning, a user can still click on the "Review Swap" button. - dispatch(setBalanceError(balanceError)); - setTokenFromError( - fromToken && - countDecimals(newInputValue) > fromToken.decimals - ? 'tooManyDecimals' - : null, - ); - }; - return ( ); @@ -364,8 +332,6 @@ export default function Swap() { swapComplete={false} errorKey={swapsErrorKey} txHash={tradeTxData?.hash} - inputValue={inputValue} - maxSlippage={maxSlippage} submittedTime={tradeTxData?.submittedTime} /> ); @@ -433,8 +399,6 @@ export default function Swap() { submittingSwap={ routeState === 'awaiting' && !(approveTxId || tradeTxId) } - inputValue={inputValue} - maxSlippage={maxSlippage} /> ) : (