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

Move Icelandic symbol after the digits #929

Merged
merged 4 commits into from May 10, 2020

Conversation

viktorsmari
Copy link
Contributor

@viktorsmari viktorsmari commented May 6, 2020

In Rails number_to_currency helper the kr symbol comes after the digits.

I might be wrong, but most of the major Icelandic online sites have the "kr." symbol after the price.

I am Icelandic and it looks odd having the kr123 vs 123 kr.

ActionController::Base.helpers.number_to_currency(12345, locale: :is)
=> "12.345 kr."

Money.from_amount(12345, 'ISK').format
=> "kr12,345"

From the rails-i18n we can see the formatting is 'number unit':
format: "%n %u"
https://github.com/svenfuchs/rails-i18n/blob/master/rails/locale/is.yml

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0002%) to 99.965% when pulling e530178 on viktorsmari:patch-1 into e6aa20e on RubyMoney:master.

@coveralls
Copy link

coveralls commented May 6, 2020

Coverage Status

Coverage remained the same at 99.965% when pulling d7713b5 on viktorsmari:patch-1 into 2efd45f on RubyMoney:master.

@antstorm
Copy link
Contributor

antstorm commented May 9, 2020

@viktorsmari this PR makes sense to me, can you please fix the specs so I can merge it?

Copy link
Contributor

@antstorm antstorm left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@antstorm antstorm merged commit 265de50 into RubyMoney:master May 10, 2020
@viktorsmari viktorsmari deleted the patch-1 branch November 26, 2021 22:47
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

3 participants