From 92a79904f7ad222366603eb7d5b269884d3e716a Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Thu, 6 May 2021 07:14:42 -0700 Subject: [PATCH] Swaps: Improve hardware wallet UX (#10987) --- app/_locales/en/messages.json | 20 +++ ui/ducks/swaps/swaps.js | 24 ++- ui/helpers/constants/routes.js | 2 + .../awaiting-signatures.test.js.snap | 26 ++++ .../__snapshots__/swap-step-icon.test.js.snap | 25 ++++ .../awaiting-signatures.js | 139 ++++++++++++++++++ .../awaiting-signatures.test.js | 17 +++ ui/pages/swaps/awaiting-signatures/index.js | 1 + ui/pages/swaps/awaiting-signatures/index.scss | 34 +++++ .../awaiting-signatures/swap-step-icon.js | 40 +++++ .../swap-step-icon.test.js | 11 ++ ui/pages/swaps/index.js | 12 +- ui/pages/swaps/index.scss | 1 + ui/selectors/selectors.js | 10 ++ 14 files changed, 359 insertions(+), 3 deletions(-) create mode 100644 ui/pages/swaps/awaiting-signatures/__snapshots__/awaiting-signatures.test.js.snap create mode 100644 ui/pages/swaps/awaiting-signatures/__snapshots__/swap-step-icon.test.js.snap create mode 100644 ui/pages/swaps/awaiting-signatures/awaiting-signatures.js create mode 100644 ui/pages/swaps/awaiting-signatures/awaiting-signatures.test.js create mode 100644 ui/pages/swaps/awaiting-signatures/index.js create mode 100644 ui/pages/swaps/awaiting-signatures/index.scss create mode 100644 ui/pages/swaps/awaiting-signatures/swap-step-icon.js create mode 100644 ui/pages/swaps/awaiting-signatures/swap-step-icon.test.js diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 2f4469d7f..87d80765c 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1804,6 +1804,10 @@ "swapAggregator": { "message": "Aggregator" }, + "swapAllowSwappingOf": { + "message": "Allow swapping of $1", + "description": "Shows a user that they need to allow a token for swapping on their hardware wallet" + }, "swapAmountReceived": { "message": "Guaranteed amount" }, @@ -1829,6 +1833,9 @@ "message": "Checking $1", "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." }, + "swapConfirmWithHwWallet": { + "message": "Confirm with your hardware wallet" + }, "swapCustom": { "message": "custom" }, @@ -1874,6 +1881,13 @@ "swapFinalizing": { "message": "Finalizing..." }, + "swapFromTo": { + "message": "The swap of $1 to $2", + "description": "Tells a user that they need to confirm on their hardware wallet a swap of 2 tokens. $1 is a source token and $2 is a destination token" + }, + "swapGasFeesSplit": { + "message": "Gas fees on the previous screen are split between these two transactions." + }, "swapHighSlippageWarning": { "message": "Slippage amount is very high." }, @@ -2015,6 +2029,9 @@ "swapThisWillAllowApprove": { "message": "This will allow $1 to be swapped." }, + "swapToConfirmWithHwWallet": { + "message": "to confirm with your hardware wallet" + }, "swapTokenAvailable": { "message": "Your $1 has been added to your account.", "description": "This message is shown after a swap is successful and communicates the exact amount of tokens the user has received for a swap. The $1 is a decimal number of tokens followed by the token symbol." @@ -2044,6 +2061,9 @@ "swapTransactionComplete": { "message": "Transaction complete" }, + "swapTwoTransactions": { + "message": "2 transactions" + }, "swapUnknown": { "message": "Unknown" }, diff --git a/ui/ducks/swaps/swaps.js b/ui/ducks/swaps/swaps.js index 349e913c1..49cdd7082 100644 --- a/ui/ducks/swaps/swaps.js +++ b/ui/ducks/swaps/swaps.js @@ -26,6 +26,7 @@ import { cancelTx, } from '../../store/actions'; import { + AWAITING_SIGNATURES_ROUTE, AWAITING_SWAP_ROUTE, BUILD_QUOTE_ROUTE, LOADING_QUOTES_ROUTE, @@ -52,6 +53,7 @@ import { getUSDConversionRate, getSwapsDefaultToken, getCurrentChainId, + isHardwareWallet, } from '../../selectors'; import { ERROR_FETCHING_QUOTES, @@ -73,6 +75,7 @@ export const FALLBACK_GAS_MULTIPLIER = 1.5; const initialState = { aggregatorMetadata: null, approveTxId: null, + tradeTxId: null, balanceError: false, fetchingQuotes: false, fromToken: null, @@ -95,6 +98,7 @@ const slice = createSlice({ clearSwapsState: () => initialState, navigatedBackToBuildQuote: (state) => { state.approveTxId = null; + state.tradeTxId = null; state.balanceError = false; state.fetchingQuotes = false; state.customGas.limit = null; @@ -585,7 +589,7 @@ export const signAndSendTransactions = (history, metaMetricsEvent) => { return async (dispatch, getState) => { const state = getState(); const chainId = getCurrentChainId(state); - + const hardwareWalletUsed = isHardwareWallet(state); let swapsFeatureIsLive = false; try { swapsFeatureIsLive = await fetchSwapsFeatureLiveness(chainId); @@ -605,7 +609,10 @@ export const signAndSendTransactions = (history, metaMetricsEvent) => { const { sourceTokenInfo = {}, destinationTokenInfo = {} } = metaData; await dispatch(setBackgroundSwapRouteState('awaiting')); await dispatch(stopPollingForQuotes()); - history.push(AWAITING_SWAP_ROUTE); + + if (!hardwareWalletUsed) { + history.push(AWAITING_SWAP_ROUTE); + } const { fast: fastGasEstimate } = getSwapGasPriceEstimateData(state); @@ -694,6 +701,13 @@ export const signAndSendTransactions = (history, metaMetricsEvent) => { let finalApproveTxMeta; const approveTxParams = getApproveTxParams(state); + + // For hardware wallets we go to the Awaiting Signatures page first and only after a user + // completes 1 or 2 confirmations, we redirect to the Awaiting Swap page. + if (hardwareWalletUsed) { + history.push(AWAITING_SIGNATURES_ROUTE); + } + if (approveTxParams) { const approveTxMeta = await dispatch( addUnapprovedTransaction( @@ -765,6 +779,12 @@ export const signAndSendTransactions = (history, metaMetricsEvent) => { return; } + // Only after a user confirms swapping on a hardware wallet (second `updateAndApproveTx` call above), + // we redirect to the Awaiting Swap page. + if (hardwareWalletUsed) { + history.push(AWAITING_SWAP_ROUTE); + } + await forceUpdateMetamaskState(dispatch); }; }; diff --git a/ui/helpers/constants/routes.js b/ui/helpers/constants/routes.js index f2666ffbf..8c079a8f0 100644 --- a/ui/helpers/constants/routes.js +++ b/ui/helpers/constants/routes.js @@ -32,6 +32,7 @@ const SWAPS_ROUTE = '/swaps'; const BUILD_QUOTE_ROUTE = '/swaps/build-quote'; const VIEW_QUOTE_ROUTE = '/swaps/view-quote'; const LOADING_QUOTES_ROUTE = '/swaps/loading-quotes'; +const AWAITING_SIGNATURES_ROUTE = '/swaps/awaiting-signatures'; const AWAITING_SWAP_ROUTE = '/swaps/awaiting-swap'; const SWAPS_ERROR_ROUTE = '/swaps/swaps-error'; const SWAPS_MAINTENANCE_ROUTE = '/swaps/maintenance'; @@ -191,6 +192,7 @@ export { VIEW_QUOTE_ROUTE, LOADING_QUOTES_ROUTE, AWAITING_SWAP_ROUTE, + AWAITING_SIGNATURES_ROUTE, SWAPS_ERROR_ROUTE, SWAPS_MAINTENANCE_ROUTE, }; diff --git a/ui/pages/swaps/awaiting-signatures/__snapshots__/awaiting-signatures.test.js.snap b/ui/pages/swaps/awaiting-signatures/__snapshots__/awaiting-signatures.test.js.snap new file mode 100644 index 000000000..a7b8b3546 --- /dev/null +++ b/ui/pages/swaps/awaiting-signatures/__snapshots__/awaiting-signatures.test.js.snap @@ -0,0 +1,26 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`AwaitingSignatures renders the component with initial props for 1 confirmation 1`] = ` + +`; diff --git a/ui/pages/swaps/awaiting-signatures/__snapshots__/swap-step-icon.test.js.snap b/ui/pages/swaps/awaiting-signatures/__snapshots__/swap-step-icon.test.js.snap new file mode 100644 index 000000000..681947428 --- /dev/null +++ b/ui/pages/swaps/awaiting-signatures/__snapshots__/swap-step-icon.test.js.snap @@ -0,0 +1,25 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`SwapStepIcon renders the component 1`] = ` +
+ + + + +
+`; diff --git a/ui/pages/swaps/awaiting-signatures/awaiting-signatures.js b/ui/pages/swaps/awaiting-signatures/awaiting-signatures.js new file mode 100644 index 000000000..cb2bc40d8 --- /dev/null +++ b/ui/pages/swaps/awaiting-signatures/awaiting-signatures.js @@ -0,0 +1,139 @@ +import React, { useContext, useEffect } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { useHistory } from 'react-router-dom'; + +import { I18nContext } from '../../../contexts/i18n'; +import { useNewMetricEvent } from '../../../hooks/useMetricEvent'; +import { + getFetchParams, + getApproveTxParams, + prepareToLeaveSwaps, +} from '../../../ducks/swaps/swaps'; +import { + DEFAULT_ROUTE, + BUILD_QUOTE_ROUTE, +} from '../../../helpers/constants/routes'; +import PulseLoader from '../../../components/ui/pulse-loader'; +import Typography from '../../../components/ui/typography'; +import Box from '../../../components/ui/box'; +import { + BLOCK_SIZES, + COLORS, + TYPOGRAPHY, + FONT_WEIGHT, + JUSTIFY_CONTENT, + DISPLAY, +} from '../../../helpers/constants/design-system'; +import SwapsFooter from '../swaps-footer'; +import SwapStepIcon from './swap-step-icon'; + +export default function AwaitingSignatures() { + const t = useContext(I18nContext); + const history = useHistory(); + const dispatch = useDispatch(); + const fetchParams = useSelector(getFetchParams); + const { destinationTokenInfo, sourceTokenInfo } = fetchParams?.metaData || {}; + const approveTxParams = useSelector(getApproveTxParams); + const needsTwoConfirmations = Boolean(approveTxParams); + + const awaitingSignaturesEvent = useNewMetricEvent({ + event: 'Awaiting Signature(s) on a HW wallet', + sensitiveProperties: { + needs_two_confirmations: needsTwoConfirmations, + token_from: sourceTokenInfo?.symbol, + token_from_amount: fetchParams?.value, + token_to: destinationTokenInfo?.symbol, + request_type: fetchParams?.balanceError ? 'Quote' : 'Order', + slippage: fetchParams?.slippage, + custom_slippage: fetchParams?.slippage === 2, + }, + category: 'swaps', + }); + + useEffect(() => { + awaitingSignaturesEvent(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + const headerText = needsTwoConfirmations + ? t('swapTwoTransactions') + : t('swapConfirmWithHwWallet'); + + return ( +
+ + + + + + {headerText} + + {needsTwoConfirmations && ( + <> + + {t('swapToConfirmWithHwWallet')} + + + + {t('swapGasFeesSplit')} + + + )} + + { + await dispatch(prepareToLeaveSwaps()); + // Go to the default route and then to the build quote route in order to clean up + // the `inputValue` local state in `pages/swaps/index.js` + history.push(DEFAULT_ROUTE); + history.push(BUILD_QUOTE_ROUTE); + }} + submitText={t('cancel')} + hideCancel + /> +
+ ); +} diff --git a/ui/pages/swaps/awaiting-signatures/awaiting-signatures.test.js b/ui/pages/swaps/awaiting-signatures/awaiting-signatures.test.js new file mode 100644 index 000000000..c8e466ec2 --- /dev/null +++ b/ui/pages/swaps/awaiting-signatures/awaiting-signatures.test.js @@ -0,0 +1,17 @@ +import React from 'react'; +import configureMockStore from 'redux-mock-store'; + +import { + renderWithProvider, + createSwapsMockStore, +} from '../../../../test/jest'; +import AwaitingSignatures from '.'; + +describe('AwaitingSignatures', () => { + it('renders the component with initial props for 1 confirmation', () => { + const store = configureMockStore()(createSwapsMockStore()); + const { getByText } = renderWithProvider(, store); + expect(getByText('Confirm with your hardware wallet')).toBeInTheDocument(); + expect(document.querySelector('.swaps-footer')).toMatchSnapshot(); + }); +}); diff --git a/ui/pages/swaps/awaiting-signatures/index.js b/ui/pages/swaps/awaiting-signatures/index.js new file mode 100644 index 000000000..3950ca6bf --- /dev/null +++ b/ui/pages/swaps/awaiting-signatures/index.js @@ -0,0 +1 @@ +export { default } from './awaiting-signatures'; diff --git a/ui/pages/swaps/awaiting-signatures/index.scss b/ui/pages/swaps/awaiting-signatures/index.scss new file mode 100644 index 000000000..9c847236c --- /dev/null +++ b/ui/pages/swaps/awaiting-signatures/index.scss @@ -0,0 +1,34 @@ +.awaiting-signatures { + display: flex; + flex-flow: column; + align-items: center; + flex: 1; + width: 100%; + + &__content { + flex-flow: column; + } + + div { + text-align: center; + display: flex; + justify-content: center; + } + + &__steps { + display: flex; + flex-direction: column; + align-items: flex-start; + margin: 16px auto 12px auto; + + li { + margin-bottom: 4px; + display: flex; + align-items: center; + + svg { + margin-right: 4px; + } + } + } +} diff --git a/ui/pages/swaps/awaiting-signatures/swap-step-icon.js b/ui/pages/swaps/awaiting-signatures/swap-step-icon.js new file mode 100644 index 000000000..3e5d50665 --- /dev/null +++ b/ui/pages/swaps/awaiting-signatures/swap-step-icon.js @@ -0,0 +1,40 @@ +import React from 'react'; + +export default function SwapStepIcon({ stepNumber = 1 }) { + switch (stepNumber) { + case 1: + return ( + + + + + ); + case 2: + return ( + + + + + ); + default: + return undefined; // Don't return any SVG if a step number is not supported. + } +} diff --git a/ui/pages/swaps/awaiting-signatures/swap-step-icon.test.js b/ui/pages/swaps/awaiting-signatures/swap-step-icon.test.js new file mode 100644 index 000000000..47c92d7aa --- /dev/null +++ b/ui/pages/swaps/awaiting-signatures/swap-step-icon.test.js @@ -0,0 +1,11 @@ +import React from 'react'; + +import { renderWithProvider } from '../../../../test/jest'; +import SwapStepIcon from './swap-step-icon'; + +describe('SwapStepIcon', () => { + it('renders the component', () => { + const { container } = renderWithProvider(); + expect(container).toMatchSnapshot(); + }); +}); diff --git a/ui/pages/swaps/index.js b/ui/pages/swaps/index.js index bf5dfb17f..a53c8527a 100644 --- a/ui/pages/swaps/index.js +++ b/ui/pages/swaps/index.js @@ -33,6 +33,7 @@ import { fetchSwapsLiveness, } from '../../ducks/swaps/swaps'; import { + AWAITING_SIGNATURES_ROUTE, AWAITING_SWAP_ROUTE, BUILD_QUOTE_ROUTE, VIEW_QUOTE_ROUTE, @@ -66,6 +67,7 @@ import { getSwapsTokensReceivedFromTxMeta, fetchAggregatorMetadata, } from './swaps.util'; +import AwaitingSignatures from './awaiting-signatures'; import AwaitingSwap from './awaiting-swap'; import LoadingQuote from './loading-swaps-quotes'; import BuildQuote from './build-quote'; @@ -78,6 +80,7 @@ export default function Swap() { const { pathname } = useLocation(); const isAwaitingSwapRoute = pathname === AWAITING_SWAP_ROUTE; + const isAwaitingSignaturesRoute = pathname === AWAITING_SIGNATURES_ROUTE; const isSwapsErrorRoute = pathname === SWAPS_ERROR_ROUTE; const isLoadingQuotesRoute = pathname === LOADING_QUOTES_ROUTE; @@ -243,7 +246,7 @@ export default function Swap() {
{t('swap')}
- {!isAwaitingSwapRoute && ( + {!isAwaitingSwapRoute && !isAwaitingSignaturesRoute && (
{ @@ -372,6 +375,13 @@ export default function Swap() { ); }} /> + { + return ; + }} + />