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

Add support for alternate currencies based on the tenant #8

Merged
merged 9 commits into from
Aug 14, 2019

Conversation

nerdenough
Copy link
Contributor

This PR adds support to the boilerplate for multiple currencies, based on the tenant's settings. It adds a custom CurrencyValue component which builds on top of react-intl's FormattedNumber component and Intl.NumberFormat.

Examples

I couldn't find a way to change my language and currency through my account, but hardcoding both into the app gives decent examples.

Locale en, currency nok:
image

Locale nb-no, currency nok:
image

Locale en, currency nzd:
image

Locale en, currency btc:
image

Locale de, currency eur:
image

Notes

  • I've currently locked react-intl to 3.0.0 due to Can't import the named export 'Component' from non EcmaScript module (only default export is available) formatjs/formatjs#1395.
  • Probably need to look at better error handling and default local/currency if the tenant query fails.
  • From what I can tell the Intl.NumberFormat functionality doesn't support special use cases like ,- for NOK or :- for SEK. When the locale is nb-NO and currency is NOK the currency will be rendered as kr 999,00. When the locale is en the currency will be rendered as NOK 999.00.
  • The original VAT amount was vatAmount.toFixed(2). I have kept this value at 2dp for consistency but not sure if it's required.

@nerdenough
Copy link
Contributor Author

Might need to leave the minimumFractionDigitals at its default (2dp for most currencies). Setting it to 0 results in values like this:
image

Whereas it would be more consistent to display like this:
image

@hakonkrogh
Copy link
Contributor

Ideally, we would like the boilerplate to make one single request to the server to fetch all the data it needs for the first render. Performance FTW! This is why the existing query fetches the component data AND the top menu.

Can you move the query for fetching tenant info to the same query as well? And also, what kind of server timings are we getting if we do that?

@nerdenough
Copy link
Contributor Author

Removing next-i18next in this PR as the removal depends on the CurrencyValue component as a replacement in some cases, which is required for #13.

@nerdenough nerdenough self-assigned this Aug 12, 2019
@nerdenough
Copy link
Contributor Author

Average timings (of 10):
Before: 6.4ms
After: 7.23ms

@nerdenough nerdenough merged commit 4a65ae8 into CrystallizeAPI:master Aug 14, 2019
@nerdenough nerdenough deleted the alternate-currencies branch August 14, 2019 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants