Skip to content

Commit

Permalink
Improve formatCurrency short form symbol formatting with exceptinos (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
sunical committed Nov 1, 2022
1 parent f6ba893 commit 4b5aa94
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 33 deletions.
15 changes: 15 additions & 0 deletions .changeset/quiet-foxes-attend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'@shopify/react-i18n': minor
---

Update currency symbol formatting to follow Polaris localized currency formatting guidelines. No longer displays a character or ISO code beside the currency symbol, for certain currencies and locales. The following table summarizes the changes, assuming a locale of `en-US`:

| Method | Previous output | New output |
| ------------------------------------------------------------- | -------------------------------- | ------------------------------- |
| `i18n.formatCurrency(2, {currency: 'AUD', form: 'short'})` | `A$2.00` | `$2.00` |
| `i18n.formatCurrency(2, {currency: 'AUD', form: 'explicit'})` | `A$2.00 AUD` | `$2.00 AUD` |
| `i18n.formatCurrency(2, {currency: 'AUD', form: 'auto'})` | `A$2.00 AUD` | `$2.00 AUD` |
| `i18n.formatCurrency(2, {currency: 'SGD', form: 'explicit'})` | `SGD 2.00 SGD` | `$2.00 SGD` |
| `i18n.getCurrencySymbol('AUD')` | `{symbol: 'A$', prefixed: true}` | `{symbol: '$', prefixed: true}` |

Deprecate usage of`i18n.getCurrencySymbolLocalized(locale, currency)`. Use `i18n.getCurrencySymbol(currency, locale)` instead.
5 changes: 5 additions & 0 deletions packages/react-i18n/src/constants/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,8 @@ export const EASTERN_NAME_ORDER_FORMATTERS = new Map([
full ? `${lastName}${firstName}` : lastName,
],
]);

