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

[amp story shopping] Currency i18n #37126

Closed
processprocess opened this issue Dec 6, 2021 · 2 comments · Fixed by #37249
Closed

[amp story shopping] Currency i18n #37126

processprocess opened this issue Dec 6, 2021 · 2 comments · Fixed by #37249

Comments

@processprocess
Copy link
Contributor

processprocess commented Dec 6, 2021

Description

The currency symbol and number formatting need to handle i18n.

Screen Shot 2021-12-06 at 4 36 37 PM

Use Intl.NumberFormat to enable language-sensitive number formatting.

Locale will need to be retrieved, code example.

A product-price-currency entry will need to be specified in the config to display the correct currency symbol:

product-price: 100,
product-price-currency: ‘USD’

Since the currency symbol's position is dynamic it will need to be the same size as the number.

cc @newmuis who may have more context on i18n.

Alternatives Considered

product-price as a string that includes currency symbol and number formatting.
This makes validation and number formatting complex.

Additional Context

We do not want to support currency conversion - that would require a request to an API.

No response

@newmuis
Copy link
Contributor

newmuis commented Dec 8, 2021

Good catch! I think we should likely separate them out semantically: the price (a Number, 100 in this example), and the currency (an enum, USD in this example). We can then separately evaluate the currency to understand how to present it

@gmajoulet
Copy link
Contributor

+1 to accepting a currency string, and passing the Number/float + currency string into Intl.NumberFormat https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants