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

Fix android number format #24

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

t1sh0o
Copy link

@t1sh0o t1sh0o commented Jul 21, 2020

It seems that the currency should be set on the DecimalFormatSymbols instance before setting it to the NumberFormat instance in order to display proper currency.

With the old implementation, we had the following results, from which the first is ok, but the second is wrong:

const bgFormat = new Intl.NumberFormat('bg-BG', {
	style: 'currency',
	currency: 'BGN',
});
const usFormat = new Intl.NumberFormat('en-US', {
	style: 'currency',
	currency: 'BGN',
});
console.log(bgFormat.format(123.9)); // 123,90 лв.
console.log(usFormat.format(123.9)); // $123.90

And with the new one, the second call results in BGN123.90 which seems to be the correct result.

Fixes #23

It seems that the currency should be set on the DecimalFormatSymbols instance before setting it to the NumberFormat instance in order to display proper currency.
@cla-bot
Copy link

cla-bot bot commented Jul 21, 2020

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla.
CLA has not been signed by users: @t1sh0o.
After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

@t1sh0o
Copy link
Author

t1sh0o commented Jul 21, 2020

@cla-bot check

@cla-bot cla-bot bot added the cla: yes label Jul 21, 2020
@cla-bot
Copy link

cla-bot bot commented Jul 21, 2020

The cla-bot has been summoned, and re-checked this pull request!

@StefanNedelchev StefanNedelchev mentioned this pull request Aug 3, 2020
@wendt88
Copy link

wendt88 commented Feb 4, 2021

@NathanWalker can you merge this PR please? thx

canmertc pushed a commit to canmertc/nativescript-intl that referenced this pull request Mar 25, 2024
(perf) move instance creation to the constructor.
(feat) expose native formatter instance.
(fix) merge NativeScript#24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wrong currency
3 participants