Skip to content

Commit

Permalink
fix: numeric input field rounding errors
Browse files Browse the repository at this point in the history
  • Loading branch information
alexfreska committed Jan 5, 2024
1 parent aae13a9 commit e470c16
Show file tree
Hide file tree
Showing 9 changed files with 231 additions and 191 deletions.
6 changes: 6 additions & 0 deletions .changeset/moody-plums-collect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'hostd': minor
'renterd': minor
---

Fixed an issue where siacoin and numeric input values would jump to an incorrect value.
2 changes: 1 addition & 1 deletion libs/design-system/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@
"clipboard-polyfill": "^4.0.1",
"@visx/xychart": "^2.18.0",
"react-dropzone": "^14.2.3",
"react-number-format": "^5.3.1",
"@radix-ui/react-radio-group": "^1.0.0",
"@radix-ui/react-accordion": "^1.0.0",
"@radix-ui/react-avatar": "^1.0.0",
"react-currency-input-field": "^3.6.5",
"@radix-ui/react-checkbox": "^1.0.0",
"framer-motion": "^7.6.5",
"@radix-ui/react-dialog": "^1.0.0",
Expand Down
130 changes: 91 additions & 39 deletions libs/design-system/src/core/BaseNumberField.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import CurrencyInput from 'react-currency-input-field'
import { textFieldStyles } from './TextField'
import { Text } from './Text'
import { cx } from 'class-variance-authority'
import { VariantProps } from '../types'
import { useEffect, useState } from 'react'
import { NumericFormat } from 'react-number-format'
import { useEffect, useMemo, useState } from 'react'