export const CurrencyShortFormException = {
BRL: 'R$',
HKD: 'HK$',
} as const;
53 changes: 38 additions & 15 deletions packages/react-i18n/src/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
currencyDecimalPlaces,
DEFAULT_DECIMAL_PLACES,
EASTERN_NAME_ORDER_FORMATTERS,
CurrencyShortFormException,
} from './constants';
import {
MissingCurrencyCodeError,
Expand All @@ -52,7 +53,6 @@ export interface NumberFormatOptions extends Intl.NumberFormatOptions {
as?: 'number' | 'currency' | 'percent';
precision?: number;
}

export interface CurrencyFormatOptions extends NumberFormatOptions {
form?: 'auto' | 'short' | 'explicit' | 'none';
}
Expand Down Expand Up @@ -343,18 +343,21 @@ export class I18n {
return WEEK_START_DAYS.get(country) || DEFAULT_WEEK_START_DAY;
}

getCurrencySymbol = (currencyCode?: string) => {
getCurrencySymbol = (currencyCode?: string, locale: string = this.locale) => {
const currency = currencyCode || this.defaultCurrency;
if (currency == null) {
throw new MissingCurrencyCodeError(
'formatCurrency cannot be called without a currency code.',
);
}
return this.getCurrencySymbolLocalized(this.locale, currency);
return this.getShortCurrencySymbol(currency, locale);
};

/**
* @deprecated Replace usage of `i18n.getCurrencySymbolLocalized(locale, currency)` with `i18n.getCurrencySymbol(currency, locale)`
*/
getCurrencySymbolLocalized(locale: string, currency: string) {
return getCurrencySymbol(locale, {currency});
return this.getShortCurrencySymbol(currency, locale);
}

formatName(
Expand Down Expand Up @@ -479,24 +482,44 @@ export class I18n {
//
// For other currencies, e.g. CHF and OMR, the "symbol" is the ISO currency code.
// In those cases, we return the full currency code without stripping the country.
private getShortCurrencySymbol(currencyCode = this.defaultCurrency || '') {
const currency = currencyCode || this.defaultCurrency || '';
private getShortCurrencySymbol(
currency: string = this.defaultCurrency || '',
locale: string = this.locale,
) {
const regionCode = currency.substring(0, 2);
const info = this.getCurrencySymbol(currency);
const shortSymbol = info.symbol.replace(regionCode, '');
let shortSymbolResult: {symbol: string; prefixed: boolean};

// currencyDisplay: 'narrowSymbol' was added to iOS in v14.5. See https://caniuse.com/?search=currencydisplay
// We still support ios 12/13, so we need to check if this works and fallback to the default if not
// All other supported browsers understand narrowSymbol, so once our minimum iOS version is updated we can remove this fallback
try {
shortSymbolResult = getCurrencySymbol(locale, {
currency,
currencyDisplay: 'narrowSymbol',
});
} catch {
shortSymbolResult = getCurrencySymbol(locale, {currency});
}

if (currency in CurrencyShortFormException) {
return {
symbol: CurrencyShortFormException[currency],
prefixed: shortSymbolResult.prefixed,
};
}

const shortSymbol = shortSymbolResult.symbol.replace(regionCode, '');
const alphabeticCharacters = /[A-Za-zÀ-ÖØ-öø-ÿĀ-ɏḂ-ỳ]/;

return alphabeticCharacters.exec(shortSymbol)
? info
: {symbol: shortSymbol, prefixed: info.prefixed};
? shortSymbolResult
: {symbol: shortSymbol, prefixed: shortSymbolResult.prefixed};
}

private humanizeDate(date: Date, options?: Intl.DateTimeFormatOptions) {
if (isFutureDate(date)) {
return this.humanizeFutureDate(date, options);
} else {
return this.humanizePastDate(date, options);
}
return isFutureDate(date)
? this.humanizeFutureDate(date, options)
: this.humanizePastDate(date, options);
}

private formatDateTime(
Expand Down
85 changes: 67 additions & 18 deletions packages/react-i18n/src/tests/i18n.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,20 @@ jest.mock('../utilities', () => ({
...jest.requireActual('../utilities'),
translate: jest.fn(),
getTranslationTree: jest.fn(),
getCurrencySymbol: jest.fn(
jest.requireActual('../utilities').getCurrencySymbol,
),
}));

const originalGetCurrencySymbol: jest.Mock =
jest.requireActual('../utilities').getCurrencySymbol;

/* eslint-disable @typescript-eslint/no-var-requires */
const translate: jest.Mock = require('../utilities').translate;
const getTranslationTree: jest.Mock =
require('../utilities').getTranslationTree;
const getCurrencySymbolMock: jest.Mock =
require('../utilities').getCurrencySymbol;
/* eslint-enable @typescript-eslint/no-var-requires */

describe('I18n', () => {
Expand Down Expand Up @@ -849,7 +857,7 @@ describe('I18n', () => {
${'en-US'} | ${'OMR'} | ${'OMR 1,234.560'}
${'en-US'} | ${'USD'} | ${'$1,234.56 USD'}
${'fr-FR'} | ${'EUR'} | ${'1 234,56 € EUR'}
${'fr-FR'} | ${'JPY'} | ${'1 235 JPY'}
${'fr-FR'} | ${'JPY'} | ${'1 235 ¥ JPY'}
${'fr-FR'} | ${'OMR'} | ${'1 234,560 OMR'}
${'fr-FR'} | ${'USD'} | ${'1 234,56 $ USD'}
`(
Expand Down Expand Up @@ -881,7 +889,7 @@ describe('I18n', () => {
${'en-US'} | ${'OMR'} | ${'-OMR 1,234.560'}
${'en-US'} | ${'USD'} | ${'-$1,234.56 USD'}
${'fr-FR'} | ${'EUR'} | ${'-1 234,56 € EUR'}
${'fr-FR'} | ${'JPY'} | ${'-1 235 JPY'}
${'fr-FR'} | ${'JPY'} | ${'-1 235 ¥ JPY'}
${'fr-FR'} | ${'OMR'} | ${'-1 234,560 OMR'}
${'fr-FR'} | ${'USD'} | ${'-1 234,56 $ USD'}
`(
Expand Down Expand Up @@ -921,7 +929,10 @@ describe('I18n', () => {
const amount = 1234.56;

const i18n = new I18n(defaultTranslations, {locale});
const result = i18n.formatCurrency(amount, {form: 'none', currency});
const result = i18n.formatCurrency(amount, {
form: 'none',
currency,
});
expect(sanitizeSpaces(result)).toBe(expected);
},
);
Expand Down Expand Up @@ -965,16 +976,18 @@ describe('I18n', () => {
${'de-AT'} | ${'OMR'} | ${'OMR 1 234,560'}
${'de-AT'} | ${'USD'} | ${'$ 1 234,56'}
${'en-AU'} | ${'AUD'} | ${'$1,234.56'}
${'en-US'} | ${'AUD'} | ${'A$1,234.56'}
${'en-US'} | ${'AUD'} | ${'$1,234.56'}
${'en-US'} | ${'EUR'} | ${'€1,234.56'}
${'en-US'} | ${'JPY'} | ${'¥1,235'}
${'en-US'} | ${'OMR'} | ${'OMR 1,234.560'}
${'en-US'} | ${'USD'} | ${'$1,234.56'}
${'fr-FR'} | ${'EUR'} | ${'1 234,56 €'}
${'fr-FR'} | ${'JPY'} | ${'1 235 JPY'}
${'fr-FR'} | ${'JPY'} | ${'1 235 ¥'}
${'fr-FR'} | ${'OMR'} | ${'1 234,560 OMR'}
${'fr-FR'} | ${'USD'} | ${'1 234,56 $'}
${'sv-SE'} | ${'USD'} | ${'1 234,56 $'}
${'en-US'} | ${'SGD'} | ${'$1,234.56'}
${'fr-FR'} | ${'SGD'} | ${'1 234,56 $'}
`(
'formats 1234.56 of $currency in $locale to expected $expected',
({locale, currency, expected}) => {
Expand All @@ -1000,10 +1013,12 @@ describe('I18n', () => {
${'en-US'} | ${'OMR'} | ${'-OMR 1,234.560'}
${'en-US'} | ${'USD'} | ${'-$1,234.56'}
${'fr-FR'} | ${'EUR'} | ${'-1 234,56 €'}
${'fr-FR'} | ${'JPY'} | ${'-1 235 JPY'}
${'fr-FR'} | ${'JPY'} | ${'-1 235 ¥'}
${'fr-FR'} | ${'OMR'} | ${'-1 234,560 OMR'}
${'fr-FR'} | ${'USD'} | ${'-1 234,56 $'}
${'sv-SE'} | ${'USD'} | ${'-1 234,56 $'}
${'en-US'} | ${'SGD'} | ${'-$1,234.56'}
${'fr-FR'} | ${'SGD'} | ${'-1 234,56 $'}
`(
'formats -1234.56 of $currency in $locale to expected $expected',
({locale, currency, expected}) => {
Expand Down Expand Up @@ -2128,37 +2143,71 @@ describe('I18n', () => {
});
});

describe('#getShortCurrencySymbol()', () => {
describe('#getCurrencySymbol()', () => {
it.each`
currency | locale | expectedPrefixed | expectedSymbol
${'CZK'} | ${'cs-CZ'} | ${false} | ${' Kč'}
${'EUR'} | ${'en-US'} | ${true} | ${'€'}
${'EUR'} | ${'fr-FR'} | ${false} | ${' €'}
${'USD'} | ${'en-CA'} | ${true} | ${'$'}
${'USD'} | ${'en-US'} | ${true} | ${'$'}
${'USD'} | ${'fr-FR'} | ${false} | ${' $'}
${'OMR'} | ${'en-US'} | ${true} | ${'OMR '}
${'SGD'} | ${'en-US'} | ${true} | ${'$'}
${'AUD'} | ${'en-AU'} | ${true} | ${'$'}
`(
'returns $shortSymbol for $currency in $locale',
({currency, locale, expectedPrefixed, expectedSymbol}) => {
const i18n = new I18n(defaultTranslations, {locale});
// eslint-disable-next-line dot-notation
const {symbol, prefixed} = i18n['getShortCurrencySymbol'](currency);
const {symbol, prefixed} = i18n.getCurrencySymbol(currency);
expect(prefixed).toStrictEqual(expectedPrefixed);
expect(sanitizeSpaces(symbol)).toStrictEqual(expectedSymbol);
},
);
});

describe('#getCurrencySymbol()', () => {
it('returns the locale-specific currency symbol and its position', () => {
const expected = {
symbol: '€',
prefixed: true,
};
it.each`
currency | locale | expectedPrefixed | expectedSymbol
${'BRL'} | ${'en-US'} | ${true} | ${'R$'}
${'HKD'} | ${'en-US'} | ${true} | ${'HK$'}
`(
'returns the listed short symbol in the exceptions list for the given currency - $currency',
({currency, locale, expectedPrefixed, expectedSymbol}) => {
const i18n = new I18n(defaultTranslations, {locale});
const {symbol, prefixed} = i18n.getCurrencySymbol(currency);
expect(prefixed).toStrictEqual(expectedPrefixed);
expect(sanitizeSpaces(symbol)).toStrictEqual(expectedSymbol);
},
);

const i18n = new I18n(defaultTranslations, {locale: 'en'});
describe('when narrowSymbol throws an error', () => {
beforeEach(() => {
getCurrencySymbolMock.mockImplementation((...args) => {
if (args[1]?.currencyDisplay === 'narrowSymbol') {
throw new Error();
}
return originalGetCurrencySymbol(...args);
});
});

afterEach(() => {
getCurrencySymbolMock.mockReset();
getCurrencySymbolMock.mockImplementation(originalGetCurrencySymbol);
});

expect(i18n.getCurrencySymbol('eur')).toStrictEqual(expected);
it.each`
currency | locale | expectedPrefixed | expectedSymbol
${'SGD'} | ${'en-US'} | ${true} | ${'SGD'}
${'AUD'} | ${'en-US'} | ${true} | ${'A$'}
`(
'returns fallback value ("currencyDisplay: symbol") when "currencyDisplay: narrowSymbol" is not supported by the browser',
({currency, locale, expectedPrefixed, expectedSymbol}) => {
const i18n = new I18n(defaultTranslations, {locale});
const result = i18n.getCurrencySymbol(currency);
result.symbol = result.symbol.replace(/\s/g, '');
expect(result.prefixed).toStrictEqual(expectedPrefixed);
expect(sanitizeSpaces(result.symbol)).toStrictEqual(expectedSymbol);
},
);
});
});

Expand Down

0 comments on commit 4b5aa94

Please sign in to comment.