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

fix: reset trade completely if default token is duplicated #5140

Merged
merged 2 commits into from
Dec 2, 2024

Conversation

shoom3301
Copy link
Collaborator

@shoom3301 shoom3301 commented Dec 2, 2024

Summary

By default we use token symbols in URL.
But in some cases we can have more than one token with the same symbol.
In this case if we meet a symbol of that token we do:

  • Display "There is more than one token in the list of tokens with the symbol..." alert
  • Reset the widget state to default assets
    It works perfectly with everything but WETH :) Because WETH is a default asset and here we go in a trouble.
    To avoid that now we check if the token with duplicated symbol is WETH, then we completely clean up the widget assets state.

Fixes #4323

To Test

  1. Open /11155111/swap/WTH (Sepolia)
  2. Add 0x7b79995e5f793A07Bc00c21412e50Ecae098E7f9 (symbol=WETH) as a custom token and choose it as buy token
  3. You should see "There is more than one token in the list of tokens with the symbol..." alert
  • AR: the widget becomes frozen, you cannot pick any token
  • ER: the widget should reset assets and then you can pick any token to trade

Copy link

vercel bot commented Dec 2, 2024

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

Name Status Preview Updated (UTC)
cosmos ✅ Ready (Inspect) Visit Preview Dec 2, 2024 1:35pm
cowfi ✅ Ready (Inspect) Visit Preview Dec 2, 2024 1:35pm
explorer-dev ✅ Ready (Inspect) Visit Preview Dec 2, 2024 1:35pm
swap-dev ✅ Ready (Inspect) Visit Preview Dec 2, 2024 1:35pm
widget-configurator ✅ Ready (Inspect) Visit Preview Dec 2, 2024 1:35pm

Copy link
Contributor

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

Thank you for returning back this functionality!
Notes:

  1. the modal does not appear inside Safe, but it was never displayed there before.
  2. still, it would be nice to enhance the modal UI one day :)

@shoom3301 shoom3301 merged commit 32e1ca0 into develop Dec 2, 2024
11 of 12 checks passed
@shoom3301 shoom3301 deleted the fix/double-default-toekn branch December 2, 2024 15:50
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2024
Comment on lines 62 to -57
}

return () => {
if (timeoutId) {
clearTimeout(timeoutId)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we need to clear the timeout when the hook umounts?

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

Successfully merging this pull request may close these issues.

No warning when add token with duplicating name
3 participants