type Props = VariantProps<typeof textFieldStyles> &
Omit<React.ComponentProps<typeof CurrencyInput>, 'size' | 'className'> & {
Omit<React.ComponentProps<typeof NumericFormat>, 'size' | 'className'> & {
units?: string
}

Expand All @@ -16,10 +16,10 @@ export function BaseNumberField({
size = 'small',
state,
noSpin,
thousandsGroupStyle: customThousandsGroupStyle,
focus,
cursor,
className,
decimalsLimit,
onValueChange,
...props
}: Props) {
Expand All @@ -28,48 +28,34 @@ export function BaseNumberField({
const [locale, setLocale] = useState<string>()
useEffect(() => {
setLocale(navigator.language)
// Below code allows for dynamically changing language without refresh.
// Seems like it can cause NaN in the CurrencyInput so disabled for now.
// const onLanguageChange = () => {
// setLocale(navigator.language)
// }
// setLocale(navigator.language)
// window.addEventListener('languagechange', onLanguageChange)
// return () => {
// window.removeEventListener('languagechange', onLanguageChange)
// }
// allows for dynamically changing language without refresh
const onLanguageChange = () => {
setLocale(navigator.language)
}
setLocale(navigator.language)
window.addEventListener('languagechange', onLanguageChange)
return () => {
window.removeEventListener('languagechange', onLanguageChange)
}
}, [])

const decimalSeparator = useMemo(() => getDecimalSeparator(locale), [locale])
const { groupingSeparator, groupingStyle } = useMemo(
() => getGroupingInfo(locale, customThousandsGroupStyle),
[locale, customThousandsGroupStyle]
)

return (
<div className="relative">
<CurrencyInput
<NumericFormat
{...props}
decimalsLimit={decimalsLimit}
// For some reason decimalsLimit=0 is ignored and defaults to 2.
// Adding allowDecimals=false fixes that issue.
intlConfig={
// locale: navigator.language,
locale
? {
locale,
}
: undefined
}
allowDecimals={!!decimalsLimit}
autoComplete="off"
spellCheck={false}
onValueChange={onValueChange}
transformRawValue={(value) => {
if (value.length > 0) {
if (value[0] === '.') {
return '0.' + value.slice(1)
}
if (value[0] === ',') {
return '0,' + value.slice(1)
}
}

return value
}}
lang={locale}
decimalSeparator={decimalSeparator}
thousandsGroupStyle={groupingStyle}
thousandSeparator={groupingSeparator}
className={cx(
textFieldStyles({
variant,
Expand Down Expand Up @@ -104,3 +90,69 @@ export function BaseNumberField({
</div>
)
}

function getGroupingStyle(groups: number[]): 'thousand' | 'lakh' | 'wan' {
// Standard thousand grouping
if (groups.every((size) => size === 3)) {
return 'thousand'
}

// Indian lakh/crore grouping
if (
groups.length >= 2 &&
groups.slice(1).every((size) => size === 2) &&
groups[0] === 3
) {
return 'lakh'
}

// Chinese/Japanese wan grouping
if (groups.every((size) => size === 4)) {
return 'wan'
}

return 'thousand'
}

function getGroupingInfo(
locale: string | undefined,
customThousandsGroupStyle?: 'none' | 'thousand' | 'lakh' | 'wan'
): {
groupingSeparator: string
groupingStyle: 'thousand' | 'lakh' | 'wan' | 'none'
} {
if (customThousandsGroupStyle === 'none') {
return { groupingSeparator: '', groupingStyle: 'none' }
}

const largeNumber = 123456789
const formattedNumber = new Intl.NumberFormat(locale).format(largeNumber)

// Find the grouping separator
const groupingSeparator = formattedNumber.replace(/[0-9]/g, '')[0] // First non-numeric character

// Determine the grouping style
const groups = formattedNumber
.split(groupingSeparator)
.map((group) => group.length)

if (customThousandsGroupStyle) {
return {
groupingSeparator,
groupingStyle: customThousandsGroupStyle,
}
}

const groupingStyle = getGroupingStyle(groups)

return { groupingSeparator, groupingStyle }
}

function getDecimalSeparator(locale?: string) {
const numberWithDecimal = 1.1
const formattedNumber = new Intl.NumberFormat(locale).format(
numberWithDecimal
)
const decimalSeparator = formattedNumber[1] // Assuming '.' is at position 1
return decimalSeparator
}
40 changes: 18 additions & 22 deletions libs/design-system/src/core/NumberField.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ describe('NumberField', () => {
await user.type(input, '44')
fireEvent.blur(input)
expect(input.value).toBe('44')
expect(onChange.mock.calls.length).toBe(4)
expect(Number(onChange.mock.calls[3][0])).toBe(44)
expect(onChange.mock.calls.length).toBe(3)
expect(Number(onChange.mock.calls[2][0])).toBe(44)
await user.click(input)
await user.type(input, '4')
fireEvent.blur(input)
expect(input.value).toBe('444')
expect(onChange.mock.calls.length).toBe(6)
expect(Number(onChange.mock.calls[5][0])).toBe(444)
expectOnChangeValues([undefined, '4', '44', '44', '444', '444'], onChange)
expect(onChange.mock.calls.length).toBe(4)
expect(Number(onChange.mock.calls[3][0])).toBe(444)
expectOnChangeValues([undefined, '4', '44', '444'], onChange)
})

it('updates value starting with decimal', async () => {
Expand Down Expand Up @@ -93,20 +93,20 @@ describe('NumberField', () => {
await user.clear(input)
await user.type(input, '4444')
await user.type(input, '.5')
expect(input.value).toBe('44.445')
expect(input.value).toBe('4.444,5')
await user.type(input, ',5')
expect(input.value).toBe('44.445,5')
expect(input.value).toBe('4.444,55')
expectOnChangeValues(
[
'3333',
undefined,
'4',
'44',
'444',
'4444',
'4444',
'44445',
'44445',
'44445.5',
'4444.5',
'4444.55',
],
onChange
)
Expand All @@ -122,25 +122,25 @@ describe('NumberField', () => {
onChange,
})

expect(input.value).toBe('₽3333')
expect(input.value).toBe('₽3.333')
await user.click(input)
await user.clear(input)
await user.type(input, '4444')
await user.type(input, '.5')
expect(input.value).toBe('₽44445')
expect(input.value).toBe('₽4.444,5')
await user.type(input, ',5')
expect(input.value).toBe('₽44445,5')
expect(input.value).toBe('₽4.444,55')
expectOnChangeValues(
[
'3333',
undefined,
'4',
'44',
'444',
'4444',
'4444',
'44445',
'44445',
'44445.5',
'4444.5',
'4444.55',
],
onChange
)
Expand All @@ -162,8 +162,8 @@ describe('NumberField', () => {
fireEvent.blur(input)
// Either way limits to 6 (not rounding)
expect(input.value).toBe('0.123456')
expect(onChange.mock.calls.length).toBe(13)
expect(Number(onChange.mock.calls[12][0])).toBe(0.123456)
expect(onChange.mock.calls.length).toBe(9)
expect(Number(onChange.mock.calls[8][0])).toBe(0.123456)
expectOnChangeValues(
[
undefined,
Expand All @@ -175,10 +175,6 @@ describe('NumberField', () => {
'0.1234',
'0.12345',
'0.123456',
'0.123456',
'0.123456',
'0.123456',
'0.123456',
],
onChange
)
Expand Down
Loading

0 comments on commit e470c16

Please sign in to comment.