-
Notifications
You must be signed in to change notification settings - Fork 2
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
#1694 Add commas to numbers #1698
Conversation
Visit the preview URL for this PR (updated for commit e777b3c): https://jac-admin-develop--pr1698-feature-1694-add-com-av2eb7do.web.app (expires Thu, 01 Sep 2022 11:58:06 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
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.
It's good to try and keep our template code as simple and robust as possible.
With that in mind could we update these changes to use a filter instead, along the lines of:
{{ diversity.totalApplications | formatNumber }}
instead of
{{ diversity.totalApplications.toLocaleString('en-GB') }}
Within the formatNumber
filter we can then check the value exists before we do anything to it (avoiding possible errors if data doesn't exist). Plus moving forward we will have one place to make changes, should we need to change anything.
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.
@HalcyonJAC excellent this is much nicer... and units tests too 👍
What's included?
Add commas to numbers so they are easier to read, e.g.,
1000 -> 1,000
.This will be anywhere we display numbers on the platform:
Who should test?
✅ Product owner
✅ Developers
✅ UTG
How to test?
Go to the admin platform and check number display
Risk - how likely is this to impact other areas?
🟢 No risk - this is a self-contained piece of work
Additional context
Demo:
1694.Add.commas.to.numbers.mov
Related permissions
PREVIEW:DEVELOP
can be OFF, DEVELOP or STAGING