currency pricerange formatter #943
Conversation
- use iso-country-currency npm library to retrieve currency symbol based on location's country code (ISO format), and construct the price range accordingly. J=SLAP-1552 TEST=manual&auto - added jest test for Formatter.priceRange - add priceRange for a UK location. See that it's displayed in £ instead of default $
static/js/formatters-internal.js
Outdated
| * @param {string} countrycode The country code from LiveAPI entity (e.g. profile.address.countryCode) | ||
| * @return {string} The price range with correct currency symbol formatting according to country code | ||
| */ | ||
| export function priceRange(priceRange, countryCode) { |
There was a problem hiding this comment.
Minor nit, but I find it confusing to have a function called priceRange that takes in a priceRange. How about naming this something like localizePriceRange?
There was a problem hiding this comment.
I think keeping the function name as priceRange would be more consistent with the other formatter functions like Formatter.price(). I can rename priceRange param to defaultPriceRange or entityPriceRange?
There was a problem hiding this comment.
Sure, defaultPriceRange or priceRangeFieldValue sgtm!
static/js/formatters-internal.js
Outdated
| } | ||
| } | ||
| console.warn(`Unable to determine currency symbol from ISO country code ${countryCode}.`); | ||
| return ''; |
There was a problem hiding this comment.
In this case, would it be better to simply return the original price range, non-localized?
There was a problem hiding this comment.
I thought it might be a bit misleading to have default dollar signs on international locations if we can't localized it so I left it blank. But I can return the original price range if you think that's fine!
There was a problem hiding this comment.
My first inclination is that we return the original price range. That way, there's at least something displayed. The formatter is then strictly better than just plopping the original price range on the page. Let's go with that approach for now and see if we get QA feedback.
static/js/formatters-internal.js
Outdated
| if (currencyCode) { | ||
| const currencySymbol = getSymbolFromCurrency(currencyCode); | ||
| if (currencySymbol) { | ||
| return Array(priceRange.length+1).join(currencySymbol); |
There was a problem hiding this comment.
For my own edification, what does the priceRange object look like?
There was a problem hiding this comment.
the given priceRange from entity, if it's defined, is like this: '$', '$$', '$$$', or '$$$$'.
There was a problem hiding this comment.
So it's a string? The param type listed in the method signature is object.
There was a problem hiding this comment.
oops, I missed that. Updated!
static/js/formatters-internal.js
Outdated
| console.warn(`No price range or country code given.`); | ||
| return ''; | ||
| } | ||
| const currencyCode = LocaleCurrency.getCurrency(countryCode); | ||
| if (currencyCode) { | ||
| const currencySymbol = getSymbolFromCurrency(currencyCode); | ||
| if (currencySymbol) { | ||
| return Array(priceRange.length+1).join(currencySymbol); | ||
| return Array(defaultPriceRange.length+1).join(currencySymbol); |
There was a problem hiding this comment.
Could we just do defaultPriceRange.replaceAll('$", currencySymbol) instead? Seems a bit simpler.
There was a problem hiding this comment.
hm you are right. I guess the backend will only have $ for now. I will update this!
## Version 1.24.0 ### Features - A new formatter was added that prints the list of categories applied to an entity. (#946) - Added a new localized price range formatter. (#943) ### Enhancements - Google's text highlighting was added to the "View Details" link on the `document-search` Direct Answer card. Note that the highlighting is only applied when the user's browser is Chrome. (#945) - The Open Status translations are no longer hard-coded in `static` JS files. Instead, they are sourced from the Theme's .PO files. This means they can be overridden in custom .PO files. ### Bugfixes - Styling was added to the `prominent-image` and `prominent-video` cards to ensure that images and videos, respectively, are the same size. (#941, #950)
locale-currencyandcurrency-symbol-mapnpm library to retrieve currency symbol based on location's country code (ISO format), and construct the price range accordingly.currency-symbol-mapfrom the node_modules exclusion in webpack babel for production build so it can be transpile to es5J=SLAP-1552
TEST=manual&auto