Skip to content
This repository has been archived by the owner on Apr 15, 2019. It is now read-only.

Conform all numbers with comma - Closes #326 #339

Merged
merged 16 commits into from
Jun 12, 2017

Conversation

yasharAyari
Copy link
Contributor

@yasharAyari yasharAyari commented Jun 7, 2017

Use angular number filter in lisk and forging component to fix this issue.

Copy link
Contributor

@slaweet slaweet left a comment

Choose a reason for hiding this comment

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

We must not round LSK to two floating places. LSK has 8 significant floating places and has to be displayed with that precision (if the all the floating places are used). In the future, the price of LSK can be much higher and more floating places are then important.

Also, this PR should not contain commit from the other PR (issue #321).

@reyraa reyraa changed the title 326 conform all numbers with comma Conform all numbers with comma Jun 8, 2017
@slaweet slaweet changed the title Conform all numbers with comma Conform all numbers with comma - Closes #326 Jun 8, 2017
@@ -1,2 +1,2 @@
span(ng-bind='$ctrl.amount | lsk')
span(ng-bind='$ctrl.amount | lsk | number:8')
Copy link
Contributor

Choose a reason for hiding this comment

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

number:8 is also not the answer as we don't want to show the trailing zeros.
100.00000000 should be 100
100.12300000 should be 100.123

Copy link
Contributor

Choose a reason for hiding this comment

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

$ctrl.amount | lsk | number might work

Copy link
Contributor

@slaweet slaweet left a comment

Choose a reason for hiding this comment

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

Good job @yasharAyari

@yasharAyari yasharAyari merged commit 752dcf4 into development Jun 12, 2017
@reyraa reyraa deleted the 326-conform-all-numbers-with-comma branch June 12, 2017 15:48
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.

4 participants