feat(assets): add token details screen #4266
Conversation
|
just from the videos-
very cool though! |
| text: t('tokenDetails.actions.send'), | ||
| iconComponent: QuickActionsSend, | ||
| onPress: () => { | ||
| navigate(Screens.Send, { defaultTokenOverride: token.address! }) |
There was a problem hiding this comment.
I recently saw a refactor here by @MuckT that changes the meaning of defaultTokenOverride from address to tokenId, but the type is still string. Some faint alarm bell went off when I saw that, and I'm realizing now that I was worried that one of you could merge the changes without seeing this change and we'd end up with a bug. @MuckT what do you think should be done to prevent this? Maybe the defaultTokenOverride parameter name could be changed (in Tom's PR) so we get a compilation error if someone makes this mistake
There was a problem hiding this comment.
Happy to change it from defaultTokenOverride to defaultTokenIdOverride in #4242.
There was a problem hiding this comment.
Left a TODO, so this can be addressed depending on which one merges first
There was a problem hiding this comment.
I think Tom's suggestion of changing the variable name is safer, since it doesn't depend on merge order-- in either case, a build error should take place if one is merged without the other taking the changes into account yet
src/tokens/TokenDetails.tsx
Outdated
| iconComponent: QuickActionsAdd, | ||
| onPress: () => { | ||
| navigate(Screens.FiatExchangeAmount, { | ||
| currency: token.symbol as CiCoCurrency, |
There was a problem hiding this comment.
this type cast seems to rely on the internal logic of useCashInTokens, which could change and cause a bug here...
There was a problem hiding this comment.
same with the following one
There was a problem hiding this comment.
added an explicit condition on the navigate function
|
Codecov Report
@@ Coverage Diff @@
## main #4266 +/- ##
==========================================
+ Coverage 83.96% 83.99% +0.03%
==========================================
Files 705 711 +6
Lines 25966 26131 +165
Branches 3361 3385 +24
==========================================
+ Hits 21803 21950 +147
- Misses 4099 4117 +18
Partials 64 64
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Description
Figma
Test plan
Unit tests, manual by enabling dummy feature flag
Showing all actions with swap enabled:
asset-details.mp4
Showing actions with swap disabled (withdraw is hidden behind more):
asset-details-no-swap.mp4
Related issues
Backwards compatibility
Yes