Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/ninety-cats-pull.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@radui/ui": patch
---

AddedsnapOnStep, smallStep support for NumberField
5 changes: 3 additions & 2 deletions src/components/ui/NumberField/contexts/NumberFieldContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ import React from 'react';

export type NumberFieldContextType = {
inputValue: number|'';
handleOnChange: (input: number|'') => void;
handleStep: (opts: { direction: 'increment' | 'decrement'; type: 'small' | 'large' }) => void;
handleOnChange: (input: number | '') => void;
handleStep: (opts: { direction: 'increment' | 'decrement'; type: 'small' | 'normal' | 'large' }) => void;
id?: string;
name?: string;
disabled?: boolean;
readOnly?: boolean;
required?: boolean;
rootClass?: string;
locale?: string;
};
Comment on lines +13 to 14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Locale added to context but unused across components.

The PR goal includes locale formatting/parsing. Passing locale without using Intl.NumberFormat for display/parse doesn’t meet the feature.

I can provide a minimal formatter/parser integration (Intl.NumberFormat + inputMode="decimal" + text input with onBlur formatting and onChange parsing). Want a patch?

🤖 Prompt for AI Agents
In src/components/ui/NumberField/contexts/NumberFieldContext.tsx around lines 13-14, the locale property is being passed in the context but never used for formatting/parsing; update the NumberField implementation to create an Intl.NumberFormat from that locale and use it to format the displayed value on blur and to parse user input on change. Specifically: derive a formatter = new Intl.NumberFormat(locale, { maximumFractionDigits, useGrouping: true }) and use inputMode="decimal" on the input; onChange keep a raw string state and parse it by stripping grouping chars and replacing the locale decimal separator with '.' then Number(parsed) (handle empty string and NaN), and onBlur replace the input value with formatter.format(parsedNumber); ensure the context exposes locale or the formatter so child components use the same formatter, and provide sensible fallbacks when locale is missing. 

