diff --git a/clients/packages/checkout/src/components/CheckoutPWYWForm.test.tsx b/clients/packages/checkout/src/components/CheckoutPWYWForm.test.tsx index c1093ba37de..7cd1badb3ed 100644 --- a/clients/packages/checkout/src/components/CheckoutPWYWForm.test.tsx +++ b/clients/packages/checkout/src/components/CheckoutPWYWForm.test.tsx @@ -1,4 +1,5 @@ import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { type AcceptedLocale } from '@polar-sh/i18n' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { createBaseCheckout, @@ -11,6 +12,7 @@ function renderPWYW( amount?: number minimumAmount?: number currency?: string + locale?: AcceptedLocale } = {}, ) { const update = vi.fn() @@ -27,7 +29,7 @@ function renderPWYW( update={update} checkout={checkout} productPrice={productPrice} - locale="en" + locale={overrides.locale ?? 'en'} />, ) @@ -134,4 +136,106 @@ describe('CheckoutPWYWForm', () => { expect(update).not.toHaveBeenCalled() }) }) + + describe('locale-aware thousands/decimal separator parsing', () => { + beforeEach(() => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it.each([ + ['5,000', 500000], + ['1,234', 123400], + ['12,345', 1234500], + ['1,000,000', 100000000], + ] as const)( + 'treats a comma as a thousands separator for en: %p -> %i cents', + async (value, amount) => { + const { update } = renderPWYW({ amount: 1500, minimumAmount: 500 }) + const input = screen.getByRole('textbox') as HTMLInputElement + + fireEvent.change(input, { target: { value } }) + + await act(async () => { + await vi.advanceTimersByTimeAsync(600) + }) + + expect(update).toHaveBeenCalledWith({ amount }) + }, + ) + + it('preserves a period decimal together with thousands separators for en', async () => { + const { update } = renderPWYW({ amount: 1500, minimumAmount: 500 }) + const input = screen.getByRole('textbox') as HTMLInputElement + + fireEvent.change(input, { target: { value: '5,000.99' } }) + + await act(async () => { + await vi.advanceTimersByTimeAsync(600) + }) + + expect(update).toHaveBeenCalledWith({ amount: 500099 }) + }) + + it('still rounds a period decimal correctly for en', async () => { + const { update } = renderPWYW({ amount: 1500, minimumAmount: 500 }) + const input = screen.getByRole('textbox') as HTMLInputElement + + fireEvent.change(input, { target: { value: '5.55' } }) + + await act(async () => { + await vi.advanceTimersByTimeAsync(600) + }) + + expect(update).toHaveBeenCalledWith({ amount: 555 }) + }) + + it.each([ + ['12,5', 1250], + ['12,50', 1250], + ['12,500', 1250], + ['1.234,56', 123456], + ] as const)( + 'treats a comma as the decimal separator for de: %p -> %i cents', + async (value, amount) => { + const { update } = renderPWYW({ + amount: 1500, + minimumAmount: 500, + currency: 'eur', + locale: 'de', + }) + const input = screen.getByRole('textbox') as HTMLInputElement + + fireEvent.change(input, { target: { value } }) + + await act(async () => { + await vi.advanceTimersByTimeAsync(600) + }) + + expect(update).toHaveBeenCalledWith({ amount }) + }, + ) + + it('does not collapse a US thousands-separated amount below the minimum', async () => { + // Regression for the reported bug: "1,234" used to be parsed as 123 cents + // (123400 intended), which fell below the $5 minimum and surfaced a + // misleading "below minimum" error instead of the parser bug. + const { update } = renderPWYW({ amount: 1500, minimumAmount: 500 }) + const input = screen.getByRole('textbox') as HTMLInputElement + + fireEvent.change(input, { target: { value: '1,234' } }) + + await act(async () => { + await vi.advanceTimersByTimeAsync(600) + }) + + expect(update).toHaveBeenCalledWith({ amount: 123400 }) + expect( + screen.queryByText(/Amount must be at least/i), + ).not.toBeInTheDocument() + }) + }) }) diff --git a/clients/packages/checkout/src/components/CheckoutPWYWForm.tsx b/clients/packages/checkout/src/components/CheckoutPWYWForm.tsx index 87a9557c1fc..107ca0a1da4 100644 --- a/clients/packages/checkout/src/components/CheckoutPWYWForm.tsx +++ b/clients/packages/checkout/src/components/CheckoutPWYWForm.tsx @@ -135,6 +135,7 @@ export const CheckoutPWYWForm = ({ onChange={field.onChange} placeholder={0} disabled={field.disabled} + locale={locale} /> diff --git a/clients/packages/checkout/src/components/ui/MoneyInput.tsx b/clients/packages/checkout/src/components/ui/MoneyInput.tsx index 23fe6515170..6cc55e031c7 100644 --- a/clients/packages/checkout/src/components/ui/MoneyInput.tsx +++ b/clients/packages/checkout/src/components/ui/MoneyInput.tsx @@ -1,4 +1,9 @@ -import { getCurrencyDecimalFactor, isDecimalCurrency } from '@polar-sh/currency' +import { + getCurrencyDecimalFactor, + getLocaleDecimalSeparator, + isDecimalCurrency, + parseMoneyValue, +} from '@polar-sh/currency' import { ChangeEvent, FocusEvent, useCallback, useMemo, useState } from 'react' import { cn } from '@polar-sh/ui/lib/utils' import { Input } from './Input' @@ -17,6 +22,7 @@ interface Props { preSlot?: React.ReactNode postSlot?: React.ReactNode step?: number + locale?: string } const MoneyInput = (props: Props) => { @@ -33,6 +39,7 @@ const MoneyInput = (props: Props) => { onFocus, disabled, step = 0.1, + locale = 'en', } = props const decimalFactor = useMemo( @@ -40,6 +47,10 @@ const MoneyInput = (props: Props) => { [currency], ) const isNonDecimalCurrency = !isDecimalCurrency(currency) + const decimalSeparator = useMemo( + () => getLocaleDecimalSeparator(locale), + [locale], + ) const getInternalValue = useCallback( (value: number | null | undefined): string | undefined => { @@ -116,54 +127,13 @@ const MoneyInput = (props: Props) => { return } - // Strip everything except numbers, commas, and periods - // (people can paste in anything, so the keydown handler is not enough) - const cleaned = input.replace(/[^0-9,.]/g, '') - - // By default, parse the full value as a whole number, stripping out decimal separators - // - // Leave a trailing comma, otherwise people can't type in decimals - // (the onBlur handler will strip it if it's dangling on blur) - let newValue = cleaned.replace(/[,.](?!$)/g, '').replace(/,$/, '.') - - // However, if we detect a decimal separator, round it to 2 decimal places - // - // We support period decimal separator (enforced when typing) - // but also support comma decimal separators (might be pasted in) - const decimalMatch = cleaned.match(/([.,])([0-9]+)$/) - - if (decimalMatch) { - const maxDecimalPrecision = 2 - const decimalPart = decimalMatch[2] - const integerPart = cleaned - .slice(0, -decimalMatch[0].length) - .replace(/[,.]/g, '') - const trimmedDecimalPart = decimalPart.slice(0, maxDecimalPrecision) - - const parsedValue = Number.parseFloat( - `${integerPart}.${trimmedDecimalPart}`, - ) - - if (!Number.isNaN(parsedValue)) { - const decimalPlaces = Math.min( - maxDecimalPrecision, - decimalPart.length, - ) - const formatted = parsedValue.toFixed(decimalPlaces) - - // This covers when the user deletes the last integer part and prevents inserting a `0` - // in place of the last integer part that would make the caret jump to the end of the input - // This way the user can continue typing - newValue = - integerPart.length > 0 - ? formatted - : formatted.replace(/^0(?=\.)/, '') - } - } - - updateValue(newValue) + // Locale-aware parsing of the typed/pasted value. For period-decimal + // locales (en, …) a comma is a thousands separator so "5,000" stays 5000; + // for comma-decimal locales (de, …) a comma is the decimal separator so + // "12,50" becomes 12.50. See @polar-sh/currency `parseMoneyValue`. + updateValue(parseMoneyValue(input, decimalSeparator)) }, - [updateValue, isNonDecimalCurrency], + [updateValue, isNonDecimalCurrency, decimalSeparator], ) const onBlur = useCallback( @@ -246,15 +216,18 @@ const MoneyInput = (props: Props) => { updateValue(newValue) } - // Prevent multiple decimal points + // Prevent multiple decimal points. The committed value is displayed with + // a period (toFixed), and the locale's own decimal separator may be a + // comma, so block a second separator once either is already present. if ( (e.key === '.' || e.key === ',') && - e.currentTarget.value.includes('.') + (e.currentTarget.value.includes(decimalSeparator) || + e.currentTarget.value.includes('.')) ) { e.preventDefault() } }, - [step, updateValue, isNonDecimalCurrency], + [step, updateValue, isNonDecimalCurrency, decimalSeparator], ) const currencyLabel = ( diff --git a/clients/packages/currency/src/index.test.ts b/clients/packages/currency/src/index.test.ts index 5c8b049b8c3..40cdaf4fd26 100644 --- a/clients/packages/currency/src/index.test.ts +++ b/clients/packages/currency/src/index.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest' -import { formatCurrency } from './index' +import { + formatCurrency, + getLocaleDecimalSeparator, + parseMoneyValue, +} from './index' describe('formatCurrency', () => { describe('Compact mode', () => { @@ -197,3 +201,127 @@ describe('formatCurrency', () => { }) }) }) + +describe('getLocaleDecimalSeparator', () => { + it.each([ + ['en', '.'], + ['en-US', '.'], + ['ja', '.'], + ['ko', '.'], + ['de', ','], + ['de-DE', ','], + ['fr', ','], + ['fr-FR', ','], + ['es', ','], + ['it', ','], + ['nl', ','], + ['pt', ','], + ['pt-PT', ','], + ['sv', ','], + ['tr', ','], + ['pl', ','], + ['hu', ','], + ] as const)('returns the decimal separator for %s', (locale, expected) => { + expect(getLocaleDecimalSeparator(locale)).toBe(expected) + }) +}) + +describe('parseMoneyValue', () => { + describe('period-decimal locales (en) — comma is a thousands separator', () => { + it.each([ + ['5', '5'], + ['5.5', '5.5'], + ['5.55', '5.55'], + ['5000', '5000'], + ['5,000', '5000'], + ['1,234', '1234'], + ['12,345', '12345'], + ['1,000,000', '1000000'], + ['1,000,000,000', '1000000000'], + ['5,000.99', '5000.99'], + ['1,000.50', '1000.50'], + ['12,345.67', '12345.67'], + ['1,234,567.89', '1234567.89'], + ['0.25', '0.25'], + ['0.99', '0.99'], + ] as const)( + 'parses %p -> %p (no thousands collapse)', + (input, expected) => { + expect(parseMoneyValue(input, '.')).toBe(expected) + }, + ) + + it('strips a trailing comma as a thousands separator (not a decimal)', () => { + expect(parseMoneyValue('5,', '.')).toBe('5') + }) + + it('treats a mid-string comma as thousands while typing groups', () => { + expect(parseMoneyValue('5,0', '.')).toBe('50') + expect(parseMoneyValue('5,00', '.')).toBe('500') + expect(parseMoneyValue('5,000', '.')).toBe('5000') + }) + + it('keeps a trailing period so the user can type decimals', () => { + expect(parseMoneyValue('5.', '.')).toBe('5.') + expect(parseMoneyValue('12.', '.')).toBe('12.') + }) + + it('rounds the fractional part to 2 digits', () => { + expect(parseMoneyValue('5.555', '.')).toBe('5.55') + expect(parseMoneyValue('5.999', '.')).toBe('5.99') + expect(parseMoneyValue('1,234.567', '.')).toBe('1234.56') + }) + + it('does not reinsert a leading zero when the integer part is empty', () => { + expect(parseMoneyValue('.5', '.')).toBe('.5') + expect(parseMoneyValue('.55', '.')).toBe('.55') + }) + + it('strips everything except digits, commas and periods', () => { + expect(parseMoneyValue('$5,000.99', '.')).toBe('5000.99') + expect(parseMoneyValue('abc 1,234.56 xyz', '.')).toBe('1234.56') + expect(parseMoneyValue('5 000,99', '.')).toBe('500099') + }) + + it('returns an empty string for empty / non-numeric input', () => { + expect(parseMoneyValue('', '.')).toBe('') + expect(parseMoneyValue('abc', '.')).toBe('') + }) + }) + + describe('comma-decimal locales (de) — comma is the decimal separator', () => { + it.each([ + ['12,5', '12.5'], + ['12,50', '12.50'], + ['12,500', '12.50'], + ['1,99', '1.99'], + ['1.234,56', '1234.56'], + ['12.345,67', '12345.67'], + ['1.234.567,89', '1234567.89'], + ] as const)('parses %p -> %p (European convention)', (input, expected) => { + expect(parseMoneyValue(input, ',')).toBe(expected) + }) + + it('keeps a trailing comma so the user can type decimals', () => { + expect(parseMoneyValue('12,', ',')).toBe('12.') + }) + + it('treats a pasted period (no comma) as the decimal of an edited value', () => { + // The component displays committed values with a period (toFixed), so + // editing "12.50" must round-trip, and a European paste of "1.234" + // (thousands-only) is a known pre-existing edge, not a regression. + expect(parseMoneyValue('12.50', ',')).toBe('12.50') + expect(parseMoneyValue('12.5', ',')).toBe('12.5') + }) + + it('rounds the fractional part to 2 digits', () => { + expect(parseMoneyValue('12,555', ',')).toBe('12.55') + expect(parseMoneyValue('1.234,999', ',')).toBe('1234.99') + }) + + it('returns an empty string for empty / non-numeric input', () => { + expect(parseMoneyValue('', ',')).toBe('') + expect(parseMoneyValue('abc', ',')).toBe('') + }) + }) +}) diff --git a/clients/packages/currency/src/index.ts b/clients/packages/currency/src/index.ts index ff5574bc2d3..2b6cd79eaf4 100644 --- a/clients/packages/currency/src/index.ts +++ b/clients/packages/currency/src/index.ts @@ -252,3 +252,123 @@ export const formatCurrency = return formatCurrencySubcent(cents, currency, locales) } } + +/** + * Returns the decimal separator (`.` or `,`) used by a given locale to format + * fractional numbers. + * + * Money inputs need this to decide whether a typed/pasted comma is a decimal + * separator (e.g. `de` → `12,50`) or a thousands separator (e.g. `en` → `5,000`). + * Defaults to `.` when the locale resolves to a period-separated format. + * + * @param locale - BCP-47 locale tag (e.g. `'en'`, `'de'`, `'fr-FR'`) + * @returns `'.'` or `','` + * @example + * getLocaleDecimalSeparator('en') // '.' + * getLocaleDecimalSeparator('de') // ',' + */ +export const getLocaleDecimalSeparator = ( + locale: Intl.LocalesArgument, +): '.' | ',' => { + const formatted = new Intl.NumberFormat(locale).format(1.1) + return formatted.includes(',') ? ',' : '.' +} + +/** + * Normalizes a raw money input string into a display string using `.` as the + * decimal separator, interpreting separators according to the locale's decimal + * separator. + * + * Used by `MoneyInput` to convert what a user types or pastes into a stable + * display value before it is turned into minor units (cents). + * + * Rules: + * - Strips everything except digits, commas, and periods. + * - The locale's decimal separator, when present, is the decimal point; the + * other separator is treated as a thousands separator and stripped. + * - For period-decimal locales (`en`, `ja`, …) a comma is always a thousands + * separator, so `5,000` → `5000` (never collapsed to `5.00`). + * - For comma-decimal locales (`de`, `fr`, …) the committed value is displayed + * with a period (see `MoneyInput`'s `getInternalValue`, which uses + * `toFixed`), so a period in an input that has no comma is treated as the + * decimal point to keep editing those values correct (e.g. `12.50` → `12.50`). + * - At most one decimal separator is honored (the last one); earlier + * occurrences are stripped from the integer part. + * - The fractional part is rounded to at most 2 digits. + * - A trailing decimal separator with no fractional digits yet (e.g. `5.`) is + * preserved so the user can keep typing decimals; a blur handler is expected + * to strip it. + * + * @param input - Raw input string (may contain any characters) + * @param decimalSeparator - The locale's decimal separator (`.` or `,`) + * @returns A normalized display string with `.` as the decimal separator + * @example + * parseMoneyValue('5,000', '.') // '5000' + * parseMoneyValue('5,000.99', '.') // '5000.99' + * parseMoneyValue('12,50', ',') // '12.50' + * parseMoneyValue('1.234,56', ',') // '1234.56' + * parseMoneyValue('12.50', ',') // '12.50' (editing a period-displayed value) + */ +export const parseMoneyValue = ( + input: string, + decimalSeparator: '.' | ',', +): string => { + const cleaned = input.replace(/[^0-9,.]/g, '') + if (cleaned === '') return '' + + // Determine which separator acts as the decimal point in this input. + // - The locale's decimal separator is preferred when present. + // - For comma-decimal locales the committed value is displayed with a + // period, so a period in an input that has no comma is treated as the + // decimal to keep editing those values correct. + // - For period-decimal locales a lone comma is always a thousands separator + // (the reported bug: "5,000" must not collapse to "5.00"). + let decimalSep: '.' | ',' | null + if (cleaned.includes(decimalSeparator)) { + decimalSep = decimalSeparator + } else if (decimalSeparator === ',' && cleaned.includes('.')) { + decimalSep = '.' + } else { + decimalSep = null + } + + if (decimalSep === null) { + // No decimal point: strip any thousands separators from the pure integer. + const thousandsSeparator = decimalSeparator === '.' ? ',' : '.' + return cleaned.split(thousandsSeparator).join('') + } + + // Strip the thousands separator (the character that is not the decimal point) + const thousandsSeparator = decimalSep === '.' ? ',' : '.' + const withoutThousands = cleaned.split(thousandsSeparator).join('') + const lastSepIndex = withoutThousands.lastIndexOf(decimalSep) + + // Integer part: everything before the last decimal separator, with any + // earlier stray decimal separators stripped + const integerPart = withoutThousands + .slice(0, lastSepIndex) + .split(decimalSep) + .join('') + const decimalPart = withoutThousands.slice(lastSepIndex + 1) + + // Trailing decimal separator with no decimals yet (e.g. "5." or "12,"): + // keep it so the user can continue typing decimals + if (decimalPart === '') { + return integerPart.length > 0 ? `${integerPart}.` : '.' + } + + const maxDecimalPrecision = 2 + const trimmedDecimalPart = decimalPart.slice(0, maxDecimalPrecision) + const parsedValue = Number.parseFloat(`${integerPart}.${trimmedDecimalPart}`) + + if (Number.isNaN(parsedValue)) { + return integerPart + } + + const decimalPlaces = Math.min(maxDecimalPrecision, decimalPart.length) + const formatted = parsedValue.toFixed(decimalPlaces) + + // When the integer part was deleted, avoid reinserting a leading "0" that + // would make the caret jump to the end of the input + return integerPart.length > 0 ? formatted : formatted.replace(/^0(?=\.)/, '') +} diff --git a/clients/packages/ui/src/components/atoms/MoneyInput.tsx b/clients/packages/ui/src/components/atoms/MoneyInput.tsx index f9f34e083de..0280852d44a 100644 --- a/clients/packages/ui/src/components/atoms/MoneyInput.tsx +++ b/clients/packages/ui/src/components/atoms/MoneyInput.tsx @@ -1,4 +1,9 @@ -import { getCurrencyDecimalFactor, isDecimalCurrency } from '@polar-sh/currency' +import { + getCurrencyDecimalFactor, + getLocaleDecimalSeparator, + isDecimalCurrency, + parseMoneyValue, +} from '@polar-sh/currency' import { ChangeEvent, FocusEvent, useCallback, useMemo, useState } from 'react' import { twMerge } from 'tailwind-merge' import { Input } from '@polar-sh/orbit' @@ -17,6 +22,7 @@ interface Props { preSlot?: React.ReactNode postSlot?: React.ReactNode step?: number + locale?: string } const MoneyInput = (props: Props) => { @@ -33,6 +39,7 @@ const MoneyInput = (props: Props) => { onFocus, disabled, step = 0.1, + locale = 'en', } = props const decimalFactor = useMemo( @@ -40,6 +47,10 @@ const MoneyInput = (props: Props) => { [currency], ) const isNonDecimalCurrency = !isDecimalCurrency(currency) + const decimalSeparator = useMemo( + () => getLocaleDecimalSeparator(locale), + [locale], + ) const getInternalValue = useCallback( (value: number | null | undefined): string | undefined => { @@ -116,54 +127,13 @@ const MoneyInput = (props: Props) => { return } - // Strip everything except numbers, commas, and periods - // (people can paste in anything, so the keydown handler is not enough) - const cleaned = input.replace(/[^0-9,.]/g, '') - - // By default, parse the full value as a whole number, stripping out decimal separators - // - // Leave a trailing comma, otherwise people can't type in decimals - // (the onBlur handler will strip it if it's dangling on blur) - let newValue = cleaned.replace(/[,.](?!$)/g, '').replace(/,$/, '.') - - // However, if we detect a decimal separator, round it to 2 decimal places - // - // We support period decimal separator (enforced when typing) - // but also support comma decimal separators (might be pasted in) - const decimalMatch = cleaned.match(/([.,])([0-9]+)$/) - - if (decimalMatch) { - const maxDecimalPrecision = 2 - const decimalPart = decimalMatch[2] - const integerPart = cleaned - .slice(0, -decimalMatch[0].length) - .replace(/[,.]/g, '') - const trimmedDecimalPart = decimalPart.slice(0, maxDecimalPrecision) - - const parsedValue = Number.parseFloat( - `${integerPart}.${trimmedDecimalPart}`, - ) - - if (!Number.isNaN(parsedValue)) { - const decimalPlaces = Math.min( - maxDecimalPrecision, - decimalPart.length, - ) - const formatted = parsedValue.toFixed(decimalPlaces) - - // This covers when the user deletes the last integer part and prevents inserting a `0` - // in place of the last integer part that would make the caret jump to the end of the input - // This way the user can continue typing - newValue = - integerPart.length > 0 - ? formatted - : formatted.replace(/^0(?=\.)/, '') - } - } - - updateValue(newValue) + // Locale-aware parsing of the typed/pasted value. For period-decimal + // locales (en, …) a comma is a thousands separator so "5,000" stays 5000; + // for comma-decimal locales (de, …) a comma is the decimal separator so + // "12,50" becomes 12.50. See @polar-sh/currency `parseMoneyValue`. + updateValue(parseMoneyValue(input, decimalSeparator)) }, - [updateValue, isNonDecimalCurrency], + [updateValue, isNonDecimalCurrency, decimalSeparator], ) const onBlur = useCallback( @@ -246,15 +216,18 @@ const MoneyInput = (props: Props) => { updateValue(newValue) } - // Prevent multiple decimal points + // Prevent multiple decimal points. The committed value is displayed with + // a period (toFixed), and the locale's own decimal separator may be a + // comma, so block a second separator once either is already present. if ( (e.key === '.' || e.key === ',') && - e.currentTarget.value.includes('.') + (e.currentTarget.value.includes(decimalSeparator) || + e.currentTarget.value.includes('.')) ) { e.preventDefault() } }, - [step, updateValue, isNonDecimalCurrency], + [step, updateValue, isNonDecimalCurrency, decimalSeparator], ) const currencyLabel = (