Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: pull header & wallet components out of Swap component #123

Open
wants to merge 3 commits into
base: wallet-connect-flow
Choose a base branch
from
Open

refactor: pull header & wallet components out of Swap component #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
740de3d
feat: refactor header and wallet connection
kristiehuang Aug 3, 2022
d55db52
use title for header title
kristiehuang Aug 3, 2022
02dfaa8
don't use title prop
kristiehuang Aug 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 31 additions & 6 deletions src/components/Header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
import { Trans } from '@lingui/macro'
import { useWeb3React } from '@web3-react/core'
import useOnSupportedNetwork from 'hooks/useOnSupportedNetwork'
import { largeIconCss } from 'icons'
import { ReactElement, ReactNode } from 'react'
import { useAtom } from 'jotai'
import { useEffect } from 'react'
import { onConnectWalletClickAtom } from 'state/swap'
import styled from 'styled-components/macro'
import { ThemedText } from 'theme'

import Wallet from './ConnectWallet'
import Row from './Row'
import Settings from './Swap/Settings'

const HeaderRow = styled(Row)`
height: 1.75em;
Expand All @@ -13,15 +20,33 @@ const HeaderRow = styled(Row)`
`

export interface HeaderProps {
title?: ReactElement
children: ReactNode
hideConnectionUI?: boolean
onConnectWalletClick?: () => void | Promise<boolean>
}

export default function Header({ title, children }: HeaderProps) {
export default function Header(props: HeaderProps) {
const { isActive } = useWeb3React()
const onSupportedNetwork = useOnSupportedNetwork()
const isDisabled = !(isActive && onSupportedNetwork)

const [onConnectWalletClick, setOnConnectWalletClick] = useAtom(onConnectWalletClickAtom)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to set this without referencing the already-set value:

const setOnConnectWalletClick = useSetAtom(onConnectWalletClickAtom)
useEffect(() => setOnConnectWalletClick(props.onConnectWalletClick), [props.onConnectWalletClick, setOnConnectWalletClick])

useEffect(() => {
if (props.onConnectWalletClick !== onConnectWalletClick) {
setOnConnectWalletClick((old: (() => void | Promise<boolean>) | undefined) => (old = props.onConnectWalletClick))
}
}, [props.onConnectWalletClick, onConnectWalletClick, setOnConnectWalletClick])

return (
<HeaderRow iconSize={1.2}>
<Row gap={0.5}>{title && <ThemedText.Subhead1>{title}</ThemedText.Subhead1>}</Row>
<Row gap={1}>{children}</Row>
<Row gap={0.5}>
<ThemedText.Subhead1>
<Trans>Swap</Trans>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should widgets expand past Swap, this will need to be configurable.
Can you keep passing title into the header?

</ThemedText.Subhead1>
</Row>
<Row gap={1}>
<Wallet disabled={props.hideConnectionUI} />
<Settings disabled={isDisabled} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Settings is specific to the Swap widget (similar to the title) - this either needs to be a prop, or a callback to activate the Swap's settings.

</Row>
</HeaderRow>
)
}
26 changes: 2 additions & 24 deletions src/components/Swap/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,20 @@
import { JsonRpcProvider } from '@ethersproject/providers'
import { Trans } from '@lingui/macro'
import { useWeb3React } from '@web3-react/core'
import { Provider as Eip1193Provider } from '@web3-react/types'
import Wallet from 'components/ConnectWallet'
import { SwapInfoProvider } from 'hooks/swap/useSwapInfo'
import useSyncConvenienceFee, { FeeOptions } from 'hooks/swap/useSyncConvenienceFee'
import useSyncTokenDefaults, { TokenDefaults } from 'hooks/swap/useSyncTokenDefaults'
import { usePendingTransactions } from 'hooks/transactions'
import useHasFocus from 'hooks/useHasFocus'
import useOnSupportedNetwork from 'hooks/useOnSupportedNetwork'
import { useAtom } from 'jotai'
import { useEffect, useState } from 'react'
import { displayTxHashAtom, onConnectWalletClickAtom } from 'state/swap'
import { useState } from 'react'
import { displayTxHashAtom } from 'state/swap'
import { SwapTransactionInfo, Transaction, TransactionType, WrapTransactionInfo } from 'state/transactions'

import Dialog from '../Dialog'
import Header from '../Header'
import { BoundaryProvider } from '../Popover'
import Input from './Input'
import Output from './Output'
import ReverseButton from './ReverseButton'
import Settings from './Settings'
import { StatusDialog } from './Status'
import SwapButton from './SwapButton'
import Toolbar from './Toolbar'
Expand All @@ -42,13 +36,8 @@ function getTransactionFromMap(
return
}

// SwapProps also currently includes props needed for wallet connection, since the wallet connection component exists within the Swap component
// TODO(kristiehuang): refactor WalletConnection outside of Swap component
export interface SwapProps extends TokenDefaults, FeeOptions {
hideConnectionUI?: boolean
provider?: Eip1193Provider | JsonRpcProvider
routerUrl?: string
onConnectWalletClick?: () => void | Promise<boolean>
}

export default function Swap(props: SwapProps) {
Expand All @@ -68,19 +57,8 @@ export default function Swap(props: SwapProps) {

const focused = useHasFocus(wrapper)

const [onConnectWalletClick, setOnConnectWalletClick] = useAtom(onConnectWalletClickAtom)
useEffect(() => {
if (props.onConnectWalletClick !== onConnectWalletClick) {
setOnConnectWalletClick((old: (() => void | Promise<boolean>) | undefined) => (old = props.onConnectWalletClick))
}
}, [props.onConnectWalletClick, onConnectWalletClick, setOnConnectWalletClick])

return (
<>
<Header title={<Trans>Swap</Trans>}>
<Wallet disabled={props.hideConnectionUI} />
<Settings disabled={isDisabled} />
</Header>
<div ref={setWrapper}>
<BoundaryProvider value={wrapper}>
<SwapInfoProvider disabled={isDisabled} routerUrl={props.routerUrl}>
Expand Down
4 changes: 3 additions & 1 deletion src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'polyfills'

import Header, { HeaderProps } from 'components/Header'
import Swap, { SwapProps } from 'components/Swap'
import Widget, { WidgetProps } from 'components/Widget'
export type { Provider as EthersProvider } from '@ethersproject/abstract-provider'
Expand All @@ -14,11 +15,12 @@ export type { DefaultAddress, TokenDefaults } from 'hooks/swap/useSyncTokenDefau
export type { Theme } from 'theme'
export { darkTheme, defaultTheme, lightTheme } from 'theme'

export type SwapWidgetProps = SwapProps & WidgetProps
export type SwapWidgetProps = HeaderProps & SwapProps & WidgetProps
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an interface extending the existing props:

export interface SwapWidgetProps extends HeaderProps, SwapProps, WidgetProps {}

This is slightly easier on the type system: it reduces time for type checking and it surfaces better types downstream.


export function SwapWidget(props: SwapWidgetProps) {
return (
<Widget {...props}>
<Header {...props} />
<Swap {...props} />
</Widget>
)
Expand Down