Skip to content
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

Consultation's budget amounts formatting #143

Merged
merged 2 commits into from
Dec 12, 2016

Conversation

danguita
Copy link
Contributor

@danguita danguita commented Dec 12, 2016

Connects to #119.

What does this PR do?

This PR normalizes the budget amounts format across different sections on the UI, taking care of both server and client sides.

In the server side we're using a helper method to wrap the number formatting, while in client side we're propagating the same format options through a numeric format library which is included as a GobiertoBudgetConsultations specific vendor.

How should this be manually tested?

Let's check that any budget amount in the Consultation response process match the following assertions:

assert_equal "124000", budget_amount_to_human(123567)
assert_equal "123000", budget_amount_to_human(123000)
assert_equal "123 M", budget_amount_to_human(123000000)
assert_equal "1.24 M", budget_amount_to_human(1240000)

screen shot 2016-12-12 at 12 38 18 pm

Final notes

There would be slight differences between the server and client-side number formatting, although I would not care about them since there's an enhancement that will drop any repeated logic at #133.

- Take advantage of the Numeral-js library, which is loaded as a
  specific `GobiertoBudgetConsultations` vendor.
@ferblape
Copy link
Member

👍 Works OK
👍 I agree with your thoughts, no need to worry about that now

Merging!

@ferblape ferblape merged commit 6d9a43b into master Dec 12, 2016
@ferblape ferblape deleted the 119-consultation-budget-line-amount-formatting branch December 12, 2016 14:50
@danguita
Copy link
Contributor Author

Cool, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants