fix(currencyFilter/formatNumber)

[petebacondarwin]

See #5672 and #5674

[mary-poppins]

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

• Uses the issue template (#8363)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

[petebacondarwin]

So there are actually a couple of things that are going on here.

The first is that there was a bug in formatNumber where it was not correctly formatting some small numbers in the case where fractionSize is not provided - this has actually got nothing to do with the currency filter.

The second is that we should be able to provide the fractionSize as a parameter of the currency filter (or have it fall back correctly to the current locale. This change is trivial, assuming that the previous fix is in place.

I reworked and reworded the tests to make it clear what was being tested here. I think that mixing of currency and formatNumber tests was confusing the discussion.

@caitp, @park9140 and @genuinefafa - does this make sense?

[genuinefafa]

Yes, it does. It makes absolute sense.
That "2" parameter was pretty ugly to even watch! 👍

[park9140]

Looks good to me.

[wesleycho]

Heh, I opened a PR for optional rounding in the currency filter before seeing this - this would be incredibly useful!

[wesleycho]

I should add two comments though - for negative values, there is a modification that needs to be made so one can round to whole numbers with the currency filter.

In this line in the formatNumber function:

if (fractionSize && fractionSize !== "0") formatedText += decimalSep + fraction.substr(0, fractionSize);

I recommend adding && fractionSize >= 0 to the condition here. The reason is because say someone wants to use -2. Given the currency filter's current behavior in this PR, if one has {{amount | currency: '$': -2}}, $1234.5678 would round to $1200.. This proposed change would remove the trailing dot.

In addition, I propose that the currency filter's rounding value should be optional, and it should fallback to the previous behavior if omitted.

Implementation of these two features can be observed in #8391.

[petebacondarwin]

@wesleycho:

• I added your decimal point fix.
• The behaviour after this PR is that the fractionSize is indeed optional as you request, but instead of falling back to the previous hard-coded value of 2, it falls back to the default maximum fraction size for the current locale, which for most locales is indeed 2.

[genuinefafa]

@petebacondarwin excelent! that's excelent... 👍

[genuinefafa]

What's going on with this PR? I just updated to latest angularjs#1.2.25 and this is not included. 😢

[park9140]

@petebacondarwin It looks like this is only pulled into master and thus only 1.3 is going to have it? Can we get it added to 1.2?