From 2d58320e51099140c6a5b483dde8da356f17727b Mon Sep 17 00:00:00 2001 From: Joren Broekema Date: Thu, 8 Sep 2022 09:19:07 +0200 Subject: [PATCH] feat: format number add thousandSeparator option, fix type decimalSep (#1774) --- .changeset/neat-pots-prove.md | 6 ++ docs/components/input-amount/use-cases.md | 6 +- .../systems/form/formatting-and-parsing.md | 2 +- .../test/lion-input-amount.test.js | 12 ++++ packages/localize/index.js | 1 + .../src/number/formatNumberToParts.js | 55 +++++++++++++----- .../localize/src/number/getGroupSeparator.js | 6 +- .../src/number/getSeparatorsFromNumber.js | 58 +++++++++++++++++++ packages/localize/src/number/parseNumber.js | 2 +- .../localize/test/number/formatNumber.test.js | 58 +++++++++++++++++++ .../number/getSeparatorsFromNumber.test.js | 48 +++++++++++++++ .../localize/types/LocalizeMixinTypes.d.ts | 7 ++- 12 files changed, 240 insertions(+), 21 deletions(-) create mode 100644 .changeset/neat-pots-prove.md create mode 100644 packages/localize/src/number/getSeparatorsFromNumber.js create mode 100644 packages/localize/test/number/getSeparatorsFromNumber.test.js diff --git a/.changeset/neat-pots-prove.md b/.changeset/neat-pots-prove.md new file mode 100644 index 000000000..f8c4ac878 --- /dev/null +++ b/.changeset/neat-pots-prove.md @@ -0,0 +1,6 @@ +--- +'@lion/input-amount': minor +'@lion/localize': minor +--- + +Allow specifying thousandSeparator for format number. BREAKING: change decimalSeparator type to only be ',' or '.'. diff --git a/docs/components/input-amount/use-cases.md b/docs/components/input-amount/use-cases.md index d45d41910..83f3a0945 100644 --- a/docs/components/input-amount/use-cases.md +++ b/docs/components/input-amount/use-cases.md @@ -108,15 +108,15 @@ export const noDecimals = () => html` For copy pasting numbers into the input-amount, there is slightly different parsing behavior. -Normally, when it receives an input with only 1 separator character, we check the locale to determine whether this character is a thousand separator, or a decimal separator. +Normally, when it receives an input with only 1 separator character, we check the locale to determine whether this character is a group (thousand) separator, or a decimal separator. When a user pastes the input from a different source, we find this approach (locale-based) quite unreliable, because it may have been copied from a 'mathematical context' (like an Excel sheet) or a context with a different locale. Therefore, we use the heuristics based method to parse the input when it is pasted by the user. ### What this means If the user in an English locale types `400,0` it will become `4,000.00` -because the locale determines that the comma is a thousand separator, not a decimal separator. +because the locale determines that the comma is a group separator, not a decimal separator. If the user in an English locale pastes `400,0` instead, it will become `400.00` because we cannot rely on locale. -Therefore, instead, we determine that the comma cannot be a thousand separator because it is not followed by 3 digits after. +Therefore, instead, we determine that the comma cannot be a group separator because it is not followed by 3 digits after. It is more likely to be a decimal separator. diff --git a/docs/fundamentals/systems/form/formatting-and-parsing.md b/docs/fundamentals/systems/form/formatting-and-parsing.md index 703ccdcba..96a482911 100644 --- a/docs/fundamentals/systems/form/formatting-and-parsing.md +++ b/docs/fundamentals/systems/form/formatting-and-parsing.md @@ -89,7 +89,7 @@ export const unparseable = () => html` A formatter should return a `formattedValue`. It accepts the current modelValue and an options object. -Below is a very naive and limited parser that ignores non-digits. The formatter then uses `Intl.NumberFormat` to format it with thousand separators. +Below is a very naive and limited parser that ignores non-digits. The formatter then uses `Intl.NumberFormat` to format it with group (thousand) separators. Formatted value is reflected back to the user `on-blur` of the field, but only if the field has no errors (validation). diff --git a/packages/input-amount/test/lion-input-amount.test.js b/packages/input-amount/test/lion-input-amount.test.js index 9f3875976..75a89a73a 100644 --- a/packages/input-amount/test/lion-input-amount.test.js +++ b/packages/input-amount/test/lion-input-amount.test.js @@ -70,6 +70,18 @@ describe('', () => { expect(el.formattedValue).to.equal('99.00'); }); + it('supports overriding groupSeparator in formatOptions', async () => { + const el = /** @type {LionInputAmount} */ ( + await fixture( + html``, + ) + ); + expect(el.formattedValue).to.equal('9,999.00'); + }); + it('ignores global locale change if property is provided', async () => { const el = /** @type {LionInputAmount} */ ( await fixture(html` diff --git a/packages/localize/index.js b/packages/localize/index.js index 18520ebb8..7d837cdf6 100644 --- a/packages/localize/index.js +++ b/packages/localize/index.js @@ -13,5 +13,6 @@ export { getCurrencyName } from './src/number/getCurrencyName.js'; export { getDecimalSeparator } from './src/number/getDecimalSeparator.js'; export { getFractionDigits } from './src/number/getFractionDigits.js'; export { getGroupSeparator } from './src/number/getGroupSeparator.js'; +export { getSeparatorsFromNumber } from './src/number/getSeparatorsFromNumber.js'; export { normalizeCurrencyLabel } from './src/number/normalizeCurrencyLabel.js'; export { parseNumber } from './src/number/parseNumber.js'; diff --git a/packages/localize/src/number/formatNumberToParts.js b/packages/localize/src/number/formatNumberToParts.js index 47eaf3424..b6dbfce41 100644 --- a/packages/localize/src/number/formatNumberToParts.js +++ b/packages/localize/src/number/formatNumberToParts.js @@ -1,4 +1,5 @@ import { emptyStringWhenNumberNan } from './utils/emptyStringWhenNumberNan.js'; +import { getSeparatorsFromNumber } from './getSeparatorsFromNumber.js'; import { getDecimalSeparator } from './getDecimalSeparator.js'; import { getGroupSeparator } from './getGroupSeparator.js'; import { getLocale } from '../utils/getLocale.js'; @@ -48,14 +49,29 @@ export function formatNumberToParts(number, options = {}) { let formattedParts = []; const formattedNumber = Intl.NumberFormat(computedLocale, options).format(parsedNumber); - const regexCurrency = /[.,\s0-9]/; + const { decimalSeparator, groupSeparator } = getSeparatorsFromNumber( + parsedNumber, + formattedNumber, + options, + ); + + // eslint-disable-next-line no-irregular-whitespace + const regexCurrency = /[.,\s0-9 _ ]/; const regexMinusSign = /[-]/; // U+002D, Hyphen-Minus, - const regexNum = /[0-9]/; - const regexSeparator = /[.,]/; const regexSpace = /[\s]/; let currency = ''; let numberPart = ''; let fraction = false; + let isGroup = false; + const group = getGroupSeparator(computedLocale, options); + const decimal = getDecimalSeparator(computedLocale, options); + if (decimalSeparator && groupSeparator && group === decimal) { + throw new Error(`Decimal and group (thousand) separator are the same character: '${group}'. +This can happen due to both props being specified as the same, or one of the props being the same as the other one from default locale. +Please specify .groupSeparator / .decimalSeparator on the formatOptions object to be different.`); + } + for (let i = 0; i < formattedNumber.length; i += 1) { // detect minusSign if (regexMinusSign.test(formattedNumber[i])) { @@ -76,24 +92,35 @@ export function formatNumberToParts(number, options = {}) { currency = ''; } - // detect dot and comma separators - if (regexSeparator.test(formattedNumber[i])) { + // group sep must be lead by / followed by a number + if ( + formattedNumber[i] === groupSeparator && + formattedNumber[i - 1].match(regexNum) && + formattedNumber[i + 1].match(regexNum) + ) { // Write number grouping if (numberPart) { formattedParts.push({ type: 'integer', value: numberPart }); numberPart = ''; } - const decimal = getDecimalSeparator(computedLocale, options); - if (formattedNumber[i] === decimal || options.decimalSeparator === decimal) { - formattedParts.push({ type: 'decimal', value: decimal }); - fraction = true; - } else { - formattedParts.push({ type: 'group', value: formattedNumber[i] }); - } + + formattedParts.push({ type: 'group', value: group }); + isGroup = true; } + + if (formattedNumber[i] === decimalSeparator) { + // Write number grouping + if (numberPart) { + formattedParts.push({ type: 'integer', value: numberPart }); + numberPart = ''; + } + + formattedParts.push({ type: 'decimal', value: decimal }); + fraction = true; + } + // detect literals (empty spaces) or space group separator if (regexSpace.test(formattedNumber[i])) { - const group = getGroupSeparator(computedLocale); const hasNumberPart = !!numberPart; // Write number grouping if (numberPart && !fraction) { @@ -106,10 +133,12 @@ export function formatNumberToParts(number, options = {}) { // If space equals the group separator it gets type group if (normalSpaces(formattedNumber[i]) === group && hasNumberPart && !fraction) { formattedParts.push({ type: 'group', value: formattedNumber[i] }); - } else { + // if we already pushed it as a group separator, don't add it as a literal on top.. + } else if (!isGroup) { formattedParts.push({ type: 'literal', value: formattedNumber[i] }); } } + isGroup = false; // Numbers after the decimal sign are fractions, write the last // fractions at the end of the number if (fraction === true && i === formattedNumber.length - 1) { diff --git a/packages/localize/src/number/getGroupSeparator.js b/packages/localize/src/number/getGroupSeparator.js index 6bfa21d0c..e6e7c8b11 100644 --- a/packages/localize/src/number/getGroupSeparator.js +++ b/packages/localize/src/number/getGroupSeparator.js @@ -5,9 +5,13 @@ import { normalSpaces } from './utils/normalSpaces.js'; * Gets the group separator * * @param {string} [locale] To override the browser locale + * @param {import('../../types/LocalizeMixinTypes').FormatNumberOptions} [options] * @returns {string} */ -export function getGroupSeparator(locale) { +export function getGroupSeparator(locale, options) { + if (options && options.groupSeparator) { + return options.groupSeparator; + } const computedLocale = getLocale(locale); const formattedNumber = Intl.NumberFormat(computedLocale, { style: 'decimal', diff --git a/packages/localize/src/number/getSeparatorsFromNumber.js b/packages/localize/src/number/getSeparatorsFromNumber.js new file mode 100644 index 000000000..318bb5764 --- /dev/null +++ b/packages/localize/src/number/getSeparatorsFromNumber.js @@ -0,0 +1,58 @@ +/** + * + * @param {number} parsedNumber + * @param {string} formattedNumber + * @param {import('../../types/LocalizeMixinTypes').FormatNumberOptions} [options] + * @returns {{groupSeparator: string|null, decimalSeparator: string|null}} + */ +export function getSeparatorsFromNumber(parsedNumber, formattedNumber, options) { + // separator can only happen if there is at least 1 digit before and after the separator + // eslint-disable-next-line no-irregular-whitespace + const regexSeparator = /[0-9](?[\s,. _ '])[0-9]/g; + + /** @type {string[]} */ + const separators = []; + let match; + // eslint-disable-next-line no-cond-assign + while ((match = regexSeparator.exec(formattedNumber)) !== null) { + if (match.groups && match.groups.sep) { + separators.push(match.groups?.sep); + } + } + + let groupSeparator = null; + let decimalSeparator = null; + if (separators) { + if (separators.length === 1) { + const parts = formattedNumber.split(separators[0]); + // Not sure if decimal or group, because only 1 separator. + // if the separator is followed by at least 3 or more digits + // and if the original number value is more or equal than 1000 or less or equal than -1000 + // or the minimum integer digits is forced to more than 3, + // it has to be the group separator + if ( + parts[1].replace(/[^0-9]/g, '').length >= 3 && + (parsedNumber >= 1000 || + parsedNumber <= -1 * 1000 || + (options?.minimumIntegerDigits && options.minimumIntegerDigits > 3)) + ) { + [groupSeparator] = separators; + } else { + [decimalSeparator] = separators; + } + } else if (separators.every(val => val === separators[0])) { + // multiple separators, check if they are all the same or not + // if the same, it means they are group separators + // if not, it means that the last one must be the decimal separator + [groupSeparator] = separators; + } else { + [groupSeparator] = separators; + decimalSeparator = separators[separators.length - 1]; + } + } + + return { + groupSeparator, + decimalSeparator, + }; +} diff --git a/packages/localize/src/number/parseNumber.js b/packages/localize/src/number/parseNumber.js index f76a20740..74066a9c6 100644 --- a/packages/localize/src/number/parseNumber.js +++ b/packages/localize/src/number/parseNumber.js @@ -94,7 +94,7 @@ function parseHeuristic(value) { // 1. put placeholder at decimal separator const numberString = value .replace(/(,|\.)([^,|.]*)$/g, '_decSep_$2') - .replace(/(,|\.| )/g, '') // 2. remove all thousand separators + .replace(/(,|\.| )/g, '') // 2. remove all group separators .replace(/_decSep_/, '.'); // 3. restore decimal separator return parseFloat(numberString); } diff --git a/packages/localize/test/number/formatNumber.test.js b/packages/localize/test/number/formatNumber.test.js index 259232b52..c68bfab16 100644 --- a/packages/localize/test/number/formatNumber.test.js +++ b/packages/localize/test/number/formatNumber.test.js @@ -179,6 +179,64 @@ describe('formatNumber', () => { maximumFractionDigits: 2, }), ).to.equal('112.345.678,00'); + expect( + formatNumber(112345678, { + style: 'decimal', + minimumFractionDigits: 2, + maximumFractionDigits: 2, + groupSeparator: ' ', + decimalSeparator: '.', + }), + ).to.equal('112 345 678.00'); + }); + + it('throws when decimal and group separator are the same value, only when problematic', () => { + localize.locale = 'nl-NL'; + const fn = () => + formatNumber(112345678, { + style: 'decimal', + minimumFractionDigits: 2, + maximumFractionDigits: 2, + decimalSeparator: '.', // same as group separator for nl-NL + }); + + expect(fn).to.throw(`Decimal and group (thousand) separator are the same character: '.'. +This can happen due to both props being specified as the same, or one of the props being the same as the other one from default locale. +Please specify .groupSeparator / .decimalSeparator on the formatOptions object to be different.`); + + const fn2 = () => + formatNumber(112345678, { + style: 'decimal', + minimumFractionDigits: 2, + maximumFractionDigits: 2, + groupSeparator: ',', + decimalSeparator: ',', + }); + + expect(fn2).to.throw(`Decimal and group (thousand) separator are the same character: ','. +This can happen due to both props being specified as the same, or one of the props being the same as the other one from default locale. +Please specify .groupSeparator / .decimalSeparator on the formatOptions object to be different.`); + + // this one doesn't end up with decimals, so not a problem + const fn3 = () => + formatNumber(112345678, { + groupSeparator: ',', + decimalSeparator: ',', + }); + + expect(fn3).to.not.throw(); + + // this one doesn't end up with group separators (<1000), so not a problem + const fn4 = () => + formatNumber(112.345678, { + style: 'decimal', + minimumFractionDigits: 2, + maximumFractionDigits: 2, + groupSeparator: ',', + decimalSeparator: ',', + }); + + expect(fn4).to.not.throw(); }); it('formats 2-digit decimals correctly', () => { diff --git a/packages/localize/test/number/getSeparatorsFromNumber.test.js b/packages/localize/test/number/getSeparatorsFromNumber.test.js new file mode 100644 index 000000000..3a5eebc55 --- /dev/null +++ b/packages/localize/test/number/getSeparatorsFromNumber.test.js @@ -0,0 +1,48 @@ +import { expect } from '@open-wc/testing'; + +import { getSeparatorsFromNumber } from '../../src/number/getSeparatorsFromNumber.js'; + +describe('getSeparatorsFromNumber', () => { + it('returns group separator for locale', () => { + expect(getSeparatorsFromNumber(99, '99.00')).to.eql({ + groupSeparator: null, + decimalSeparator: '.', + }); + expect(getSeparatorsFromNumber(1000, '1,000')).to.eql({ + groupSeparator: ',', + decimalSeparator: null, + }); + expect(getSeparatorsFromNumber(12345678901, '12,345,678.901')).to.eql({ + groupSeparator: ',', + decimalSeparator: '.', + }); + expect(getSeparatorsFromNumber(12345678901, '12_345_678_901')).to.eql({ + groupSeparator: '_', + decimalSeparator: null, + }); + expect(getSeparatorsFromNumber(123, '123,00 €')).to.eql({ + groupSeparator: null, + decimalSeparator: ',', + }); + expect(getSeparatorsFromNumber(123, '€123,00')).to.eql({ + groupSeparator: null, + decimalSeparator: ',', + }); + expect(getSeparatorsFromNumber(1234, '123.400 dollar')).to.eql({ + groupSeparator: '.', + decimalSeparator: null, + }); + expect(getSeparatorsFromNumber(1234.5, '1 234,50 €')).to.eql({ + groupSeparator: ' ', + decimalSeparator: ',', + }); + expect(getSeparatorsFromNumber(-1234, '-1,234')).to.eql({ + groupSeparator: ',', + decimalSeparator: null, + }); + expect(getSeparatorsFromNumber(123, '0,123', { minimumIntegerDigits: 4 })).to.eql({ + groupSeparator: ',', + decimalSeparator: null, + }); + }); +}); diff --git a/packages/localize/types/LocalizeMixinTypes.d.ts b/packages/localize/types/LocalizeMixinTypes.d.ts index 8f102c72b..adfd58c80 100644 --- a/packages/localize/types/LocalizeMixinTypes.d.ts +++ b/packages/localize/types/LocalizeMixinTypes.d.ts @@ -19,7 +19,6 @@ export declare interface FormatDateOptions extends Intl.DateTimeFormatOptions { roundMode?: string; returnIfNaN?: string; - decimalSeparator?: string; mode?: 'pasted' | 'auto'; postProcessors?: Map; @@ -35,7 +34,11 @@ export declare interface FormatNumberOptions extends Intl.NumberFormatOptions { numberingSystem?: string; roundMode?: string; returnIfNaN?: string; - decimalSeparator?: string; + // https://en.wikipedia.org/wiki/Decimal_separator#Current_standards + decimalSeparator?: ',' | '.'; + // https://en.wikipedia.org/wiki/Decimal_separator#Digit_grouping + // note the half space in there as well + groupSeparator?: ',' | '.' | ' ' | '_' | ' ' | "'"; mode?: 'pasted' | 'auto'; postProcessors?: Map;