-  
 -   Notifications  
You must be signed in to change notification settings  - Fork 53
 
Add snapOnStep, smallStep support for NumberField #1601
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
b93312c c2ce6ca f307d9f a3d39ef d5789ab 9830afa 8fd039d a4ff45f File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@radui/ui": patch | ||
| --- | ||
|   |  ||
| AddedsnapOnStep, smallStep support for NumberField | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|      |  @@ -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' })} | ||
|   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "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 |  ||
| className={clsx(`${rootClass}-decrement`, className)} | ||
| disabled={disabled || readOnly} | ||
| type="button" | ||
|      |  ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|      |  @@ -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' })} | ||
|   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |  ||
| className={clsx(`${rootClass}-increment`, className)} | ||
| disabled={disabled || readOnly} | ||
| type="button" | ||
|      |  ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|      |  @@ -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; | ||||||||||||||||||||||||||||||||||||||||
|      |  @@ -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    There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove dead code and fix decimal handling. Lines 73-76 compute  
 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
  Suggested change   
 🤖 Prompt for AI Agents |  ||||||||||||||||||||||||||||||||||||||||
|   |  ||||||||||||||||||||||||||||||||||||||||
| // 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    There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove debug console.log before merging. Line 108 contains a debug  Apply this diff:  if (direction === 'decrement') { amount = -amount; - console.log(amount); }📝 Committable suggestion
  Suggested change   
 🤖 Prompt for AI Agents |  ||||||||||||||||||||||||||||||||||||||||
|   |  ||||||||||||||||||||||||||||||||||||||||
| applyStep(amount); | ||||||||||||||||||||||||||||||||||||||||
|      |  @@ -99,6 +120,7 @@ const NumberFieldRoot = forwardRef<NumberFieldRootElement, NumberFieldRootProps> | |||||||||||||||||||||||||||||||||||||||
| disabled, | ||||||||||||||||||||||||||||||||||||||||
| readOnly, | ||||||||||||||||||||||||||||||||||||||||
| required, | ||||||||||||||||||||||||||||||||||||||||
| locale, | ||||||||||||||||||||||||||||||||||||||||
| rootClass | ||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||
|   |  ||||||||||||||||||||||||||||||||||||||||
|      |  ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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