const NumberFieldContext = React.createContext<NumberFieldContextType | null>(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const NumberFieldDecrement = forwardRef<NumberFieldDecrementElement, NumberField
return (
<button
ref={ref}
onClick={() => handleStep({ direction: 'decrement', type: 'small' })}
onClick={() => handleStep({ direction: 'decrement', type: 'normal' })}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

"normal" step type will not compile until context type is updated.

handleStep is invoked with type: 'normal', but NumberFieldContext.handleStep only allows 'small' | 'large'. This will cause TS errors until the context type includes 'normal'.

Fix is in NumberFieldContext.tsx (see suggested diff there).

🤖 Prompt for AI Agents
In src/components/ui/NumberField/fragments/NumberFieldDecrement.tsx around line 18, the onClick calls handleStep with type: 'normal' but NumberFieldContext.handleStep currently only accepts 'small' | 'large'; update the context type definition in src/components/ui/NumberField/NumberFieldContext.tsx to include 'normal' in the union and ensure the context implementation and any switch/handlers accept and properly handle the 'normal' case (or alternatively change this call to an allowed type), then rebuild to confirm TypeScript errors are resolved. 
className={clsx(`${rootClass}-decrement`, className)}
disabled={disabled || readOnly}
type="button"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const NumberFieldIncrement = forwardRef<NumberFieldIncrementElement, NumberField
return (
<button
ref={ref}
onClick={() => handleStep({ direction: 'increment', type: 'small' })}
onClick={() => handleStep({ direction: 'increment', type: 'normal' })}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Same type mismatch as Decrement ("normal" not in union).

handleStep({ type: 'normal' }) requires NumberFieldContext to allow 'normal'.

See the context type fix suggested in NumberFieldContext.tsx.

🤖 Prompt for AI Agents
In src/components/ui/NumberField/fragments/NumberFieldIncrement.tsx at line 19, the call handleStep({ direction: 'increment', type: 'normal' }) uses a 'normal' type that isn't part of the StepType union; update the NumberFieldContext type definition (in NumberFieldContext.tsx) to include 'normal' in the union for the step type (and any related types/interfaces), or alternatively change the call here to one of the existing allowed type values — prefer adding 'normal' to the context type so handleStep accepts { type: 'normal' } consistently across components. 
className={clsx(`${rootClass}-increment`, className)}
disabled={disabled || readOnly}
type="button"
Expand Down
15 changes: 12 additions & 3 deletions src/components/ui/NumberField/fragments/NumberFieldInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,18 @@ const NumberFieldInput = forwardRef<NumberFieldInputElement, NumberFieldInputPro
disabled,
readOnly,
required,
locale,
rootClass
} = context;

const handleKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
if (event.key === 'ArrowUp' && !event.shiftKey) {
event.preventDefault();
handleStep({ direction: 'increment', type: 'small' });
handleStep({ direction: 'increment', type: 'normal' });
}
if (event.key === 'ArrowDown' && !event.shiftKey) {
event.preventDefault();
handleStep({ direction: 'decrement', type: 'small' });
handleStep({ direction: 'decrement', type: 'normal' });
}
if (event.key === 'ArrowUp' && event.shiftKey) {
event.preventDefault();
Expand All @@ -40,13 +41,21 @@ const NumberFieldInput = forwardRef<NumberFieldInputElement, NumberFieldInputPro
event.preventDefault();
handleStep({ direction: 'decrement', type: 'large' });
}
if (event.key === 'ArrowUp' && event.altKey) {
event.preventDefault();
handleStep({ direction: 'increment', type: 'small' });
}
if (event.key === 'ArrowDown' && event.altKey) {
event.preventDefault();
handleStep({ direction: 'decrement', type: 'small' });
}
};
return (
<input
ref={ref}
type="number"
onKeyDown={handleKeyDown}
value={inputValue === '' ? '' : inputValue}
value={inputValue}
onChange={(e) => { const val = e.target.value; handleOnChange(val === '' ? '' : Number(val)); }}
id={id}
name={name}
Expand Down
58 changes: 40 additions & 18 deletions src/components/ui/NumberField/fragments/NumberFieldRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,24 @@ export type NumberFieldRootProps = {
onValueChange?: (value: number | '') => void
step?: number
largeStep?: number
smallStep?: number
snapOnStep?: boolean
locale?: string
min?: number
max?: number
disabled?: boolean
readOnly?: boolean
required?: boolean
} & ComponentPropsWithoutRef<'div'>;

const NumberFieldRoot = forwardRef<NumberFieldRootElement, NumberFieldRootProps>(({ children, name, defaultValue = '', value, onValueChange, largeStep, step, min, max, disabled, readOnly, required, id, className, ...props }, ref) => {
const NumberFieldRoot = forwardRef<NumberFieldRootElement, NumberFieldRootProps>(({ children, name, defaultValue = '', value, onValueChange, largeStep = 10, step = 1, smallStep = 0.1, snapOnStep = false, locale, min, max, disabled, readOnly, required, id, className, ...props }, ref) => {
const rootClass = customClassSwitcher(className, COMPONENT_NAME);
const [inputValue, setInputValue] = useControllableState<number | ''>(
value,
defaultValue,
onValueChange);

const handleOnChange = (input: number| '') => {
const handleOnChange = (input: number | '') => {
if (input === '') {
setInputValue('');
return;
Expand All @@ -47,44 +50,62 @@ const NumberFieldRoot = forwardRef<NumberFieldRootElement, NumberFieldRootProps>
};
const applyStep = (amount: number) => {
setInputValue((prev) => {
let temp = prev;
if (temp === '') {
if (min !== undefined) {
temp = min;
} else {
temp = -1;
}
let temp: number;
let nextValue: number;

// Handle empty input
if (prev === '' || prev === null) {
temp = min !== undefined ? min : 0;
} else {
temp = Number(prev);
}
const nextValue = temp + amount;

if (max !== undefined && nextValue > max) {
return max;
// Round to nearest step
if (temp % largeStep != 0 && snapOnStep && largeStep === Math.abs(amount)) {
temp = Math.round(temp / largeStep) * largeStep;
}

if (min !== undefined && nextValue < min) {
return min;
}
const amountDecimals = (amount.toString().split('.')[1] || '').length;
const tempDecimals = (temp.toString().split('.')[1] || '').length;
const decimal = amountDecimals > tempDecimals ? amountDecimals : tempDecimals;

// multiply temp and amount with same decimal count to remove decimal places
nextValue = (temp * Math.pow(10, decimal)) + (amount * Math.pow(10, decimal));

// divide nextValue by same decimal count
nextValue = nextValue / Math.pow(10, decimal);
const factor = Math.pow(10, decimal);
nextValue = (Math.round(temp * factor) + Math.round(amount * factor)) / factor;
Comment on lines +68 to +78
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Remove dead code and fix decimal handling.

Lines 73-76 compute nextValue but are immediately overwritten by lines 77-78, making the first calculation dead code. Additionally, the approach is inconsistent:

  • Lines 73-76 scale, add, then descale without rounding intermediate results
  • Lines 77-78 round each operand before adding (better for precision)

Apply this diff to remove dead code and clarify the logic:

- const amountDecimals = (amount.toString().split('.')[1] || '').length; - const tempDecimals = (temp.toString().split('.')[1] || '').length; - const decimal = amountDecimals > tempDecimals ? amountDecimals : tempDecimals; - - // multiply temp and amount with same decimal count to remove decimal places - nextValue = (temp * Math.pow(10, decimal)) + (amount * Math.pow(10, decimal)); - - // divide nextValue by same decimal count - nextValue = nextValue / Math.pow(10, decimal); + // Find decimal places in amount and temp + const amountDecimals = (amount.toString().split('.')[1] || '').length; + const tempDecimals = (temp.toString().split('.')[1] || '').length; + const decimal = Math.max(amountDecimals, tempDecimals); +  + // Scale to integers, compute step, then scale back const factor = Math.pow(10, decimal); nextValue = (Math.round(temp * factor) + Math.round(amount * factor)) / factor;
📝 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 amountDecimals = (amount.toString().split('.')[1] || '').length;
const tempDecimals = (temp.toString().split('.')[1] || '').length;
const decimal = amountDecimals > tempDecimals ? amountDecimals : tempDecimals;
// multiply temp and amount with same decimal count to remove decimal places
nextValue = (temp * Math.pow(10, decimal)) + (amount * Math.pow(10, decimal));
// divide nextValue by same decimal count
nextValue = nextValue / Math.pow(10, decimal);
const factor = Math.pow(10, decimal);
nextValue = (Math.round(temp * factor) + Math.round(amount * factor)) / factor;
// Find decimal places in amount and temp
const amountDecimals = (amount.toString().split('.')[1] || '').length;
const tempDecimals = (temp.toString().split('.')[1] || '').length;
const decimal = Math.max(amountDecimals, tempDecimals);
// Scale to integers, compute step, then scale back
const factor = Math.pow(10, decimal);
nextValue = (Math.round(temp * factor) + Math.round(amount * factor)) / factor;
🤖 Prompt for AI Agents
In src/components/ui/NumberField/fragments/NumberFieldRoot.tsx around lines 68 to 78, remove the dead scaling/descaling block (the computation that assigns nextValue using temp * Math.pow(10, decimal) and then divides it back) and keep a single, consistent calculation that uses the computed decimal (max of amount and temp decimals), a factor = Math.pow(10, decimal), and rounds each operand before adding: nextValue should be computed as (Math.round(temp * factor) + Math.round(amount * factor)) / factor; delete the earlier overwritten assignments so only the rounded approach remains. 

// Clamp to min/max
if (max !== undefined && nextValue > max) return max;
if (min !== undefined && nextValue < min) return min;

return nextValue;
});
};

const handleStep = ({ type, direction } : {type: 'small'| 'large', direction: 'increment' | 'decrement' }) => {
const handleStep = ({ type, direction } : {type: 'small'| 'normal' | 'large', direction: 'increment' | 'decrement' }) => {
let amount = 0;

switch (type) {
case 'small':
case 'normal':
if (!step) return;
amount = step;
break;
case 'large':
if (!largeStep) return;
amount = largeStep;
break;
case 'small':
if (!smallStep) return;
amount = smallStep;
break;
}

if (direction === 'decrement') {
amount *= -1;
amount = -amount;
console.log(amount);
}
Comment on lines 106 to 109
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove debug console.log before merging.

Line 108 contains a debug console.log(amount) statement that should be removed.

Apply this diff:

 if (direction === 'decrement') { amount = -amount; - console.log(amount); }
📝 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 (direction === 'decrement') {
amount *= -1;
amount = -amount;
console.log(amount);
}
if (direction === 'decrement') {
amount = -amount;
}
🤖 Prompt for AI Agents
In src/components/ui/NumberField/fragments/NumberFieldRoot.tsx around lines 106 to 109, remove the debug console.log(amount) call: delete the console.log(...) line so no debug logging remains in the decrement branch and ensure the surrounding logic remains unchanged. 

applyStep(amount);
Expand All @@ -99,6 +120,7 @@ const NumberFieldRoot = forwardRef<NumberFieldRootElement, NumberFieldRootProps>
disabled,
readOnly,
required,
locale,
rootClass
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ 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.Root value={value} onValueChange={setValue} defaultValue={3} step={1} largeStep={5}>
<NumberField.Decrement>-</NumberField.Decrement>
<NumberField.Input />
<NumberField.Increment>+</NumberField.Increment>
Expand Down
Loading