-
Notifications
You must be signed in to change notification settings - Fork 436
feat: add Add to wallet button, show USD amounts and fix small-number rounding #2631
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
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…apping atokens or collateral
? isEthereum | ||
? `a${addToken.symbol}` | ||
: getATokenSymbol(networkConfigs[chainId].name, addToken.symbol) | ||
: addToken?.symbol ?? ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mgrabina I have fixed it so that we are showing only for aToken swaps. I also created this helper to define the correct token symbol for the aToken depending on each market. It is working on the Base market and makes adding tokens seamless, but I’m not able to test on mainnet and other markets as I don’t have funds, and using a fork is not working.
Before approving this, we need to test that the correct aToken symbol is created in different markets to avoid failures when adding the token to the wallet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm i think is safer to just use ModalType instead of inferring from the token symbol that cannot be standarized
@mgrabina the margins + paddings are now fixed Normal Swap, no collateral![]() ![]() ![]() |
? isEthereum | ||
? `a${addToken.symbol}` | ||
: getATokenSymbol(networkConfigs[chainId].name, addToken.symbol) | ||
: addToken?.symbol ?? ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm i think is safer to just use ModalType instead of inferring from the token symbol that cannot be standarized
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
General Changes
Closes ALE-11 : Improved the UX of SwitchTxSuccessView.tsx by adding USD values as a subheader.
Closes ALE-13 : Added an “Add to wallet” button to integrate the received token into the user’s wallet.
Closes ALE-12 : Now we don’t round small numbers. If the number is smaller than 4 decimals, we will show <0.00001.
Developer Notes
a{nameOfChainAbbreviation}{tokenName}
. This causes a conflict when sending the RPC to the wallet, since the parameters we have only include the symbol of the normal aToken, not the market-specific one.Questions:
a{nameOfChainAbbreviation}{tokenName}
, or should this only be done in the case of underlying tokens?Reviewer Checklist
Please ensure you, as the reviewer(s), have gone through this checklist to ensure that the code changes are ready to ship safely and to help mitigate any downstream issues that may occur.
.env.example
file as well as the pertinant.github/actions/*
files