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

Use user locale instead of default locale to format currencies #2443

Merged
merged 3 commits into from
Mar 8, 2018

Conversation

bertocq
Copy link
Collaborator

@bertocq bertocq commented Feb 6, 2018

Where

What

All currencies are formatted using the default locale language (I18n.default_locale) instead of the user selected language. We need to use the user language to format currencies

How

  • Changing I18n.default_locale for I18n.locale
  • Unifying currency format logic into Budget#formatted_price method

Screenshots

Before

screen shot 2018-02-06 at 11 11 07

After

screen shot 2018-02-06 at 11 20 15

Notes

None

ApplicationHelper#format_price and Budget#formatted_amount has the same
objective and code, but the Budget#formatted_amount method also uses the
currency of the Budget to correctly give currencies format.

By replacing usage of format_price with formatted_amount we can remove
format_price and have a single location for currency format logic.
@bertocq bertocq changed the title [WIP] Use user locale instead of default locale to format currencies Use user locale instead of default locale to format currencies Mar 8, 2018
Copy link
Member

@voodoorai2000 voodoorai2000 left a comment

Choose a reason for hiding this comment

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

Awesome!

@bertocq
Copy link
Collaborator Author

bertocq commented Mar 8, 2018

Checked at Madrid's fork 👌 production server

@bertocq bertocq merged commit a55218a into master Mar 8, 2018
@bertocq bertocq deleted the fix_currency_locale branch March 8, 2018 17:03
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.

None yet

2 participants