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

Trailing space issue in Angular Locale #15018

Closed
TudorSfatosu opened this issue Aug 12, 2016 · 5 comments
Closed

Trailing space issue in Angular Locale #15018

TudorSfatosu opened this issue Aug 12, 2016 · 5 comments

Comments

@TudorSfatosu
Copy link

TudorSfatosu commented Aug 12, 2016

Note: for support questions, please use one of these channels: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#question. This repository's issues are reserved for feature requests and bug reports.

Do you want to request a feature or report a bug?
Reporting a bug

What is the current behavior?
A trailing space is added to RO and IT angular locale, maybe in similar locales as well.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).
DEMO: http://jsfiddle.net/4gyjtu8g/245/
one

What is the expected behavior?
There should be no trailing space, like the EN angular locale

What is the motivation / use case for changing the behavior?
To fix a general bug

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.
http://ajax.googleapis.com/ajax/libs/angularjs/1.3.0/angular.min.js
https://cdnjs.cloudflare.com/ajax/libs/angular-i18n/1.5.8/angular-locale_ro.js

Other information (e.g. stacktraces, related issues, suggestions how to fix)
two

@gkalpak
Copy link
Member

gkalpak commented Aug 12, 2016

This is kind of expected and is not specific to the ro locale. All locales that included a space before or after the currency symbol are affected in the same way.

According to the ro locale file, the currency-formatted number should be suffix with <CURRENCY_SYMBOL> (a space, followed by the currency symbol).
By default, the currency symbol is RON: currencyFilter(10) --> '10.00 RON'
When you are replace the default currency symbol with the empty string, the result will still contain the space: currencyFilter(10, '') --> '10.00 '

You won't see the same problem with the default en locale, because in that locale there is no space between the number and the currency symbol:
currencyFilter(10) --> '$10' (no space before 10)
currencyFilter(10, '') --> '10' (no space before 10)

The easy way out is to trim the output before returning it to the user, but I am not sure if there are valid usecases for having leading/trailing whitespace characters.

Does anyone happen to have any insights on that?

Obviously, it is very easy to work-around it yourself - if you are sure you don't need the extra space - by:

  1. Trimming the output:

    {{ (10 | currency:'').trim() }}

    (Bonus points for creating a trim filter 😛)

  2. Create your custom filter that wraps currency and automatically trims the output:

    .filter('currencyNoSymbol', function currencyNoSymbolFactory(currencyFilter) {
      return function currencyNoSymbol(input, fracSize) {
        var output = currencyFilter(input, '', fracSize);
        return output && output.trim();
      };
    })

    Then use it like this:

    {{ 10 | currencyNoSymbol }}
  3. Overwrite/Decorate the built-in currency filter to always trim the output (then use it normally).

EDIT: These are suggested as work-arounds for people to use in their apps, until this is properly fixed in core.

@gkalpak gkalpak modified the milestones: Purgatory, Backlog Aug 12, 2016
@gkalpak gkalpak self-assigned this Aug 12, 2016
@wesleycho
Copy link
Contributor

I think it make sense to have the currency filter do the trimming in core itself, as it sounds like it is unintended behavior in this case. The overhead added should be minimal, and it would be easy to address without impacting anyone.

@gkalpak
Copy link
Member

gkalpak commented Aug 12, 2016

The only thing I am concerned/unsure about, is whether there is some locale that requires a space before/after its values. (I doubt it, but I am not sure either.)

@gkalpak
Copy link
Member

gkalpak commented Aug 12, 2016

From a quick search through the i18n files I didn't find any locale for which the trimming would be problematic.

This is more involved though, because there is another usecase that suffers from the same issue, but won't be solved by trimming: negative values for locales that have the currency symbol before the number. E.g.:

// Assuming a locale that formats `10` as `X 10.00` and `-10` as `-X 10`
// (where `X` is the currency symbol):

// Desired
currencyFilter(-10, '')   // --> '-10'

// Actual
currencyFilter(-10, '')   // --> '- 10' (space between `-` and `10`)

@gkalpak gkalpak modified the milestones: Backlog, Purgatory Aug 12, 2016
@TudorSfatosu
Copy link
Author

I want to thank you guys for the involvement on this matter.
I don't feel like adding more for the moment.
If you feel you have more questions for me, just let me know!

gkalpak added a commit to gkalpak/angular.js that referenced this issue Sep 7, 2016
In most locales, this won't make a difference (since they do not have whotespace around their
currency symbols). In locales where there is a whitespace separating the currency symbol from the
number, it makes sense to also remove such whitespace if the user specified an empty currency symbol
(indicating they just want the number).

Fixes angular#15018
Closes angular#15085
gkalpak added a commit to gkalpak/angular.js that referenced this issue Sep 7, 2016
In most locales, this won't make a difference (since they do not have whotespace around their
currency symbols). In locales where there is a whitespace separating the currency symbol from the
number, it makes sense to also remove such whitespace if the user specified an empty currency symbol
(indicating they just want the number).

Fixes angular#15018
Closes angular#15085
@petebacondarwin petebacondarwin modified the milestones: Backlog, Backlog2 Oct 17, 2016
@petebacondarwin petebacondarwin added this to the Backlog2 milestone Oct 17, 2016
gkalpak added a commit that referenced this issue Dec 19, 2017
In most locales, this won't make a difference (since they do not have
whitespace around their currency symbols). In locales where there is a
whitespace separating the currency symbol from the number, it makes
sense to also remove such whitespace if the user specified an empty
currency symbol (indicating they just want the number).

Fixes #15018
Closes #15085

Closes #15105
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.