Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Incorrect Currency & Number Format for es-us Locale #8931

Closed
mgallag opened this issue Sep 4, 2014 · 7 comments
Closed

Incorrect Currency & Number Format for es-us Locale #8931

mgallag opened this issue Sep 4, 2014 · 7 comments
Assignees
@caitp
Milestone
1.3.0-rc.2

Comments

@mgallag
Copy link
Contributor

mgallag commented Sep 4, 2014

The en-us locale configuration incorrectly uses a comma for number formatting, € as a default currency symbol, places the currency symbol after the amount and uses a decimal as the group separator. Number and currency rules should be similar to en-us locale rules.

<!doctype html>
<html lang="es">
<head>
  <meta charset="UTF-8">
  <title>Incorrect Number and Currency Format: es-us</title>
  <script src="//code.angularjs.org/1.2.23/angular.js"></script>
  <script src="//code.angularjs.org/1.2.23/i18n/angular-locale_es-us.js"></script>
</head>
<body ng-app="myApp"> 
  <script>
    angular.module('myApp', ['ngLocale'])
      .controller('MyCtrl', ['$scope', '$locale', function($scope, $locale) {
        $scope.localeId = $locale.id;
        $scope.amount = 1234.56;
      }]);
  </script>
  <ul ng-controller="MyCtrl">
    <li>{{localeId}}</li>
    <li>{{amount | currency}}</li>
    <li>{{amount | number : 2}}</li>
  </ul>
</body>
</html>

Expected:

  • es-us
  • $1,234.56
  • 1,234.56

Actual:

  • es-us
  • 1.234,56 €
  • 1.234,56
@dfalgout
Copy link

dfalgout commented Sep 6, 2014

I submitted a PR #8965 that would correct this issue.

@dfalgout dfalgout mentioned this issue Sep 6, 2014
Closed
@lgalfaso
Copy link
Contributor

lgalfaso commented Sep 6, 2014

@mgallag I think you are right on the use of $ and not €, and on the positioning of the currency symbol. Now, I do think that the current decimal and group delimiters are right, and so does the CLDR

@mgallag
Copy link
Contributor Author

mgallag commented Sep 8, 2014

@lgalfaso The number formatting that you describe sounds correct for es-es but not es-us. I live in the US and although I am not a spanish speaker, seeing a comma used in an US dollar amount where the decimal point should be seems like it could be misleading.

If you dig through the data in the latest CLDR release (see links below) you will see that the group separator for the es-US locale is , and the decimal symbol is ..

http://cldr.unicode.org/index/downloads/cldr-25
Direct download: http://unicode.org/Public/cldr/25/json.zip

@mgallag mgallag mentioned this issue Sep 10, 2014
Closed
@mgallag
Copy link
Contributor Author

mgallag commented Sep 10, 2014

Opened PR #9013 that updates the Closure Slurper script to pull down missing i18n number format rules.

It solves for the currency symbol in this issue but not for the group separator or decimal point. These are configured (IMHO incorrectly) in the Closure Library project. I will open an issue to track this in the Closure Library repo. Once resolved, we will need to regenerate the Angular locale files to pick up the change.

The PR also added and updated many locale files in Angular which I believe may have had similar issues.

@caitp
Copy link
Contributor

caitp commented Sep 10, 2014

I think there are already 3 different PRs fixing this, if not more.

@mgallag, The thing is, these scripts are automatically generated, so the appropriate thing to do is to fix the scripts which generate them, not the locale sources themselves.

However, I've said we probably should land these fixes because we don't run the generator scripts very frequently --- I haven't heard anyone elses opinion on this however.

@mgallag
Copy link
Contributor Author

mgallag commented Sep 10, 2014

@caitp I did fix the generator scripts, then I regenerated them. Please check the PR.

@caitp
Copy link
Contributor

caitp commented Sep 10, 2014

Yes, I see you're right, that puts it above the other fixes. Unfortunately the diff is likely to crash browsers whenever we try to look at it =)

Anyways, I think we can merge that, but please: squash the commits (you want 2 commits: the updated generators and the regenerated locales), and you also need to update the commit message to follow the rules in https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit

mgallag pushed a commit to mgallag/angular.js that referenced this issue Sep 10, 2014
fix(ngLocale): Regenerate Locale Files
287e8e0 
Fixes number formatting and symbols for many locales.
Adds support for additional locales.

Closes angular#8931
Closes angular#8583
Closees angular#7799
mgallag pushed a commit to mgallag/angular.js that referenced this issue Sep 10, 2014
fix(ngLocale): Regenerate Locale Files
3e5615a 
Fixes number formatting and symbols for many locales.
Adds support for additional locales.

Closes angular#8931
Closes angular#8583
Closes angular#7799
@btford btford added this to the 1.3.0-rc.2 milestone Sep 10, 2014
mgallag added a commit to mgallag/angular.js that referenced this issue Sep 10, 2014
@mgallag
fix(ngLocale): Regenerate Locale Files
9e5ba41 
Fixes number formatting and symbols for many locales.
Adds support for additional locales.

Closes angular#8931
Closes angular#8583
Closes angular#7799
@caitp caitp closed this as completed in 6a96a82 Sep 11, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@caitp caitp

Labels
None yet
Projects
None yet
Milestone
1.3.0-rc.2
Development

Successfully merging a pull request may close this issue.

style(ngLocale): change es-us lcoale currency format
5 participants
@lgalfaso @btford @caitp @mgallag @dfalgout