-
Notifications
You must be signed in to change notification settings - Fork 840
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(wallet): Portfolio Asset Routing #17544
Conversation
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.
Can we take the opportunity to normalize routes for all assets? There are some tokens that are deployed to the same address (using CREATE2
) across networks that'll suffer from similar bugs.
This is the format I propose:
/crypto/portfolio/0xa/ETH -> optimism eth
/crypto/portfolio/0xa/0x94b008aa00579c1307b0ef2c499ad98a8ce58e58 -> optimism USDT
/crypto/portfolio/0xa/USDT -> optimism USDT
A Storybook has been deployed to preview UI for the latest push |
2e5f568
to
a18c2a8
Compare
A Storybook has been deployed to preview UI for the latest push |
a18c2a8
to
f46a21e
Compare
A Storybook has been deployed to preview UI for the latest push |
Updated the routes to work as so:
|
components/brave_wallet_ui/components/desktop/views/portfolio/portfolio-asset.tsx
Outdated
Show resolved
Hide resolved
components/brave_wallet_ui/components/extension/assets-panel/index.tsx
Outdated
Show resolved
Hide resolved
components/brave_wallet_ui/components/desktop/views/portfolio/portfolio-asset.tsx
Outdated
Show resolved
Hide resolved
components/brave_wallet_ui/components/desktop/views/portfolio/portfolio-asset.tsx
Outdated
Show resolved
Hide resolved
f46a21e
to
3ae9e95
Compare
A Storybook has been deployed to preview UI for the latest push |
Description
We will now include
chainId
as aparam
when selectingany
assets from thePortfolio
page.Ethereum
onOptimism
orAurora
would selectEthereum
onEthereum Mainnet
Symbol
as aParam
Routes
are now normalized for the following situationscrypto/portfolio/chainId/symbol
crypto/portfolio/chainId/contractAddress
crypto/portfolio/chaindId/contractAddress/tokenId
crypto/market/symbol
(chainId and contractAddress are not available in this situation.)Resolves brave/brave-browser#28976
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run lint
,npm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
Portfolio
page and selectETH on Optimism
, it should loadETH on Optimism
on the asset details page.Portfolio
page and selectETH on Aurora
, it should loadETH on Aurora
on the asset details page.Panel
and clickView account assets
, select a token. It route correctly to the assets details page.Before:
AssetDetailsBefore.mov
After:
Screen.Recording.12.mov