angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.81k stars 27.5k forks source link

currency filter on negative value not in parentheses - AngularJS 1.3 to 1.4 regression #12870

Open wbeange opened 9 years ago

wbeange commented 9 years ago

The currency filter now formats negative numbers with a negative sign, where it previously wrapped the negative number in brackets.

AngularJS 1.3 currency filter: $scope.filteredNumber = $filter('currency')("-4.962016"); results in ($4.96) http://plnkr.co/edit/iEkkjw?p=preview

AngularJS 1.4 currency filter: $scope.filteredNumber = $filter('currency')("-4.962016"); results in -$4.96 http://plnkr.co/edit/3dwKuIHTk9tzGOdZ7Xly?p=preview

Narretz commented 9 years ago

How's that a regression? It seems to be that this is now the correct behavior.

wbeange commented 9 years ago

Hey @Narretz thanks for chiming in:

  1. I'd argue that best practices for accounting or finance or anything currency related would be to represent negative numbers in parentheses.
  2. I've called it a regression because after migrating to 1.4 from 1.3 the currency filter has changed behaviour as it previously had. If you don't think that's worded properly I can change the bug title.
Narretz commented 9 years ago

I guess we understood this as a bug fix; if you think the behavior was correct, it's a regression for you. I've personally never seen parentheses standing for negative values, but then again I have no idea about accounting or finance.

m-amr commented 9 years ago

@wbeange @Narretz I think adding a negative sign or a parentheses is related to the currency itself and the type of the user i think the best solution is make it optional so the developer can take this decision based on his web-application users type because if you are not accountant parentheses will not be a negative for you.

if we add one more parameter to currency filter called negativeFormat and can have value of '()' or '-' {{ currency_expression | currency : symbol : fractionSize: negativeFormat : format}}

In Microsoft office you can choose how to format negative currencies https://support.office.com/en-us/article/Format-numbers-as-currency-0a03bb38-1a07-458d-9e30-2b54366bc7a4

vladlep commented 8 years ago

Hi,

In my non-expert opinion, i prefer the no-parentheses representation or supporting. I understand that parentheses are used for accountancy but for normal users and user cases(online shops, banks, etc) i think showing them like that is strange.

There are quite a few people on stackoverflow interested in this and ways to go around it: stackoveflow Also, i think issue is related: https://github.com/angular/angular.js/issues/10158

bskendig commented 8 years ago

Negative currency values, in US English at least, are typically displayed in parentheses instead of with a minus sign. Microsoft Excel also displays negative currency values in parentheses. It looks like the Angular 1.3 behavior was correct, and the 1.4 behavior is incorrect. See: http://ux.stackexchange.com/questions/1869/preferred-format-to-display-negative-currency-us-english/1875

bskendig commented 8 years ago

I believe the fix is to change negPre and negSuf on lines 132 and 133 of this file: https://github.com/angular/angular.js/blob/master/src/ngLocale/angular-locale_en-us.js

The correct values are:

    "negPre": "(\u00a4",
    "negSuf": ")",

This is the commit which changed them: https://github.com/angular/angular.js/commit/0eb2c2af82d09f8494e44952a0a385ce7584fb4c#diff-d98b285553cb59da938bdb7383f56dd0

gkalpak commented 8 years ago

While parenthesis might we appropriate for some specific contexts it's probably not for all (and definitely not in all locales). Angular.js uses the Google Closure I18N library, to generate its own I18N files and there is nothing about a special negative pattern using parenthesis there. (If there were, Angular would pick it up.)

As already mentioned, using parenthesis can be confusing in many users and apps, so it's not a good default imo.

If it is appropriate for a secific app, one could:

  1. Serve a modified version of the locale file (having updated negPre and negSuf according to their needs). Pros: One can target specific locales Cons: One needs to re-apply the modification to use the localization files of a newer Angular version.
  2. Use a custom currency filter (possibly one that wraps the built-in currencyFilter), especially if the behavior should not be locale-specific. Pros: It's easy to apply the change to all locales (if it's appropriate for their usecase). Cons: More code (unless one decides it suffices to wrap the built-in currencyFilter and replace - with (...)).
  3. Overwrite negPre/negSuf in their app (at runtime). E.g.

    .run(function ($locale) {
     var currencyPatterns = $locale.NUMBER_FORMATS.PATTERNS[1];
     patterns.negPre = '(\u00a4';
     patterns.negSuf = ')';
    })

    Pros: Less code. Easy to apply to all locales (but not necessary). Cons: Relying on undocumented properties of $locale. It might break in the future without notice.

DaBlick commented 8 years ago

I think the clear feedback here is that there should be an option to specify which behavior you get.

coderbydesign commented 8 years ago

I agree that this should be an option. In many business contexts it is appropriate and preferred to have negative numbers wrapped in parens, particularly when replicating output formatting from Excel or other accounting software. While it may not be the best "default" option as it were, it does seem it would be prudent to have this as an option, as it is definitely not a bug, and could break expectations.