NumberField: Add locale support for number formatting #1603
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
WalkthroughNumberField was refactored to use locale-aware string handling: context now provides Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant NI as NumberFieldInput
participant NC as NumberFieldContext
participant NR as NumberFieldRoot
participant NF as Intl.NumberFormat
U->>NI: Type/edit text (keys / Enter / arrows)
NI->>NC: handleOnChange(rawString)
NC->>NR: onChange(rawString)
rect rgba(200,230,255,0.18)
note right of NR: Locale-aware parsing & validation
NR->>NR: detect decimal separator (locale)
NR->>NR: normalize string → numeric or empty
NR->>NR: clamp to min/max if numeric
NR->>NF: format numeric value (locale)
NF-->>NR: formatted string
end
NR-->>NC: update { inputValue, formattedValue, locale }
NC-->>NI: rerender with formattedValue
NI->>NC: onFocus / onBlur events
NC->>NR: propagate focus/blur callbacks
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)src/components/ui/NumberField/fragments/NumberFieldRoot.tsx (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
src/components/ui/NumberField/contexts/NumberFieldContext.tsx(2 hunks)src/components/ui/NumberField/fragments/NumberFieldInput.tsx(3 hunks)src/components/ui/NumberField/fragments/NumberFieldRoot.tsx(1 hunks)src/components/ui/NumberField/stories/NumberField.stories.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/ui/NumberField/fragments/NumberFieldRoot.tsx (1)
src/core/hooks/useControllableState/index.tsx (1)
useControllableState(15-78)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build
- GitHub Check: coverage
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
src/components/ui/NumberField/fragments/NumberFieldInput.tsx (1)
15-26: Restore mobile keypad + improve a11y for text input spinbuttonSwitching to type="text" drops numeric keypad hints and built‑in semantics. Add inputMode/pattern for better virtual keyboards and expose ARIA spinbutton attributes. Requires min/max to be provided by context (see Root comment).
const { - formattedValue, + formattedValue, handleOnChange, handleStep, id, name, disabled, readOnly, required, - rootClass, + rootClass, onFocus, onBlur, + // optional; add in Root context to enable these aria props + min, + max, } = context; @@ - <input + <input ref={ref} type="text" onKeyDown={handleKeyDown} value={formattedValue} onChange={(e) => handleOnChange(e.target.value)} onFocus={onFocus} onBlur={onBlur} + inputMode="decimal" + pattern="[0-9]+([.,][0-9]+)?" + role="spinbutton" + aria-valuemin={typeof min === 'number' ? min : undefined} + aria-valuemax={typeof max === 'number' ? max : undefined} + aria-valuenow={ + formattedValue === '' || readOnly ? undefined : (context.inputValue as number | undefined) + } id={id} name={name} disabled={disabled} readOnly={readOnly} required={required}Also applies to: 50-58
src/components/ui/NumberField/contexts/NumberFieldContext.tsx (1)
3-17: Context shape looks good; consider naming clarity for handlerThe additions make sense. Minor: handleOnChange now carries a string; renaming to onInputStringChange would reduce ambiguity with onValueChange at the Root level. Optional.
src/components/ui/NumberField/stories/NumberField.stories.tsx (1)
107-120: Add a locale with non‑Latin digits to validate parsingGreat to see en‑GB. Please add an Arabic or Persian example (e.g., ar‑EG) and a negative value to exercise minus sign and localized numerals.
export const WithLocale = () => ( <SandboxEditor> <div style={{ display: 'flex', flexDirection: 'column', gap: '16px' }}> <div> <p>British (en-GB)</p> <NumberField.Root defaultValue={1234567.89} locale="en-GB" step={1} largeStep={100}> <NumberField.Decrement>-</NumberField.Decrement> <NumberField.Input style={{ width: '250px' }} /> <NumberField.Increment>+</NumberField.Increment> </NumberField.Root> </div> + <div> + <p>Arabic (ar-EG) with negative</p> + <NumberField.Root defaultValue={-1234.56} locale="ar-EG" step={0.1} largeStep={10}> + <NumberField.Decrement>-</NumberField.Decrement> + <NumberField.Input style={{ width: '250px' }} /> + <NumberField.Increment>+</NumberField.Increment> + </NumberField.Root> + </div> </div> </SandboxEditor> );src/components/ui/NumberField/fragments/NumberFieldRoot.tsx (4)
35-41: Formatting effect handles empty state correctly; cache formatter for perfNice fix for empty string. Consider memoizing the formatter to avoid re-creating Intl.NumberFormat on every inputValue/typing toggle.
- useEffect(() => { + const numberFormatter = React.useMemo( + () => new Intl.NumberFormat(locale || 'en-US', { maximumFractionDigits: 20 }), + [locale] + ); + useEffect(() => { if (!isTyping) { - const formatted = inputValue === '' ? '' : new Intl.NumberFormat(locale ? locale : 'en-US', { maximumFractionDigits: 20 }).format(inputValue); + const formatted = inputValue === '' ? '' : numberFormatter.format(inputValue); setStringValue(formatted); } - }, [inputValue, isTyping, locale]); + }, [inputValue, isTyping, numberFormatter]);
42-45: Expose all separators and minus sign onceYou’ll need group and minusSign for robust parsing. Compute them once.
- const getDecimalSeparator = (locale: string) => { - const parts = new Intl.NumberFormat(locale).formatToParts(1234.5); - return parts.find(part => part.type === 'decimal')?.value || '.'; - } + const localeTokens = React.useMemo(() => { + const nf = new Intl.NumberFormat(locale || 'en-US'); + const parts = nf.formatToParts(1234567.89); + const decimal = parts.find(p => p.type === 'decimal')?.value || '.'; + const group = parts.find(p => p.type === 'group')?.value || ','; + const minusSign = new Intl.NumberFormat(locale || 'en-US') + .formatToParts(-1) + .find(p => p.type === 'minusSign')?.value || '-'; + return { decimal, group, minusSign }; + }, [locale]);
92-97: On focus, show locale digits without grouping for a smoother edit experiencetoString() yields ASCII digits and ignores numbering systems. Use Intl.NumberFormat with useGrouping: false so users in locales like ar‑EG see expected numerals while editing.
- const handleFocus = (e: React.FocusEvent<HTMLInputElement>) => { - setIsTyping(true); - const currentStringValue = inputValue === '' ? '' : inputValue.toString().replace('.', getDecimalSeparator(locale || 'en-US')); - setStringValue(currentStringValue); + const handleFocus = (e: React.FocusEvent<HTMLInputElement>) => { + setIsTyping(true); + const nfNoGroup = new Intl.NumberFormat(locale || 'en-US', { useGrouping: false, maximumFractionDigits: 20 }); + const currentStringValue = inputValue === '' ? '' : nfNoGroup.format(inputValue as number); + setStringValue(currentStringValue); onFocus?.(e); };
150-164: Expose min/max in context to unlock ARIA on the InputPropagate min and max so NumberFieldInput can set aria‑valuemin/max and improve accessibility.
const contextValues = { inputValue, formattedValue: stringValue, locale, handleOnChange, handleStep, id, name, + min, + max, disabled, readOnly, required, rootClass, onFocus: handleFocus, onBlur: handleBlur, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/ui/NumberField/contexts/NumberFieldContext.tsx(1 hunks)src/components/ui/NumberField/fragments/NumberFieldInput.tsx(3 hunks)src/components/ui/NumberField/fragments/NumberFieldRoot.tsx(4 hunks)src/components/ui/NumberField/stories/NumberField.stories.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/ui/NumberField/fragments/NumberFieldRoot.tsx (1)
src/core/hooks/useControllableState/index.tsx (1)
useControllableState(15-78)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build
- GitHub Check: coverage
🔇 Additional comments (2)
src/components/ui/NumberField/fragments/NumberFieldRoot.tsx (2)
47-86: Note: minus sign handling improved; empty->"" fix looks goodLeading '-' is preserved during typing and empty string no longer formats to 0. Remaining gaps covered above.
Also applies to: 105-127
47-86: Locale digits are not parsed; Arabic/Indic numerals break input and stepSanitization only keeps [0-9] and the decimal separator. This rejects valid numerals in locales that use non‑Latin digits and breaks typing/stepping in those locales. Also only ASCII '-' is recognized.
- const parseAndSetValue = (val: string) => { + // Map localized digits to ASCII and normalize separators before parsing + const buildDigitMap = React.useCallback((loc: string) => { + const map = new Map<string, string>(); + for (let d = 0; d <= 9; d++) { + map.set(d.toLocaleString(loc), String(d)); + } + return map; + }, []); + const digitMap = React.useMemo(() => buildDigitMap(locale || 'en-US'), [buildDigitMap, locale]); + const mapDigitsToAscii = (s: string) => { + let out = ''; + for (const ch of s) out += digitMap.get(ch) ?? ch; + return out; + }; + + const parseAndSetValue = (val: string) => { if (val === '') { setInputValue(''); return; } - - const decimal = getDecimalSeparator(locale || 'en-US'); - let isNegative = false; - let numberString = val; - - if (val.startsWith('-')) { - isNegative = true; - numberString = val.substring(1); - } - - const regex = new RegExp(`[^0-9${decimal}]`, 'g'); - const cleaned = numberString.replace(regex, ''); - const normalized = cleaned.replace(decimal, '.'); - let numericValue = parseFloat(normalized); + const { decimal, group, minusSign } = localeTokens; + const trimmed = val.trim(); + const hasMinus = trimmed.startsWith('-') || trimmed.startsWith(minusSign); + const withoutSign = hasMinus + ? trimmed.slice(trimmed.startsWith(minusSign) ? minusSign.length : 1) + : trimmed; + // Convert localized digits to ASCII, strip grouping, keep only first decimal + const ascii = mapDigitsToAscii(withoutSign).split(group).join(''); + const parts = ascii.split(decimal); + const normalized = parts[0] + (parts.length > 1 ? '.' + parts.slice(1).join('') : ''); + let numericValue = parseFloat(normalized); if (isNaN(numericValue)) { // If parsing fails (e.g., user just typed "-"), do nothing with the numeric value // but keep the string value for the input. return; } - if (isNegative) { + if (hasMinus) { numericValue *= -1; } if (max !== undefined && numericValue > max) { numericValue = max; } if (min !== undefined && numericValue < min) { numericValue = min; } setInputValue(numericValue); }
Comment on lines
+45
to
+47
| if (event.key === 'Enter') { | ||
| (event.target as HTMLInputElement).blur(); | ||
| } |
There was a problem hiding this comment.
Prevent unintended form submission on Enter
Pressing Enter blurs the input but still allows default form submission. Prevent it to avoid accidental submits while committing the value.
- if (event.key === 'Enter') {
- (event.target as HTMLInputElement).blur();
- }
+ if (event.key === 'Enter') {
+ event.preventDefault();
+ (event.target as HTMLInputElement).blur();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (event.key === 'Enter') { | |
| (event.target as HTMLInputElement).blur(); | |
| } | |
| if (event.key === 'Enter') { | |
| event.preventDefault(); | |
| (event.target as HTMLInputElement).blur(); | |
| } |
🤖 Prompt for AI Agents
In src/components/ui/NumberField/fragments/NumberFieldInput.tsx around lines 45
to 47, the Enter key handler currently blurs the input but does not prevent the
default form submission; update the handler so when event.key === 'Enter' you
call event.preventDefault() (and optionally event.stopPropagation()) before
blurring to stop the browser submitting the form, and ensure the event is typed
as a KeyboardEvent<HTMLInputElement> so preventDefault is available.
Comment on lines
105
to
127
| const applyStep = (amount: number) => { | ||
| setInputValue((prev) => { | ||
| let temp = prev; | ||
| if (temp === '') { | ||
| if (min !== undefined) { | ||
| temp = min; | ||
| } else { | ||
| temp = -1; | ||
| } | ||
| } | ||
| const nextValue = temp + amount; | ||
|
|
||
| if (max !== undefined && nextValue > max) { | ||
| return max; | ||
| } | ||
|
|
||
| if (min !== undefined && nextValue < min) { | ||
| return min; | ||
| } | ||
|
|
||
| return nextValue; | ||
| }); | ||
| const valueToStep = isTyping ? parseFloat(stringValue.replace(getDecimalSeparator(locale || 'en-US'), '.')) : (inputValue || 0); | ||
|
|
||
| let startingValue = valueToStep; | ||
| if(isNaN(valueToStep)) { | ||
| startingValue = 0; | ||
| } | ||
|
|
||
| let nextValue = startingValue + amount; | ||
|
|
||
| if (max !== undefined && nextValue > max) { | ||
| nextValue = max; | ||
| } | ||
|
|
||
| if (min !== undefined && nextValue < min) { | ||
| nextValue = min; | ||
| } | ||
|
|
||
| setInputValue(nextValue); | ||
| const nextStringValue = nextValue.toString().replace('.', getDecimalSeparator(locale || 'en-US')); | ||
| setStringValue(nextStringValue); | ||
| }; |
There was a problem hiding this comment.
Stepping while typing fails for localized digits and can expose float artifacts
- parseFloat on the raw string ignores localized numerals.
- Using JS float math can surface 0.30000000000004, etc.
Normalize input using the same mapping as parseAndSetValue and round to the precision of the step amount.
- const applyStep = (amount: number) => {
- const valueToStep = isTyping ? parseFloat(stringValue.replace(getDecimalSeparator(locale || 'en-US'), '.')) : (inputValue || 0);
-
- let startingValue = valueToStep;
- if(isNaN(valueToStep)) {
- startingValue = 0;
- }
-
- let nextValue = startingValue + amount;
+ const decimalsOf = (n: number) => {
+ const s = String(n);
+ return s.includes('.') ? s.split('.')[1].length : 0;
+ };
+ const applyStep = (amount: number) => {
+ const { decimal, group } = localeTokens;
+ let startingValue: number;
+ if (isTyping) {
+ // normalize localized input while typing
+ const ascii = mapDigitsToAscii(stringValue).split(group).join('');
+ const parts = ascii.split(decimal);
+ const normalized = parts[0] + (parts.length > 1 ? '.' + parts.slice(1).join('') : '');
+ const parsed = parseFloat(normalized);
+ startingValue = isNaN(parsed) ? 0 : parsed;
+ } else {
+ startingValue = (inputValue as number) || 0;
+ }
+ // round to precision of the step to avoid float artifacts
+ let nextValue = Number((startingValue + amount).toFixed(decimalsOf(amount)));
if (max !== undefined && nextValue > max) {
nextValue = max;
}
if (min !== undefined && nextValue < min) {
nextValue = min;
}
setInputValue(nextValue);
-
- const nextStringValue = nextValue.toString().replace('.', getDecimalSeparator(locale || 'en-US'));
- setStringValue(nextStringValue);
+ const nfNoGroup = new Intl.NumberFormat(locale || 'en-US', { useGrouping: false, maximumFractionDigits: 20 });
+ setStringValue(nfNoGroup.format(nextValue));
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const applyStep = (amount: number) => { | |
| setInputValue((prev) => { | |
| let temp = prev; | |
| if (temp === '') { | |
| if (min !== undefined) { | |
| temp = min; | |
| } else { | |
| temp = -1; | |
| } | |
| } | |
| const nextValue = temp + amount; | |
| if (max !== undefined && nextValue > max) { | |
| return max; | |
| } | |
| if (min !== undefined && nextValue < min) { | |
| return min; | |
| } | |
| return nextValue; | |
| }); | |
| const valueToStep = isTyping ? parseFloat(stringValue.replace(getDecimalSeparator(locale || 'en-US'), '.')) : (inputValue || 0); | |
| let startingValue = valueToStep; | |
| if(isNaN(valueToStep)) { | |
| startingValue = 0; | |
| } | |
| let nextValue = startingValue + amount; | |
| if (max !== undefined && nextValue > max) { | |
| nextValue = max; | |
| } | |
| if (min !== undefined && nextValue < min) { | |
| nextValue = min; | |
| } | |
| setInputValue(nextValue); | |
| const nextStringValue = nextValue.toString().replace('.', getDecimalSeparator(locale || 'en-US')); | |
| setStringValue(nextStringValue); | |
| }; | |
| // Helper to count decimal places in the step amount | |
| const decimalsOf = (n: number) => { | |
| const s = String(n); | |
| return s.includes('.') ? s.split('.')[1].length : 0; | |
| }; | |
| const applyStep = (amount: number) => { | |
| const { decimal, group } = localeTokens; | |
| let startingValue: number; | |
| if (isTyping) { | |
| // Normalize localized input while typing | |
| // 1) Map locale digits to ASCII | |
| // 2) Strip grouping separators | |
| // 3) Replace decimal separator with '.' | |
| const ascii = mapDigitsToAscii(stringValue).split(group).join(''); | |
| const parts = ascii.split(decimal); | |
| const normalized = parts[0] + (parts.length > 1 ? '.' + parts.slice(1).join('') : ''); | |
| const parsed = parseFloat(normalized); | |
| startingValue = isNaN(parsed) ? 0 : parsed; | |
| } else { | |
| startingValue = (inputValue as number) || 0; | |
| } | |
| // Round to the precision of the step to avoid float artifacts | |
| let nextValue = Number((startingValue + amount).toFixed(decimalsOf(amount))); | |
| if (max !== undefined && nextValue > max) { | |
| nextValue = max; | |
| } | |
| if (min !== undefined && nextValue < min) { | |
| nextValue = min; | |
| } | |
| setInputValue(nextValue); | |
| // Format without grouping, preserving locale-specific decimal separator | |
| const nfNoGroup = new Intl.NumberFormat(locale || 'en-US', { | |
| useGrouping: false, | |
| maximumFractionDigits: 20, | |
| }); | |
| setStringValue(nfNoGroup.format(nextValue)); | |
| }; |
🤖 Prompt for AI Agents
In src/components/ui/NumberField/fragments/NumberFieldRoot.tsx around lines 105
to 127, the applyStep function currently uses parseFloat on the raw string
(which ignores localized numerals) and plain JS float arithmetic (which can
expose artifacts like 0.30000000000004); change it to normalize the input using
the same parsing/mapping logic as parseAndSetValue (convert localized digits and
decimal separator into a canonical number), derive the precision from the step
amount (count decimal places of step), perform the arithmetic on integers or use
rounding to that precision after adding the step, then enforce min/max and
setInputValue/setStringValue using the localized formatting (replace '.' with
locale decimal separator and format to the determined precision) so stepping
works with localized digits and avoids float artifacts.
Comment on lines
20
to
+68
| export const Controlled = () => { | ||
| const [value, setValue] = React.useState<number | ''>(3); | ||
| return ( | ||
| <SandboxEditor> | ||
| <NumberField.Root value={value} onValueChange={setValue} defaultValue={3} step={1} min={0} max={10} largeStep={5}> | ||
| <NumberField.Decrement>-</NumberField.Decrement> | ||
| <NumberField.Input /> | ||
| <NumberField.Increment>+</NumberField.Increment> | ||
| </NumberField.Root> | ||
| <div style={{ marginTop: 8 }}>Current value: {value}</div> | ||
| </SandboxEditor> | ||
| ); | ||
| const [value, setValue] = React.useState<number | "">(3); | ||
| return ( | ||
| <SandboxEditor> | ||
| <NumberField.Root | ||
| value={value} | ||
| onValueChange={setValue} | ||
| defaultValue={3} | ||
| step={1} | ||
| min={0} | ||
| max={10} | ||
| largeStep={5} | ||
| > | ||
| <NumberField.Decrement>-</NumberField.Decrement> | ||
| <NumberField.Input style={{ width: '250px' }} /> | ||
| <NumberField.Increment>+</NumberField.Increment> | ||
| </NumberField.Root> | ||
| <div style={{ marginTop: 8 }}>Current value: {value}</div> | ||
| </SandboxEditor> | ||
| ); |
There was a problem hiding this comment.
Avoid passing both value and defaultValue in controlled story
This triggers the dev warning in useControllableState and can confuse consumers. Drop defaultValue when the component is controlled.
<NumberField.Root
value={value}
onValueChange={setValue}
- defaultValue={3}
step={1}
min={0}
max={10}
largeStep={5}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const Controlled = () => { | |
| const [value, setValue] = React.useState<number | ''>(3); | |
| return ( | |
| <SandboxEditor> | |
| <NumberField.Root value={value} onValueChange={setValue} defaultValue={3} step={1} min={0} max={10} largeStep={5}> | |
| <NumberField.Decrement>-</NumberField.Decrement> | |
| <NumberField.Input /> | |
| <NumberField.Increment>+</NumberField.Increment> | |
| </NumberField.Root> | |
| <div style={{ marginTop: 8 }}>Current value: {value}</div> | |
| </SandboxEditor> | |
| ); | |
| const [value, setValue] = React.useState<number | "">(3); | |
| return ( | |
| <SandboxEditor> | |
| <NumberField.Root | |
| value={value} | |
| onValueChange={setValue} | |
| defaultValue={3} | |
| step={1} | |
| min={0} | |
| max={10} | |
| largeStep={5} | |
| > | |
| <NumberField.Decrement>-</NumberField.Decrement> | |
| <NumberField.Input style={{ width: '250px' }} /> | |
| <NumberField.Increment>+</NumberField.Increment> | |
| </NumberField.Root> | |
| <div style={{ marginTop: 8 }}>Current value: {value}</div> | |
| </SandboxEditor> | |
| ); | |
| export const Controlled = () => { | |
| const [value, setValue] = React.useState<number | "">(3); | |
| return ( | |
| <SandboxEditor> | |
| <NumberField.Root | |
| value={value} | |
| onValueChange={setValue} | |
| step={1} | |
| min={0} | |
| max={10} | |
| largeStep={5} | |
| > | |
| <NumberField.Decrement>-</NumberField.Decrement> | |
| <NumberField.Input style={{ width: '250px' }} /> | |
| <NumberField.Increment>+</NumberField.Increment> | |
| </NumberField.Root> | |
| <div style={{ marginTop: 8 }}>Current value: {value}</div> | |
| </SandboxEditor> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In src/components/ui/NumberField/stories/NumberField.stories.tsx around lines 49
to 68, the Controlled story passes both value and defaultValue which triggers a
dev warning and confuses consumers; remove the defaultValue prop from the
NumberField.Root when using controlled value (keep value, onValueChange, step,
min, max, largeStep) so the component is purely controlled and retain the local
React state initialization as needed.
Comment on lines
34
to
+105
| export const FormExample = () => { | ||
| const [fieldValue, setFieldValue] = React.useState<number | ''>(2); | ||
| const [submitted, setSubmitted] = React.useState<number | null>(null); | ||
| return ( | ||
| <SandboxEditor> | ||
| <form | ||
| onSubmit={e => { | ||
| e.preventDefault(); | ||
| setSubmitted(fieldValue === '' ? null : fieldValue); | ||
| }} | ||
| > | ||
| <NumberField.Root name="quantity" value={fieldValue} onValueChange={setFieldValue} defaultValue={2} step={1} min={0} max={10} largeStep={5}> | ||
| const [fieldValue, setFieldValue] = React.useState<number | "">(2); | ||
| const [submitted, setSubmitted] = React.useState<number | null>(null); | ||
| return ( | ||
| <SandboxEditor> | ||
| <form | ||
| onSubmit={(e) => { | ||
| e.preventDefault(); | ||
| setSubmitted(fieldValue === "" ? null : fieldValue); | ||
| }} | ||
| > | ||
| <NumberField.Root | ||
| name="quantity" | ||
| value={fieldValue} | ||
| onValueChange={setFieldValue} | ||
| defaultValue={2} | ||
| step={1} | ||
| min={0} | ||
| max={10} | ||
| largeStep={5} | ||
| > | ||
| <NumberField.Decrement>-</NumberField.Decrement> | ||
| <NumberField.Input style={{ width: '250px' }} /> | ||
| <NumberField.Increment>+</NumberField.Increment> | ||
| </NumberField.Root> | ||
| <button type="submit" style={{ marginTop: 8 }}> | ||
| Submit | ||
| </button> | ||
| </form> | ||
| {submitted !== null && ( | ||
| <div style={{ marginTop: 8 }}>Submitted value: {submitted}</div> | ||
| )} | ||
| </SandboxEditor> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Same issue in FormExample: remove defaultValue when controlled
The form passes value/onValueChange and defaultValue simultaneously.
<NumberField.Root
name="quantity"
value={fieldValue}
onValueChange={setFieldValue}
- defaultValue={2}
step={1}
min={0}
max={10}
largeStep={5}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const FormExample = () => { | |
| const [fieldValue, setFieldValue] = React.useState<number | ''>(2); | |
| const [submitted, setSubmitted] = React.useState<number | null>(null); | |
| return ( | |
| <SandboxEditor> | |
| <form | |
| onSubmit={e => { | |
| e.preventDefault(); | |
| setSubmitted(fieldValue === '' ? null : fieldValue); | |
| }} | |
| > | |
| <NumberField.Root name="quantity" value={fieldValue} onValueChange={setFieldValue} defaultValue={2} step={1} min={0} max={10} largeStep={5}> | |
| const [fieldValue, setFieldValue] = React.useState<number | "">(2); | |
| const [submitted, setSubmitted] = React.useState<number | null>(null); | |
| return ( | |
| <SandboxEditor> | |
| <form | |
| onSubmit={(e) => { | |
| e.preventDefault(); | |
| setSubmitted(fieldValue === "" ? null : fieldValue); | |
| }} | |
| > | |
| <NumberField.Root | |
| name="quantity" | |
| value={fieldValue} | |
| onValueChange={setFieldValue} | |
| defaultValue={2} | |
| step={1} | |
| min={0} | |
| max={10} | |
| largeStep={5} | |
| > | |
| <NumberField.Decrement>-</NumberField.Decrement> | |
| <NumberField.Input style={{ width: '250px' }} /> | |
| <NumberField.Increment>+</NumberField.Increment> | |
| </NumberField.Root> | |
| <button type="submit" style={{ marginTop: 8 }}> | |
| Submit | |
| </button> | |
| </form> | |
| {submitted !== null && ( | |
| <div style={{ marginTop: 8 }}>Submitted value: {submitted}</div> | |
| )} | |
| </SandboxEditor> | |
| ); | |
| }; | |
| <NumberField.Root | |
| name="quantity" | |
| value={fieldValue} | |
| onValueChange={setFieldValue} | |
| step={1} | |
| min={0} | |
| max={10} | |
| largeStep={5} | |
| > |
🤖 Prompt for AI Agents
In src/components/ui/NumberField/stories/NumberField.stories.tsx around lines 71
to 105, the NumberField.Root is being used as a controlled component (it
receives value and onValueChange) but also provides defaultValue; remove the
defaultValue prop to avoid mixing controlled and uncontrolled behavior and rely
on the value/onValueChange pair to control the component (or alternatively make
it uncontrolled by removing value and onValueChange if that was intended).
BarriosXJavier
force-pushed
the
barrios/add-api-support-numberfield-locale-1601
branch
from
6159427 to
4a53d56
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implementation allows numbers to be formatted according to different regional conventions.
The NumberField.Root component accepts a locale prop that uses the Intl.NumberFormat API to format the displayed value based on the provided locale.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation