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

Currency Pipe Changes - US vs AU vs NZ Dollars #20385

Closed
Azsael opened this issue Nov 13, 2017 · 8 comments
Closed

Currency Pipe Changes - US vs AU vs NZ Dollars #20385

Azsael opened this issue Nov 13, 2017 · 8 comments
Assignees
Labels
area: i18n freq1: low regression Indicates than the issue relates to something that worked in a previous version type: bug/fix

Comments

@Azsael
Copy link

Azsael commented Nov 13, 2017

I'm submitting a...


[x] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

In the latest release, changes from using the i18n API to internal has stopped utilising locale for when displaying currency:
So

{{10 | 'AUD':'symbol':'1.0-2':'en-AU'}}
will display A$10
{{10 | 'AUD':'symbol':'1.0-2':'en-AU'}}
will display NZ$10
{{10 | 'USD':'symbol':'1.0-2':'en-AU'}}
will display $10 before this displayed as (default locale as en-AU)
{{10 | 'AUD':'symbol':'1.0-2'}}
will display A$10
{{10 | 'USD':'symbol':'1.0-2'}}
will display USD$10
{{10 | 'NZD':'symbol':'1.0-2':'en-AU'}}
will display NZ$10

Expected behavior

I would expect consistent behaviour or to utilise locale to display currency. If locale wasn't utilised I would expect

{{10 | 'USD':'symbol':'1.0-2':'en-AU'}}
will display US$10 otherwise I would expect utilise current locale setting.

The app I am working on handles all three currencies (NZD, AUD, & USD), and it utilised by people who travel so US customers to AUD products and vice versa are common, so clarity on currency is important.

What is the motivation / use case for changing the behavior?

The app I am working on handles all three currencies (NZD, AUD, & USD), and it utilised by people who travel so US customers to AUD products and vice versa are common, so clarity on currency is important.

People book a wine tour from US for their visit to Australia, the people in the admin staff manage the tours through central app which has all NZ, US, & AU tours, etc.

Environment


Angular version: 5.0.1
Browser: N/A
 For Tooling issues: N/A
Others: N/A
@ocombe
Copy link
Contributor

ocombe commented Nov 13, 2017

I don't understand what the problem is, could you create a stackblitz (https://stackblitz.com/) showing the error please?
The behavior is supposed to be the following:

  • if you don't provide a locale in the pipe, it will use the locale of the application
  • the default locale for Angular is "en-US"
  • you can change the default locale by providing the LOCALE_ID in your main module (or using --locale with the CLI in AoT mode)
  • if you set the locale in the pipe it will use that and ignore the application locale

Is this not the behavior that you notice?

@Azsael
Copy link
Author

Azsael commented Nov 14, 2017

Hey, sorry for being Confusing.

Here: https://angular-xkimgk.stackblitz.io

Problem is, locale isn't defining what "currency" symbol is being selected.

In CurrencyPipe (https://github.com/angular/angular/blob/161f88fe6f8399a92ed7ab120f0d4ec647f7b139/packages/common/src/pipes/number_pipe.ts), the findCurrencySymbol method is being called without defining what locale and if you follow code goes to generated currency code list (https://github.com/angular/angular/blob/079d884b6cbe179c3565d8db443d76b1ae041af7/packages/common/src/i18n/currencies.ts), where USD is defined only as $.

In previous version, by using the i18n libraries, if locale was en-AU, then australian (AUD) currency would be $10, and USD would be as USD$10, and vice versa if you swapped locale.

I am not using symbol-short as I need to deal with all three currencies in the app,, so would be good if we aren't considering locale in particular that the currency symbol we use is consistent/not country bias.

@ocombe
Copy link
Contributor

ocombe commented Nov 14, 2017

Ok I understand the problem and you're right, according to CLDR data AUD is $ in en-AU: https://github.com/unicode-cldr/cldr-numbers-modern/blob/master/main/en-AU/currencies.json#L124
and it's A$ in en-US: https://github.com/unicode-cldr/cldr-numbers-modern/blob/master/main/en/currencies.json#L123

We've extracted the currencies based on "en-US" locales only because it takes a lot of place to store currencies in all of the locales and we didn't even think about those kind of use cases.
We could maybe add this in the "extra" package. We need to find a good solution for this problem.

@ocombe ocombe added the feature Issue that requests a new feature label Nov 14, 2017
@ocombe ocombe self-assigned this Nov 14, 2017
@ocombe ocombe added regression Indicates than the issue relates to something that worked in a previous version freq1: low labels Nov 30, 2017
@dgfranklin
Copy link

@ocombe: I'm a Googler and my team is also facing this issue. What are your plans and eta for dealing with this? I'm willing to contribute if you're out of bandwidth.

@ocombe
Copy link
Contributor

ocombe commented Nov 30, 2017

We have a meeting to discuss this next Tuesday, it should not be very hard to resolve once we've agreed on the change.

@vicb
Copy link
Contributor

vicb commented Dec 8, 2017

@ocombe Could you please describe what we discussed during our meeting ?

IIRC we decided that:
1 - we will NOT include all the names for all the currencies in all the locales,
BUT
2 - we will update the pipe to take an optional override.

@ocombe
Copy link
Contributor

ocombe commented Dec 11, 2017

We will update the currency pipe to allow for custom symbol to be used so that you are not blocked until we figure out if we need to add those locale currency info, or if we open the i18n api so that an external library can provide this instead

@ocombe ocombe modified the milestones: 5.1.2-Sprint 1 (Dec 11 - 19, 2017), v6.0 Jan 17, 2018
@IgorMinar IgorMinar modified the milestones: v6.0, v6-candidates Jan 19, 2018
ocombe added a commit to ocombe/angular that referenced this issue Jan 25, 2018
BREAKING CHANGE: we now use locale currency values to output the symbols, since they may be different in each locale (we were only using english data previously)

Fixes angular#20385
ocombe added a commit to ocombe/angular that referenced this issue Jan 26, 2018
we now use locale currency values to output the symbols, since they may be different in each locale (we were only using english data previously)

Fixes angular#20385
ocombe added a commit to ocombe/angular that referenced this issue Jan 29, 2018
we now use locale currency values to output the symbols, since they may be different in each locale (we were only using english data previously)

Fixes angular#20385
ocombe added a commit to ocombe/angular that referenced this issue Jan 30, 2018
we now use locale currency symbols, since they may be different in each locale (we were only using english data previously)

Fixes angular#20385
ocombe added a commit to ocombe/angular that referenced this issue Jan 31, 2018
we now use locale currency symbols, since they may be different in each locale (we were only using english data previously)

Fixes angular#20385
ocombe added a commit to ocombe/angular that referenced this issue Feb 8, 2018
we now use locale currency symbols, since they may be different in each locale (we were only using english data previously)

Fixes angular#20385
@mhevery mhevery closed this as completed in 420cc7a Feb 9, 2018
jbogarthyde pushed a commit to jbogarthyde/angular that referenced this issue Feb 23, 2018
we now use locale currency symbols, since they may be different in each locale (we were only using english data previously)

Fixes angular#20385

PR Close angular#21783
leo6104 pushed a commit to leo6104/angular that referenced this issue Mar 25, 2018
we now use locale currency symbols, since they may be different in each locale (we were only using english data previously)

Fixes angular#20385

PR Close angular#21783
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: i18n freq1: low regression Indicates than the issue relates to something that worked in a previous version type: bug/fix
Projects
None yet
Development

No branches or pull requests

6 participants