-
Notifications
You must be signed in to change notification settings - Fork 212
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
Finance: Thousands Separators #1081
Finance: Thousands Separators #1081
Conversation
apps/finance/app/src/lib/utils.js
Outdated
|
||
// Thousands Separator | ||
export function separateThousands(x) { | ||
return x.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ","); |
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.
This doesn't work if the string has a decimal, e.g. ('1.23456'
).
We should do the replace on only the whole number ahead of the decimal.
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.
Okay, I just made a new commit that should fix this.
Actually @ECWireless, @bpierre and I were discussing this, and realized from Ethers that the regex may not work well everywhere. We started talking about including some formatting utilities in aragonUI instead to do this: https://github.com/aragon/aragon-ui/issues/719 |
Let's keep it here and update it when a new aragonUI version is released with the utility. |
@sohkai Is there a timeline for when the new AragonUI version is supposed to be released? It looks like a new one is released roughly every 2 months, but figured I'd ask... |
Haha yes... we have not been very good at making incremental releases lately but we've included a bunch of updates to aragonUI and will be releasing soon. @bpierre and I were discussing the formatting API on Friday, so we might wait just a bit longer on this—let's see. |
Closing in favor of #1145, since this PR uses a forked repo that has been deleted. |
Fixing issue #1065