Agent: use formatTokenAmount() and add rounding tooltip
#1148
Conversation
Voting: update VoteText's address rendering (aragon#1146)
|
@ECWireless Thank you for this!
I think we might want to reproduce all the changes made in #1121, because the codebase of the two apps is really similar:
Let me know if you need anything! |
- Adds formatTokenAmount() and BN.js values everywhere applicable except in useDownloadData.js.
|
@bpierre @Evalir sorry for the long delay in adding more to this; just finished moving from one side of the US to the other... But I've made a few more additions to the Agent App! This primarily includes just adding formatTokenAmount() from aragon/ui, as well as BN.js, into any appropriate files. I believe the only other file that needs updated is useDownloadData.js, although I'm having trouble testing out changes since Chrome is blocking all downloads from iFrames. Not sure if there is an easy way around this... But let me know what you think! I'll keep messing around with useDownloadData.js in the meantime. |
Ah, this is a good catch. We may need to update our |
|
On the above, this is fixed with: aragon/client#1436. |
sohkai
changed the title
formatTokenAmount() and add rounding tooltip
- formatTokenAmount() from utils - formatDecimals() from utils - ROUNDING_AMOUNT from utils - LOCALE_US_FORMAT from locales - round() from math-utils
|
@ECWireless I'm wondering if we could double check and align the I know for |
…nce and Agent app
|
@sohkai Ah, got it! I just copied over BalanceToken.js and Balances.js and everything works fine. It looks like Finance's Transfers.js and Agent's Transactions.js can't simply be copied between each other though, so I'll go through it again and try to align it a bit more. |
|
@sohkai I believe Agent's Transactions.js and Finance's Transfers.js are pretty well aligned now. I double-checked it, and all I found was a repeated 'symbol' (because of adding formatTokenAmount() from the UI). |
There was a problem hiding this comment.
Looking super good @ECWireless! We're almost there 🏁!
I noticed a few last things that I wanted to check!
| @@ -257,11 +257,9 @@ const Transactions = React.memo(function Transactions({ | |||
|
|
|||
| const { symbol, decimals } = tokenDetails[toChecksumAddress(token)] | |||
| const formattedAmount = formatTokenAmount( | |||
| amount, | |||
| isIncoming, | |||
| isIncoming ? amount : -Math.abs(amount), | |||
There was a problem hiding this comment.
Oh interesting, do we only have a BN object for amount at this moment? Otherwise, we should use amount.neg().
There was a problem hiding this comment.
Ah, I see why now.
I've updated the frontend reducers to push BNs into these transfer objects so we can transparently consume them as BNs elsewhere :).
There was a problem hiding this comment.
🙌 👌 🙏 ✅ Thanks @ECWireless, this was a great PR! Very excited to get this out!
I've just pushed a few commits to align it again with Finance and update the state to be BN-centric :).
|
Congrats on merging your first pull request! Aragon is proud of you 🦅 |
This first commit simply re-adds the reverts @bpierre made in #1121
Fixes #1157 #756 #1065 #1128.
This PR is still a work in progress. @bpierre are there any other issues I could tack onto this PR?
To do: