Skip to content
This repository has been archived by the owner on May 24, 2022. It is now read-only.

fix: Fixes #244. Concatenate balances to 2 decimal places on account overview page without rounding #291

Merged
merged 5 commits into from
Dec 19, 2018

Conversation

ltfschoen
Copy link
Contributor

With these changes, given an account with 0.999432 ETH and 1233.8945 TIB.

Previously the account summary page would show:
Ether: 1.00
THIBCoin: 1233.89
Now it shows:
Ether: 0.99
THIBCoin: 1233.89

Previously the Send Ether page would show: 0.999432
Now it shows: 0.999432

Previously the Send THIBCoin page would show: 1233.894500
Now it shows: 1233.8945

@ltfschoen
Copy link
Contributor Author

Note that another approach could be to use BigNumber.js rounding http://mikemcl.github.io/bignumber.js/#constructor-properties

@@ -35,7 +43,9 @@ export const TokenCard = ({
</div>
<div className='token_balance'>
{balance ? (
<span>{balance.toFixed(decimals)} </span>
<span>
{concatBalanceByDecimalPlaces(balance.toString(), decimals)}{' '}
Copy link
Collaborator

@amaury1093 amaury1093 Dec 15, 2018

Choose a reason for hiding this comment

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

I think .toFixed(decimals, 1) does exactly this, 1 == ROUND_DOWN

Edit: or .dp(decimals, 1)

Copy link
Collaborator

Choose a reason for hiding this comment

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

ping @ltfschoen Have you had a look at this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amaurymartiny i'll take a look now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amaurymartiny yes, works great!

@amaury1093 amaury1093 merged commit 5e6165d into master Dec 19, 2018
@amaury1093 amaury1093 deleted the luke-244-inconsistent-dp branch December 19, 2018 13:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants