Commit e2dd78fd authored by eddie's avatar eddie Committed by GitHub

fix: design nits on summary view (#6623)

* fix: chainId required fixes

* fix: bad merge in e2e test

* fix: remove unused test util

* fix: remove unnecessary variable

* fix: token defaults

* fix: address comments

* fix: address comments and fix tests

* fix: e2e test formatting, remove Maybe<>

* fix: remove unused variable

* fix: use feature flag for swap component on TDP

* fix: back button

* feat: copy review screen UI from widgetg

* fix: modal padding

* feat: add final detail row

* fix: remove widget comment

* fix: update unit tests

* fix: code style consistency

* fix: remove padding from AutoColumn

* fix: update snapshots

* fix: use semantic gaps

* fix: more px and gaps

* fix: design feedbacks

* fix: button radius in summary modal

* fix: design nits

* feat: update design of summary modal

* fix: font weight and vertical spacing

* fix: update snapshots

* fix: css nits

* wip: move approval to summary modal

* wip: not working

* feat: working

* fix: fix flow

* feat: simplify states and build new modal UI

* feat: todos and differs fix

* feat: update tx status modal

* feat: split up approve and permit

* feat: error state

* feat: update success and error states

* feat: undo changes to TxConfirmationModal

* feat: re-order functions

* wip: move approval to summary modal

* wip: not working

* feat: update permit2 e2e tests

* feat: tests passing

* fix: swap test

* fix: bad merge

* wip: move approval to summary modal

* wip: not working

* feat: PendingModalContent tests

* feat: useMaxAmountIn

* fix: bad merge

* fix: naming

* fix: modal flicker when refetching trade

* wip: move approval to summary modal

* wip: not working

* feat: working

* fix: fix flow

* feat: simplify states and build new modal UI

* feat: todos and differs fix

* feat: update tx status modal

* feat: split up approve and permit

* feat: error state

* feat: update success and error states

* feat: undo changes to TxConfirmationModal

* feat: remove step indicators when only one step

* feat: move content into PendingModalContent component

* fix: lint

* chore: merge

* fix: update tests for new modal

* feat: add l2 chain logo to modal

* feat: add unit test

* fix: correct modal state when moving between steps

* fix: correct modal state when moving between steps

* fix: proper error handling of user rejection of swap

* feat: update e2e test

* fix: typecheck

* feat: design updates, state updates

* fix: comments

* fix: code style improvements

* feat: require trade to be defined

* fix: remove extra props from ThemedTexts

* fix: one more trans

* fix: remove unused export

* feat: remove undefined checks and other fixes

* fix: update test

* fix: add missing dollar sign

* fix: remove null check and update test

* fix: remove max width from detail row value

* fix: remove isOpen prop

* fix: isopen

* feat: refactor approval flow into a hook

* fix: custom error type

* fix: testid fix

* fix: text colors

* fix: add comment

* wip: permit2 modal animations

* fix: entrance animations

* feat: step indicator transitions

* feat: icon aniamtions

* feat: fix spinner icon

* fix: re-organize new code

* fix: svg import

* fix: tradeMeaningfullyDiffers improvement and prepareFlow fix

* fix: address  comments

* fix: headerContent prop

* fix: change tooltip to external link

* feat: add comments explaining async state

* fix: test updates

* fix: nits

* fix: design nits

* fix: reduce nesting

* fix: address comments

* test: remove line from test for debugging

* fix: update tests

* fix: address  comments

* fix: comments

* fix: update tests

* fix: update tests

* fix: more nesting in test

* feat: correct help center article

* fix: design nits on summary view

* fix: update test

* fix: update snapshots

* fix: update e2e test

* fix: etherscan link

* fix: update error test

* fix: dont show loader unless onchain processing is happening

* fix: update designs and add comments

* fix: update content in test

* fix: update tests more

* fix: test

* fix: reorganize test code

* fix: sentence case in one more test

* fix: mainnet loading indicator on last step

* fix: re-use opacity css code

* fix: testid issue with test

* fix: update copy

* fix: update strings in test

* fix: lint

* fix: modal height and css improvements

* fix: empty

* fix: padding on l2 badge

* fix: lint
parent 96d6e00e
...@@ -5,6 +5,7 @@ import { getTestSelector } from '../utils' ...@@ -5,6 +5,7 @@ import { getTestSelector } from '../utils'
/** Initiates a swap. */ /** Initiates a swap. */
function initiateSwap() { function initiateSwap() {
// The swap button is re-rendered once enable, so we must wait until the original button is not disabled to re-select the appropriate button.
cy.get('#swap-button').should('not.be.disabled') cy.get('#swap-button').should('not.be.disabled')
// Completes the swap. // Completes the swap.
cy.get('#swap-button').click() cy.get('#swap-button').click()
...@@ -63,10 +64,10 @@ describe('Permit2', () => { ...@@ -63,10 +64,10 @@ describe('Permit2', () => {
cy.spy(provider, 'send').as('permitApprovalSpy') cy.spy(provider, 'send').as('permitApprovalSpy')
}) })
initiateSwap() initiateSwap()
cy.contains('Allow trading DAI on Uniswap').should('exist') cy.contains('Enable spending limits for DAI on Uniswap').should('exist')
cy.contains('Approved').should('exist') cy.contains('Approved').should('exist')
cy.contains('Unlock DAI for swapping').should('exist') cy.contains('Allow DAI to be used for swapping').should('exist')
cy.contains('Confirm Swap').should('exist') cy.contains('Confirm Swap').should('exist')
cy.then(() => { cy.then(() => {
...@@ -92,7 +93,7 @@ describe('Permit2', () => { ...@@ -92,7 +93,7 @@ describe('Permit2', () => {
initiateSwap() initiateSwap()
// tokenApprovalStub should reject here, and the modal should revert to the review state. // tokenApprovalStub should reject here, and the modal should revert to the review state.
cy.contains('Review Swap').should('be.visible') cy.contains('Review swap').should('be.visible')
cy.then(() => { cy.then(() => {
// The user is now allowing approval, but the permit2 signature will be rejected by the user (permitApprovalStub). // The user is now allowing approval, but the permit2 signature will be rejected by the user (permitApprovalStub).
...@@ -100,11 +101,11 @@ describe('Permit2', () => { ...@@ -100,11 +101,11 @@ describe('Permit2', () => {
}) })
cy.get(getTestSelector('confirm-swap-button')).click() cy.get(getTestSelector('confirm-swap-button')).click()
cy.contains('Allow trading DAI on Uniswap').should('exist') cy.contains('Enable spending limits for DAI on Uniswap').should('exist')
cy.contains('Approved').should('exist') cy.contains('Approved').should('exist')
// permitApprovalStub should reject here, and the modal should revert to the review state. // permitApprovalStub should reject here, and the modal should revert to the review state.
cy.contains('Review Swap') cy.contains('Review swap')
.should('be.visible') .should('be.visible')
.then(() => { .then(() => {
permitApprovalStub.restore() // allow permit approval permitApprovalStub.restore() // allow permit approval
...@@ -204,7 +205,7 @@ describe('Permit2', () => { ...@@ -204,7 +205,7 @@ describe('Permit2', () => {
.then(() => { .then(() => {
initiateSwap() initiateSwap()
const approvalTime = Date.now() const approvalTime = Date.now()
cy.contains('Allow trading DAI on Uniswap').should('exist') cy.contains('Enable spending limits for DAI on Uniswap').should('exist')
cy.contains('Confirm Swap').should('exist') cy.contains('Confirm Swap').should('exist')
cy.contains('Swapped').should('exist') cy.contains('Swapped').should('exist')
......
...@@ -19,9 +19,9 @@ describe('Swap errors', () => { ...@@ -19,9 +19,9 @@ describe('Swap errors', () => {
cy.get('#swap-button').click() cy.get('#swap-button').click()
cy.get('#confirm-swap-or-send').click() cy.get('#confirm-swap-or-send').click()
cy.contains('Review Swap').should('exist') cy.contains('Review swap').should('exist')
cy.get('body').click('topRight') cy.get('body').click('topRight')
cy.contains('Review Swap').should('not.exist') cy.contains('Review swap').should('not.exist')
}) })
}) })
......
...@@ -39,7 +39,7 @@ const ConfirmedIcon = styled(ColumnCenter)<{ inline?: boolean }>` ...@@ -39,7 +39,7 @@ const ConfirmedIcon = styled(ColumnCenter)<{ inline?: boolean }>`
padding: ${({ inline }) => (inline ? '20px 0' : '32px 0;')}; padding: ${({ inline }) => (inline ? '20px 0' : '32px 0;')};
` `
export const StyledLogo = styled.img` const StyledLogo = styled.img`
height: 16px; height: 16px;
width: 16px; width: 16px;
margin-left: 6px; margin-left: 6px;
......
...@@ -18,12 +18,14 @@ import usePrevious from 'hooks/usePrevious' ...@@ -18,12 +18,14 @@ import usePrevious from 'hooks/usePrevious'
import { getPriceUpdateBasisPoints } from 'lib/utils/analytics' import { getPriceUpdateBasisPoints } from 'lib/utils/analytics'
import { useCallback, useEffect, useState } from 'react' import { useCallback, useEffect, useState } from 'react'
import { InterfaceTrade } from 'state/routing/types' import { InterfaceTrade } from 'state/routing/types'
import styled from 'styled-components/macro'
import { ThemedText } from 'theme'
import { isL2ChainId } from 'utils/chains' import { isL2ChainId } from 'utils/chains'
import { formatSwapPriceUpdatedEventProperties } from 'utils/loggingFormatters' import { formatSwapPriceUpdatedEventProperties } from 'utils/loggingFormatters'
import { didUserReject } from 'utils/swapErrorToUserReadableMessage' import { didUserReject } from 'utils/swapErrorToUserReadableMessage'
import { tradeMeaningfullyDiffers } from 'utils/tradeMeaningFullyDiffer' import { tradeMeaningfullyDiffers } from 'utils/tradeMeaningFullyDiffer'
import { ConfirmationModalContent, StyledLogo } from '../TransactionConfirmationModal' import { ConfirmationModalContent } from '../TransactionConfirmationModal'
import { PendingConfirmModalState, PendingModalContent } from './PendingModalContent' import { PendingConfirmModalState, PendingModalContent } from './PendingModalContent'
import { ErrorModalContent, PendingModalError } from './PendingModalContent/ErrorModalContent' import { ErrorModalContent, PendingModalError } from './PendingModalContent/ErrorModalContent'
import SwapModalFooter from './SwapModalFooter' import SwapModalFooter from './SwapModalFooter'
...@@ -36,6 +38,15 @@ export enum ConfirmModalState { ...@@ -36,6 +38,15 @@ export enum ConfirmModalState {
PENDING_CONFIRMATION, PENDING_CONFIRMATION,
} }
const StyledL2Badge = styled(Badge)`
padding: 6px 8px;
`
const StyledL2Logo = styled.img`
height: 16px;
width: 16px;
`
function isInApprovalPhase(confirmModalState: ConfirmModalState) { function isInApprovalPhase(confirmModalState: ConfirmModalState) {
return confirmModalState === ConfirmModalState.APPROVING_TOKEN || confirmModalState === ConfirmModalState.PERMITTING return confirmModalState === ConfirmModalState.APPROVING_TOKEN || confirmModalState === ConfirmModalState.PERMITTING
} }
...@@ -281,12 +292,12 @@ export default function ConfirmSwapModal({ ...@@ -281,12 +292,12 @@ export default function ConfirmSwapModal({
if (isL2ChainId(chainId) && confirmModalState !== ConfirmModalState.REVIEWING) { if (isL2ChainId(chainId) && confirmModalState !== ConfirmModalState.REVIEWING) {
const info = getChainInfo(chainId) const info = getChainInfo(chainId)
return ( return (
<Badge> <StyledL2Badge>
<RowFixed data-testid="confirmation-modal-chain-icon" gap="sm"> <RowFixed data-testid="confirmation-modal-chain-icon" gap="sm">
<StyledLogo src={info.logoUrl} /> <StyledL2Logo src={info.logoUrl} />
{info.label} <ThemedText.SubHeaderSmall>{info.label}</ThemedText.SubHeaderSmall>
</RowFixed> </RowFixed>
</Badge> </StyledL2Badge>
) )
} }
return undefined return undefined
...@@ -302,7 +313,7 @@ export default function ConfirmSwapModal({ ...@@ -302,7 +313,7 @@ export default function ConfirmSwapModal({
/> />
) : ( ) : (
<ConfirmationModalContent <ConfirmationModalContent
title={confirmModalState === ConfirmModalState.REVIEWING ? <Trans>Review Swap</Trans> : undefined} title={confirmModalState === ConfirmModalState.REVIEWING ? <Trans>Review swap</Trans> : undefined}
onDismiss={onModalDismiss} onDismiss={onModalDismiss}
topContent={modalHeader} topContent={modalHeader}
bottomContent={modalBottom} bottomContent={modalBottom}
......
...@@ -28,7 +28,7 @@ describe('PendingModalContent', () => { ...@@ -28,7 +28,7 @@ describe('PendingModalContent', () => {
trade={TEST_TRADE_EXACT_INPUT} trade={TEST_TRADE_EXACT_INPUT}
/> />
) )
expect(screen.getByText('Allow trading ABC on Uniswap')).toBeInTheDocument() expect(screen.getByText('Enable spending limits for ABC on Uniswap')).toBeInTheDocument()
expect(screen.getByText('Proceed in your wallet')).toBeInTheDocument() expect(screen.getByText('Proceed in your wallet')).toBeInTheDocument()
expect(screen.getByText('Why is this required?')).toBeInTheDocument() expect(screen.getByText('Why is this required?')).toBeInTheDocument()
}) })
...@@ -46,10 +46,10 @@ describe('PendingModalContent', () => { ...@@ -46,10 +46,10 @@ describe('PendingModalContent', () => {
trade={TEST_TRADE_EXACT_INPUT} trade={TEST_TRADE_EXACT_INPUT}
/> />
) )
expect(screen.getByText('Allow trading ABC on Uniswap')).toBeInTheDocument() expect(screen.getByText('Enable spending limits for ABC on Uniswap')).toBeInTheDocument()
expect(screen.getByText('Proceed in your wallet')).toBeInTheDocument() expect(screen.getByText('Proceed in your wallet')).toBeInTheDocument()
expect(screen.getByText('Why is this required?')).toBeInTheDocument() expect(screen.getByText('Why is this required?')).toBeInTheDocument()
expect(screen.queryByText('Unlock ABC for swapping')).not.toBeInTheDocument() expect(screen.queryByText('Allow ABC to be used for swapping')).not.toBeInTheDocument()
}) })
it('renders the second step with activeStepIndex=1', () => { it('renders the second step with activeStepIndex=1', () => {
...@@ -64,10 +64,10 @@ describe('PendingModalContent', () => { ...@@ -64,10 +64,10 @@ describe('PendingModalContent', () => {
trade={TEST_TRADE_EXACT_INPUT} trade={TEST_TRADE_EXACT_INPUT}
/> />
) )
expect(screen.getByText('Unlock ABC for swapping')).toBeInTheDocument() expect(screen.getByText('Allow ABC to be used for swapping')).toBeInTheDocument()
expect(screen.getByText('Proceed in your wallet')).toBeInTheDocument() expect(screen.getByText('Proceed in your wallet')).toBeInTheDocument()
expect(screen.getByText('Why is this required?')).toBeInTheDocument() expect(screen.getByText('Why is this required?')).toBeInTheDocument()
expect(screen.queryByText('Allow trading ABC on Uniswap')).not.toBeInTheDocument() expect(screen.queryByText('Enable spending limits for ABC on Uniswap')).not.toBeInTheDocument()
}) })
}) })
......
...@@ -12,6 +12,8 @@ import { useIsTransactionConfirmed } from 'state/transactions/hooks' ...@@ -12,6 +12,8 @@ import { useIsTransactionConfirmed } from 'state/transactions/hooks'
import styled, { css, keyframes } from 'styled-components/macro' import styled, { css, keyframes } from 'styled-components/macro'
import { ExternalLink } from 'theme' import { ExternalLink } from 'theme'
import { ThemedText } from 'theme/components/text' import { ThemedText } from 'theme/components/text'
import { getExplorerLink } from 'utils/getExplorerLink'
import { ExplorerDataType } from 'utils/getExplorerLink'
import { ConfirmModalState } from '../ConfirmSwapModal' import { ConfirmModalState } from '../ConfirmSwapModal'
import { import {
...@@ -113,34 +115,29 @@ interface ContentArgs { ...@@ -113,34 +115,29 @@ interface ContentArgs {
swapPending: boolean swapPending: boolean
tokenApprovalPending: boolean tokenApprovalPending: boolean
swapTxHash?: string swapTxHash?: string
chainId?: number
} }
function getContent(args: ContentArgs): PendingModalStep { function getContent(args: ContentArgs): PendingModalStep {
const { step, approvalCurrency, swapConfirmed, swapPending, tokenApprovalPending, trade } = args const { step, approvalCurrency, swapConfirmed, swapPending, tokenApprovalPending, trade, swapTxHash, chainId } = args
switch (step) { switch (step) {
case ConfirmModalState.APPROVING_TOKEN: case ConfirmModalState.APPROVING_TOKEN:
return { return {
title: t`Allow trading ${approvalCurrency?.symbol ?? 'token'} on Uniswap`, title: t`Enable spending limits for ${approvalCurrency?.symbol ?? 'this token'} on Uniswap`,
subtitle: ( subtitle: (
<> <ExternalLink href="https://support.uniswap.org/hc/en-us/articles/8120520483085">
<Trans>First, we need your permission to use your DAI for swapping.</Trans>{' '} <Trans>Why is this required?</Trans>
<ExternalLink href="https://support.uniswap.org/hc/en-us/articles/8120520483085"> </ExternalLink>
<Trans>Why is this required?</Trans>
</ExternalLink>
</>
), ),
label: tokenApprovalPending ? t`Pending...` : t`Proceed in your wallet`, label: tokenApprovalPending ? t`Pending...` : t`Proceed in your wallet`,
} }
case ConfirmModalState.PERMITTING: case ConfirmModalState.PERMITTING:
return { return {
title: t`Unlock ${approvalCurrency?.symbol ?? 'token'} for swapping`, title: t`Allow ${approvalCurrency?.symbol ?? 'this token'} to be used for swapping`,
subtitle: ( subtitle: (
<> <ExternalLink href="https://support.uniswap.org/hc/en-us/articles/8120520483085">
<Trans>This will expire after 30 days for your security.</Trans>{' '} <Trans>Why is this required?</Trans>
<ExternalLink href="https://support.uniswap.org/hc/en-us/articles/8120520483085"> </ExternalLink>
<Trans>Why is this required?</Trans>
</ExternalLink>
</>
), ),
label: t`Proceed in your wallet`, label: t`Proceed in your wallet`,
} }
...@@ -148,13 +145,17 @@ function getContent(args: ContentArgs): PendingModalStep { ...@@ -148,13 +145,17 @@ function getContent(args: ContentArgs): PendingModalStep {
return { return {
title: swapPending ? t`Transaction submitted` : swapConfirmed ? t`Success` : t`Confirm Swap`, title: swapPending ? t`Transaction submitted` : swapConfirmed ? t`Success` : t`Confirm Swap`,
subtitle: trade ? <TradeSummary trade={trade} /> : null, subtitle: trade ? <TradeSummary trade={trade} /> : null,
label: swapConfirmed ? ( label:
<ExternalLink href={`https://etherscan.io/tx/${swapConfirmed}`} color="textSecondary"> swapConfirmed && swapTxHash && chainId ? (
<Trans>View on Explorer</Trans> <ExternalLink
</ExternalLink> href={getExplorerLink(chainId, swapTxHash, ExplorerDataType.TRANSACTION)}
) : !swapPending ? ( color="textSecondary"
t`Proceed in your wallet` >
) : null, <Trans>View on Explorer</Trans>
</ExternalLink>
) : !swapPending ? (
t`Proceed in your wallet`
) : null,
} }
} }
} }
...@@ -178,6 +179,7 @@ export function PendingModalContent({ ...@@ -178,6 +179,7 @@ export function PendingModalContent({
tokenApprovalPending, tokenApprovalPending,
swapTxHash, swapTxHash,
trade, trade,
chainId,
}) })
const currentStepContainerRef = useRef<HTMLDivElement>(null) const currentStepContainerRef = useRef<HTMLDivElement>(null)
useUnmountingAnimation(currentStepContainerRef, () => AnimationType.EXITING) useUnmountingAnimation(currentStepContainerRef, () => AnimationType.EXITING)
......
...@@ -194,7 +194,7 @@ export default function SwapModalFooter({ ...@@ -194,7 +194,7 @@ export default function SwapModalFooter({
id={InterfaceElementName.CONFIRM_SWAP_BUTTON} id={InterfaceElementName.CONFIRM_SWAP_BUTTON}
> >
<ThemedText.HeadlineSmall color="accentTextLightPrimary"> <ThemedText.HeadlineSmall color="accentTextLightPrimary">
<Trans>Swap</Trans> <Trans>Confirm swap</Trans>
</ThemedText.HeadlineSmall> </ThemedText.HeadlineSmall>
</ConfirmButton> </ConfirmButton>
</TraceEvent> </TraceEvent>
......
...@@ -26,7 +26,11 @@ const ResponsiveHeadline = ({ children, ...textProps }: PropsWithChildren<TextPr ...@@ -26,7 +26,11 @@ const ResponsiveHeadline = ({ children, ...textProps }: PropsWithChildren<TextPr
return <ThemedText.HeadlineMedium {...textProps}>{children}</ThemedText.HeadlineMedium> return <ThemedText.HeadlineMedium {...textProps}>{children}</ThemedText.HeadlineMedium>
} }
return <ThemedText.HeadlineLarge {...textProps}>{children}</ThemedText.HeadlineLarge> return (
<ThemedText.HeadlineLarge fontWeight={500} {...textProps}>
{children}
</ThemedText.HeadlineLarge>
)
} }
interface AmountProps { interface AmountProps {
......
...@@ -330,7 +330,7 @@ exports[`SwapModalFooter.tsx matches base snapshot, test trade exact input 1`] = ...@@ -330,7 +330,7 @@ exports[`SwapModalFooter.tsx matches base snapshot, test trade exact input 1`] =
<div <div
class="c7 css-iapcxi" class="c7 css-iapcxi"
> >
Swap Confirm swap
</div> </div>
</button> </button>
</div> </div>
......
...@@ -145,7 +145,7 @@ exports[`SwapModalHeader.tsx matches base snapshot, test trade exact input 1`] = ...@@ -145,7 +145,7 @@ exports[`SwapModalHeader.tsx matches base snapshot, test trade exact input 1`] =
class="c5" class="c5"
> >
<div <div
class="c8 css-xdrz3" class="c8 css-z2fexy"
data-testid="INPUT-amount" data-testid="INPUT-amount"
> >
&lt;0.00001 ABC &lt;0.00001 ABC
...@@ -188,7 +188,7 @@ exports[`SwapModalHeader.tsx matches base snapshot, test trade exact input 1`] = ...@@ -188,7 +188,7 @@ exports[`SwapModalHeader.tsx matches base snapshot, test trade exact input 1`] =
class="c5" class="c5"
> >
<div <div
class="c8 css-xdrz3" class="c8 css-z2fexy"
data-testid="OUTPUT-amount" data-testid="OUTPUT-amount"
> >
&lt;0.00001 DEF &lt;0.00001 DEF
...@@ -358,7 +358,7 @@ exports[`SwapModalHeader.tsx test trade exact output, no recipient 1`] = ` ...@@ -358,7 +358,7 @@ exports[`SwapModalHeader.tsx test trade exact output, no recipient 1`] = `
class="c5" class="c5"
> >
<div <div
class="c8 css-xdrz3" class="c8 css-z2fexy"
data-testid="INPUT-amount" data-testid="INPUT-amount"
> >
&lt;0.00001 ABC &lt;0.00001 ABC
...@@ -401,7 +401,7 @@ exports[`SwapModalHeader.tsx test trade exact output, no recipient 1`] = ` ...@@ -401,7 +401,7 @@ exports[`SwapModalHeader.tsx test trade exact output, no recipient 1`] = `
class="c5" class="c5"
> >
<div <div
class="c8 css-xdrz3" class="c8 css-z2fexy"
data-testid="OUTPUT-amount" data-testid="OUTPUT-amount"
> >
&lt;0.00001 GHI &lt;0.00001 GHI
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment