Skip to content

Commit 242337e

Browse files
lempiralempira
authored
fix: correct thousand separator detection for certain locales (#565)
* fix: correct thousand separator detection for Spanish/Portuguese/Italian locales Spanish (es-ES), Portuguese (pt-PT), and Italian (it-IT) only apply thousand grouping for numbers ≥10,000. Using 1000 caused the function to fall back to comma, creating a separator collision error in forms. - Change getThousandSeparator to use 10000 instead of 1000 - Add defensive validation in NumericFormat component - Add comprehensive test coverage for es-ES locale - Add integration test for Spanish locale in dispenser * chore: updated nsprc file for change in vuln id * chore: ignored linting for console warn
1 parent ace283d commit 242337e

File tree

5 files changed

+121
-8
lines changed

5 files changed

+121
-8
lines changed

‎.nsprc‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,6 @@
1212
"1112954": {
1313
"active": true,
1414
"expiry": "2026-02-15",
15-
"notes": "@isaacs/brace-expansion version 5.0.0 is bundled inside npm (semantic-release/npm) and can't be override. We need to wait for a fix from npm"
16-
},
17-
"1112810": {
18-
"active": true,
19-
"expiry": "2026-02-15",
20-
"notes": "There is no fix for npm yet"
15+
"notes": "@isaacs/brace-expansion version 5.0.0 is bundled inside npm and can't be override. We need to wait for a fix from npm"
2116
}
2217
}

‎src/features/forms/components/number-form-item.tsx‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,19 @@ const NumericFormatWithRef = forwardRef<HTMLInputElement, NumericFormatWithRefPr
2424
const localeDecimalSeparator = useMemo(() => getDecimalSeparator(locale), [locale])
2525
const localeThousandsGroupStyle = useMemo(() => getThousandsGroupStyle(locale), [locale])
2626

27+
// Defensive validation to prevent react-number-format error
28+
// This should never trigger but provides safety
29+
const safeThousandSeparator = useMemo(() => {
30+
if (thousandSeparator && localeThousandSeparator === localeDecimalSeparator) {
31+
// eslint-disable-next-line no-console
32+
console.warn(
33+
`Thousand separator (${localeThousandSeparator}) conflicts with decimal separator (${localeDecimalSeparator}) for locale ${locale}. Disabling thousand separator.`
34+
)
35+
return false
36+
}
37+
return thousandSeparator ? localeThousandSeparator : false
38+
}, [thousandSeparator, localeThousandSeparator, localeDecimalSeparator, locale])
39+
2740
return (
2841
<NumericFormat
2942
id={field}
@@ -35,7 +48,7 @@ const NumericFormatWithRef = forwardRef<HTMLInputElement, NumericFormatWithRefPr
3548
defaultValue=""
3649
getInputRef={ref}
3750
value={value === undefined ? '' : value.toString()}
38-
thousandSeparator={thousandSeparator ? localeThousandSeparator : false}
51+
thousandSeparator={safeThousandSeparator}
3952
thousandsGroupStyle={localeThousandsGroupStyle}
4053
decimalSeparator={localeDecimalSeparator}
4154
decimalScale={decimalScale ?? 0}

‎src/features/fund/fund-page.test.tsx‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,4 +257,83 @@ describe('fund-page', () => {
257257
})
258258
})
259259
})
260+
261+
describe('Spanish locale number formatting', () => {
262+
it('should render fund form without separator collision error in Spanish locale', async () => {
263+
// Save and mock Spanish locale
264+
const originalLanguage = Object.getOwnPropertyDescriptor(navigator, 'language')
265+
Object.defineProperty(navigator, 'language', {
266+
get: () => 'es-ES',
267+
configurable: true,
268+
})
269+
270+
// Mock dispenser API
271+
vi.spyOn(dispenserApi, 'useDispenserApi').mockImplementation(() => ({
272+
fundLimit: {
273+
state: 'hasData',
274+
data: algos(10),
275+
},
276+
fundAccount: vi.fn(),
277+
refundStatus: {
278+
state: 'hasData',
279+
data: {
280+
canRefund: true,
281+
limit: algos(10),
282+
},
283+
},
284+
refundDispenserAccount: vi.fn(),
285+
}))
286+
287+
vi.spyOn(activeWallet, 'useLoadableActiveWalletAccountSnapshotAtom').mockImplementation(() => ({
288+
state: 'hasData',
289+
data: undefined,
290+
}))
291+
292+
// Render dispenser with Spanish locale
293+
const router = createMemoryRouter(
294+
[
295+
{
296+
path: '/testnet/fund',
297+
element: (
298+
<DataProvider
299+
networkConfig={{
300+
id: testnetId,
301+
...defaultNetworkConfigs[testnetId],
302+
}}
303+
>
304+
<TooltipProvider>
305+
<DispenserApiLoggedIn
306+
networkConfig={{
307+
id: testnetId,
308+
...defaultNetworkConfigs[testnetId],
309+
}}
310+
/>
311+
</TooltipProvider>
312+
</DataProvider>
313+
),
314+
},
315+
],
316+
{ initialEntries: ['/testnet/fund'] }
317+
)
318+
319+
rtlRender(<RouterProvider router={router} />)
320+
321+
// Verify form renders without errors
322+
await waitFor(() => {
323+
expect(screen.getByText('Fund an existing TestNet account with ALGO')).toBeTruthy()
324+
})
325+
326+
// Verify amount input is accessible (would fail if separator collision occurred)
327+
await waitFor(() => {
328+
const amountInput = screen.getByRole('textbox', { name: /amount/i }) as HTMLInputElement
329+
expect(amountInput).toBeInTheDocument()
330+
expect(amountInput).not.toBeDisabled()
331+
})
332+
333+
// Restore original language
334+
if (originalLanguage) {
335+
Object.defineProperty(navigator, 'language', originalLanguage)
336+
}
337+
})
338+
})
260339
})

‎src/tests/utils/number-format.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ describe('number-format', () => {
2121
expect(getThousandSeparator('en-US')).toBe(',')
2222
expect(getThousandSeparator('de-DE')).toBe('.')
2323
expect(getThousandSeparator('fr-FR')).toBe('\u202f')
24+
expect(getThousandSeparator('es-ES')).toBe('.')
2425
expect(getThousandSeparator('ar-SA')).toBe('٬')
2526
expect(getThousandSeparator('hi-IN')).toBe(',')
2627
expect(getThousandSeparator('zh-CN')).toBe(',')
@@ -30,13 +31,27 @@ describe('number-format', () => {
3031
it('returns a string for default locale when not specified', () => {
3132
expect(typeof getThousandSeparator()).toBe('string')
3233
})
34+
35+
it('returns different thousand and decimal separators for comma-decimal locales', () => {
36+
// Spanish uses period for thousand and comma for decimal
37+
expect(getThousandSeparator('es-ES')).not.toBe(getDecimalSeparator('es-ES'))
38+
expect(getThousandSeparator('es-ES')).toBe('.')
39+
expect(getDecimalSeparator('es-ES')).toBe(',')
40+
41+
// German also uses period for thousand and comma for decimal
42+
expect(getThousandSeparator('de-DE')).not.toBe(getDecimalSeparator('de-DE'))
43+
44+
// French uses space for thousand and comma for decimal
45+
expect(getThousandSeparator('fr-FR')).not.toBe(getDecimalSeparator('fr-FR'))
46+
})
3347
})
3448

3549
describe('getDecimalSeparator', () => {
3650
it('returns the correct decimal separator for each locale', () => {
3751
expect(getDecimalSeparator('en-US')).toBe('.')
3852
expect(getDecimalSeparator('de-DE')).toBe(',')
3953
expect(getDecimalSeparator('fr-FR')).toBe(',')
54+
expect(getDecimalSeparator('es-ES')).toBe(',')
4055
expect(getDecimalSeparator('ar-SA')).toBe('٫')
4156
expect(getDecimalSeparator('hi-IN')).toBe('.')
4257
expect(getDecimalSeparator('zh-CN')).toBe('.')
@@ -54,6 +69,7 @@ describe('number-format', () => {
5469
expect(formatDecimalAmount(value, 'en-US')).toBe('1,234.56')
5570
expect(formatDecimalAmount(value, 'de-DE')).toBe('1.234,56')
5671
expect(formatDecimalAmount(value, 'fr-FR')).toBe('1\u202f234,56')
72+
expect(formatDecimalAmount(value, 'es-ES')).toBe('1234,56')
5773
expect(formatDecimalAmount(value, 'ar-SA')).toBe('1,234.56')
5874
expect(formatDecimalAmount(value, 'hi-IN')).toBe('1,234.56')
5975
expect(formatDecimalAmount(value, 'zh-CN')).toBe('1,234.56')
@@ -65,6 +81,7 @@ describe('number-format', () => {
6581
expect(formatDecimalAmount(value, 'en-US')).toBe('100,000,000')
6682
expect(formatDecimalAmount(value, 'de-DE')).toBe('100.000.000')
6783
expect(formatDecimalAmount(value, 'fr-FR')).toBe('100\u202f000\u202f000')
84+
expect(formatDecimalAmount(value, 'es-ES')).toBe('100.000.000')
6885
expect(formatDecimalAmount(value, 'ar-SA')).toBe('100,000,000')
6986
expect(formatDecimalAmount(value, 'hi-IN')).toBe('10,00,00,000')
7087
expect(formatDecimalAmount(value, 'zh-CN')).toBe('100,000,000')
@@ -76,6 +93,7 @@ describe('number-format', () => {
7693
expect(formatDecimalAmount(value, 'en-US')).toBe('1,000')
7794
expect(formatDecimalAmount(value, 'de-DE')).toBe('1.000')
7895
expect(formatDecimalAmount(value, 'fr-FR')).toBe('1\u202f000')
96+
expect(formatDecimalAmount(value, 'es-ES')).toBe('1000')
7997
expect(formatDecimalAmount(value, 'ar-SA')).toBe('1,000')
8098
expect(formatDecimalAmount(value, 'hi-IN')).toBe('1,000')
8199
expect(formatDecimalAmount(value, 'zh-CN')).toBe('1,000')
@@ -87,6 +105,7 @@ describe('number-format', () => {
87105
expect(formatDecimalAmount(value, 'en-US')).toBe('0.123456')
88106
expect(formatDecimalAmount(value, 'de-DE')).toBe('0,123456')
89107
expect(formatDecimalAmount(value, 'fr-FR')).toBe('0,123456')
108+
expect(formatDecimalAmount(value, 'es-ES')).toBe('0,123456')
90109
expect(formatDecimalAmount(value, 'ar-SA')).toBe('0.123456')
91110
expect(formatDecimalAmount(value, 'hi-IN')).toBe('0.123456')
92111
expect(formatDecimalAmount(value, 'zh-CN')).toBe('0.123456')
@@ -98,6 +117,7 @@ describe('number-format', () => {
98117
expect(formatDecimalAmount(value, 'en-US')).toBe('1,234.123456789')
99118
expect(formatDecimalAmount(value, 'de-DE')).toBe('1.234,123456789')
100119
expect(formatDecimalAmount(value, 'fr-FR')).toBe('1\u202f234,123456789')
120+
expect(formatDecimalAmount(value, 'es-ES')).toBe('1234,123456789')
101121
expect(formatDecimalAmount(value, 'ar-SA')).toBe('1,234.123456789')
102122
expect(formatDecimalAmount(value, 'hi-IN')).toBe('1,234.123456789')
103123
expect(formatDecimalAmount(value, 'zh-CN')).toBe('1,234.123456789')
@@ -109,6 +129,7 @@ describe('number-format', () => {
109129
expect(formatDecimalAmount(value, 'en-US')).toBe('12,345,678,901,234,567,890.12345')
110130
expect(formatDecimalAmount(value, 'de-DE')).toBe('12.345.678.901.234.567.890,12345')
111131
expect(formatDecimalAmount(value, 'fr-FR')).toBe('12\u202f345\u202f678\u202f901\u202f234\u202f567\u202f890,12345')
132+
expect(formatDecimalAmount(value, 'es-ES')).toBe('12.345.678.901.234.567.890,12345')
112133
expect(formatDecimalAmount(value, 'ar-SA')).toBe('12,345,678,901,234,567,890.12345')
113134
expect(formatDecimalAmount(value, 'hi-IN')).toBe('1,23,45,67,89,01,23,45,67,890.12345')
114135
expect(formatDecimalAmount(value, 'zh-CN')).toBe('12,345,678,901,234,567,890.12345')
@@ -120,6 +141,7 @@ describe('number-format', () => {
120141
expect(formatDecimalAmount(value, 'en-US')).toBe('-1,234.56')
121142
expect(formatDecimalAmount(value, 'de-DE')).toBe('-1.234,56')
122143
expect(formatDecimalAmount(value, 'fr-FR')).toBe('-1\u202f234,56')
144+
expect(formatDecimalAmount(value, 'es-ES')).toBe('-1234,56')
123145
expect(formatDecimalAmount(value, 'ar-SA')).toBe('\u200e-1,234.56')
124146
expect(formatDecimalAmount(value, 'hi-IN')).toBe('-1,234.56')
125147
expect(formatDecimalAmount(value, 'zh-CN')).toBe('-1,234.56')

‎src/utils/number-format.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,14 @@ export const getLocale = (): string => {
1414

1515
/**
1616
* Get the thousand separator for the given locale.
17+
*
18+
* Note: Uses 10000 instead of 1000 because some locales (like Spanish)
19+
* only apply grouping to numbers >= 10,000. This ensures the 'group'
20+
* part is present in formatToParts result.
1721
*/
1822
export const getThousandSeparator = (locale?: string): string => {
1923
const resolvedLocale = locale || getLocale()
20-
const parts = new Intl.NumberFormat(resolvedLocale).formatToParts(1000)
24+
const parts = new Intl.NumberFormat(resolvedLocale).formatToParts(10000)
2125
const groupPart = parts.find((part) => part.type === 'group')
2226
return groupPart?.value || ','
2327
}

0 commit comments

Comments
 (0)