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.

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

feat: move token state into query params #1770

Open
wants to merge 78 commits into
base: master
Choose a base branch
from
Open

Conversation

brtkx
Copy link
Contributor

@brtkx brtkx commented Jul 19, 2024

closes FS-675

Copy link

vercel bot commented Jul 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
arbitrum-token-bridge ✅ Ready (Inspect) Visit Preview Sep 9, 2024 4:43pm

@brtkx brtkx changed the title feat: token query params feat: move token state into query params Jul 22, 2024

// We want to be sure this fetch is completed by the time we open the USDC modals
useCCTPIsBlocked()

const [tokenBridgeParams, setTokenBridgeParams] =
useState<TokenBridgeParams | null>(null)

useEffect(() => {
Copy link
Contributor Author

@brtkx brtkx Jul 22, 2024

Choose a reason for hiding this comment

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

This caused a lot of rerenders because useSelectedToken uses tokenBridge data under the hood. This block of code is not needed anyway because we set token to null when networks change

@@ -558,43 +553,9 @@ export function TokenSearch({
}

try {
// Native USDC on L2 won't have a corresponding L1 address
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this logic to useSelectedToken.

useSelectedToken now only requires erc20ParentAddress and all the computation whether it's USDC or not is handled there.

@@ -210,7 +218,7 @@ export function TransferPanel() {
)

function closeWithResetTokenImportDialog() {
setTokenQueryParam(undefined)
setTokenQueryParam(null)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's nicer to have null instead of undefined so that data is consistent with selectedToken

@@ -225,6 +233,36 @@ export function TransferPanel() {
connectionState
})

const isTokenAlreadyImported = useMemo(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checks if token is already imported before we show the token import dialog. This way when we refresh the page we won't show the dialog all the time.

import { CommonAddress } from '../../../util/CommonAddressUtils'
import { isNetwork } from '../../../util/networks'

const commonUSDC = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to useSelectedToken as the logic is handled there

parentChain.id,
childChain.id,
tokenFromSearchParams,
Object.keys(tokensFromLists),
Copy link
Member

Choose a reason for hiding this comment

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

don't think this is needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

decimals: 6,
listIds: new Set()
})
if (isTokenNativeUSDC(_token.address)) {
Copy link
Member

Choose a reason for hiding this comment

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

double check if we still need this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed


const isTestnetMode = isNetwork(networks.sourceChain.id).isTestnet

const toggleTestnetMode = useCallback(() => {
setNetworks({
sourceChainId: isTestnetMode ? ChainId.Ethereum : ChainId.Sepolia
})
}, [isTestnetMode, setNetworks])
setSelectedToken(null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have a global listener for network change, so if network is changed in a different place, we don't forget to reset the selected token?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set token query param when ERC20 token is selected
4 participants