fix(clients): locale-aware thousands/decimal separator parsing in MoneyInput

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()
+    })
+  })
 })

clients/packages/checkout/src/components/CheckoutPWYWForm.tsx

@@ -135,6 +135,7 @@ export const CheckoutPWYWForm = ({
                     onChange={field.onChange}
                     placeholder={0}
                     disabled={field.disabled}
+                    locale={locale}
                   />
                   <FormMessage />
                 </FormItem>

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,13 +39,18 @@ const MoneyInput = (props: Props) => {
     onFocus,
     disabled,
     step = 0.1,
+    locale = 'en',
   } = props
 
   const decimalFactor = useMemo(
     () => getCurrencyDecimalFactor(currency),
     [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 = (

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('')
+    })
+  })
+})