Skip to content

I18n default currency from locale#34724

Closed
petebacondarwin wants to merge 2 commits intoangular:masterfrom
petebacondarwin:i18n-default-currency-from-locale
Closed

I18n default currency from locale#34724
petebacondarwin wants to merge 2 commits intoangular:masterfrom
petebacondarwin:i18n-default-currency-from-locale

Conversation

@petebacondarwin
Copy link
Copy Markdown
Contributor

@petebacondarwin petebacondarwin commented Jan 10, 2020

This PR is for v10 only. It adds a breaking change that is not wanted in v9.

@petebacondarwin petebacondarwin added state: blocked area: i18n Issues related to localization and internationalization target: major This PR is targeted for the next major release labels Jan 10, 2020
@petebacondarwin petebacondarwin added this to the v10-candidates milestone Jan 10, 2020
@petebacondarwin petebacondarwin requested review from a team January 10, 2020 12:51
@googlebot
Copy link
Copy Markdown

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@petebacondarwin petebacondarwin force-pushed the i18n-default-currency-from-locale branch 6 times, most recently from be32a19 to 7aeedea Compare January 16, 2020 11:06
@googlebot
Copy link
Copy Markdown

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@pullapprove pullapprove bot requested review from AndrewKushnir and alxhub January 29, 2020 20:25
@pullapprove pullapprove bot requested a review from mhevery February 6, 2020 23:33
@petebacondarwin petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: blocked labels Apr 13, 2020
@pullapprove pullapprove bot requested a review from kara April 13, 2020 08:21
@petebacondarwin petebacondarwin force-pushed the i18n-default-currency-from-locale branch from 7aeedea to 6bfc538 Compare April 13, 2020 08:23
@petebacondarwin petebacondarwin added the action: presubmit The PR is in need of a google3 presubmit label Apr 13, 2020
@petebacondarwin petebacondarwin removed request for a team, AndrewKushnir and alxhub April 13, 2020 08:41
@petebacondarwin petebacondarwin force-pushed the i18n-default-currency-from-locale branch from 6bfc538 to 27f01ee Compare April 13, 2020 08:42
@pullapprove pullapprove bot requested a review from IgorMinar April 13, 2020 18:55
@AndrewKushnir
Copy link
Copy Markdown
Contributor

@petebacondarwin quick comment: it looks like unrelated fixup commit was added to this PR.

I've also started a Blueprint Presubmit for this PR as you requested (by adding the "presubmit" label).

Thank you.

@petebacondarwin
Copy link
Copy Markdown
Contributor Author

Thanks @AndrewKushnir - The fixup commit is for this PR but for some reason it picked up the wrong commit message. I'll change that.

@petebacondarwin petebacondarwin force-pushed the i18n-default-currency-from-locale branch from 8ade2a2 to 2ca5f8e Compare April 14, 2020 09:28
@petebacondarwin petebacondarwin added state: WIP and removed action: presubmit The PR is in need of a google3 presubmit action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 14, 2020
@petebacondarwin
Copy link
Copy Markdown
Contributor Author

I need to write a migration to go with this change that will add in a provider for the default to ensure that current projects do not change unexpectedly.

The default currency code used for things like `CurrencyPipe` is now
taken from the current locale. Previously this was set to `USD` if the
developer did not explicitly provide the `DEFAULT_CURRENCY_CODE` injectable.

BREAKING CHANGE:

If `DEFAULT_CURRENCY_CODE` was not provided, the default currency for
`CurrencyPipe` was always `USD`. Now it will be the currency code for
the currently configured locale, which set by providing `LOCALE_ID` or by
setting the value globally (`$localize.locale` in IVY or `goog.LOCALE`
in G3).

To recover the original behaviour provide `DEFAULT_CURRENCY_CODE`, in your
application `NgModule`, as follows:

```ts
{provide: DEFAULT_CURRENCY_CODE, useValue: 'USD'},
```
@petebacondarwin petebacondarwin force-pushed the i18n-default-currency-from-locale branch from 2ca5f8e to f12eccd Compare April 14, 2020 21:49
@petebacondarwin
Copy link
Copy Markdown
Contributor Author

After further discussion, it is decided that this is not actually a desirable change.
The currency being displayed should not depend upon the current locale - at least not by default.

If the application wishes to display prices in locale specific currencies then it would not only need to change the currency symbol being displayed but also the value of the currency based on the current exchange rate.

So automatically setting the default currency to the current locale is not enough on its own, without further logic to update the value.

It is already possible to set the default currency code by providing the DEFAULT_CURRENCY_CODE, and if wanted it can even be set as the current locale's currency via a factory provider:

{provide: DEFAULT_CURRENCY_CODE, useFactory: getDefaultCurrencyCode, deps: [LOCALE_ID]},

Therefore we do not intend to land this PR. Closing.

@petebacondarwin petebacondarwin deleted the i18n-default-currency-from-locale branch April 16, 2020 08:27
@angular-automatic-lock-bot
Copy link
Copy Markdown

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 May 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: i18n Issues related to localization and internationalization cla: yes state: WIP target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants