Skip to content

Commit 3c9bedf

Browse files
xerox0213xerox0213
authored
fix(NumberField): snap off-grid value to the nearest grid line when stepping (#2759)
* fix(NumberField): snap off-grid value to nearest grid line when stepping * fix(NumberField): align disabled state with aligned stepping value isIncreaseDisabled/isDecreaseDisabled used the raw value ± step instead of the aligned next value, so a valid off-grid move (e.g. 8 -> 9 with max=10, step=3) was wrongly disabled. Both now reuse getNextValue, the same calc as the handler. * fix(NumberField): clamp the empty/NaN fallback value to the range When the input is empty or unparseable, the fallback wrote `min ?? 0` directly, which violates the range when only a negative max is set. It now goes through clampInputValue so the same min/max contract applies.
1 parent 5ca8f56 commit 3c9bedf

2 files changed

Lines changed: 112 additions & 23 deletions

File tree

‎packages/core/src/NumberField/NumberField.test.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,68 @@ describe('numberField', () => {
300300
await fireEvent.keyDown(input, { key: kbd.ARROW_UP }) // 21 (max not snapped to step)
301301
expect(input.value).toBe('21')
302302
})
303+
304+
it('should snap an off-grid value to the next grid line when incrementing', async () => {
305+
// Seed the off-grid value via defaultValue: typing it would snap on commit.
306+
const { input, increment } = setup({ step: 1, stepSnapping: true, defaultValue: 18.98 })
307+
308+
await userEvent.click(increment) // snap up to the nearest grid line, not 18.98 + 1 -> 20
309+
expect(input.value).toBe('19')
310+
})
311+
312+
it('should snap an off-grid value to the previous grid line when decrementing', async () => {
313+
const { input, decrement } = setup({ step: 1, stepSnapping: true, defaultValue: 18.11 })
314+
315+
await userEvent.click(decrement) // snap down to the nearest grid line, not 18.11 - 1 -> 17
316+
expect(input.value).toBe('18')
317+
})
318+
319+
it('should add a full step when the value is already on the grid', async () => {
320+
const { input, increment, decrement } = setup({ step: 1, stepSnapping: true, defaultValue: 5 })
321+
322+
await userEvent.click(increment)
323+
expect(input.value).toBe('6')
324+
await userEvent.click(decrement)
325+
expect(input.value).toBe('5')
326+
})
327+
})
328+
329+
describe('given step alignment near min/max boundaries', () => {
330+
it('should keep increment enabled when an off-grid value can still align below max', async () => {
331+
const { input, increment } = setup({ max: 10, step: 3, stepSnapping: true, defaultValue: 8 })
332+
333+
expect(increment).not.toHaveAttribute('disabled')
334+
await userEvent.click(increment) // aligns to 9, not 8 + 3
335+
expect(input.value).toBe('9')
336+
})
337+
338+
it('should keep decrement enabled when an off-grid value can still align above min', async () => {
339+
const { input, decrement } = setup({ min: 2, step: 3, stepSnapping: true, defaultValue: 4 })
340+
341+
expect(decrement).not.toHaveAttribute('disabled')
342+
await userEvent.click(decrement) // aligns to 2, not 4 - 3
343+
expect(input.value).toBe('2')
344+
})
345+
346+
it('should disable increment once the next aligned value cannot exceed max', async () => {
347+
const { increment } = setup({ max: 10, step: 3, stepSnapping: true, defaultValue: 9 })
348+
349+
expect(increment).toHaveAttribute('disabled')
350+
})
351+
352+
it('should disable decrement once the next aligned value cannot go below min', async () => {
353+
const { decrement } = setup({ min: 2, step: 3, stepSnapping: true, defaultValue: 2 })
354+
355+
expect(decrement).toHaveAttribute('disabled')
356+
})
357+
358+
it('should clamp the empty/NaN fallback to the range', async () => {
359+
const { input, increment } = setup({ max: -5 })
360+
361+
// Empty input: the bare fallback would be 0, which is above max; it must be clamped.
362+
await userEvent.click(increment)
363+
expect(input.value).toBe('-5')
364+
})
303365
})
304366

305367
describe('given setting the input value manually', async () => {

‎packages/core/src/NumberField/NumberFieldRoot.vue‎

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -94,22 +94,50 @@ const locale = useLocale(propLocale)
9494
const isFormControl = useFormControl(currentElement)
9595
const inputEl = ref<HTMLInputElement>()
9696
97-
const isDecreaseDisabled = computed(() => (
98-
!isNullish(modelValue.value) && (
99-
clampInputValue(modelValue.value) === min.value
100-
|| (min.value && !isNaN(modelValue.value)
101-
)
102-
? (handleDecimalOperation('-', modelValue.value, step.value) < min.value)
103-
: false)),
104-
)
105-
const isIncreaseDisabled = computed(() => (
106-
!isNullish(modelValue.value) && (
107-
clampInputValue(modelValue.value) === max.value
108-
|| (max.value && !isNaN(modelValue.value)
109-
)
110-
? (handleDecimalOperation('+', modelValue.value, step.value) > max.value)
111-
: false)),
112-
)
97+
const isDecreaseDisabled = computed(() => {
98+
if (isNullish(modelValue.value) || isNaN(modelValue.value))
99+
return false
100+
// Disabled when a decrement can't produce a smaller in-range value.
101+
return getNextValue('decrease', modelValue.value) >= modelValue.value
102+
})
103+
const isIncreaseDisabled = computed(() => {
104+
if (isNullish(modelValue.value) || isNaN(modelValue.value))
105+
return false
106+
// Disabled when an increment can't produce a larger in-range value.
107+
return getNextValue('increase', modelValue.value) <= modelValue.value
108+
})
109+
110+
// Compute the clamped value a single increment/decrement (or multi-step key) would land on.
111+
// When snapping is enabled and `from` is off the step grid, a tick snaps to the nearest grid
112+
// line in the requested direction (HTML stepUp/stepDown semantics), instead of adding a whole
113+
// step and rounding to nearest — which overshoots (e.g. 18.98 + step 1 -> 19.98 -> 20 not 19).
114+
function getNextValue(type: 'increase' | 'decrease', from: number, multiplier = 1): number {
115+
const stepValue = step.value ?? 1
116+
const operator = type === 'increase' ? '+' : '-'
117+
let nextValue: number
118+
119+
if (stepSnapping.value && !isNaN(stepValue)) {
120+
const snapped = snapValueToStep(from, min.value, max.value, stepValue)
121+
if (snapped === from) {
122+
nextValue = handleDecimalOperation(operator, from, stepValue * multiplier)
123+
}
124+
else {
125+
// Align to the grid line in the requested direction first…
126+
const aligned = type === 'increase'
127+
? (snapped > from ? snapped : handleDecimalOperation('+', snapped, stepValue))
128+
: (snapped < from ? snapped : handleDecimalOperation('-', snapped, stepValue))
129+
// …then apply any remaining steps for multi-step keys (PageUp/PageDown).
130+
nextValue = multiplier > 1
131+
? handleDecimalOperation(operator, aligned, stepValue * (multiplier - 1))
132+
: aligned
133+
}
134+
}
135+
else {
136+
nextValue = handleDecimalOperation(operator, from, stepValue * multiplier)
137+
}
138+
139+
return clampInputValue(nextValue)
140+
}
113141
114142
function handleChangingValue(type: 'increase' | 'decrease', multiplier = 1) {
115143
if (props.focusOnChange) {
@@ -119,14 +147,13 @@ function handleChangingValue(type: 'increase' | 'decrease', multiplier = 1) {
119147
return
120148
const currentInputValue = numberParser.parse(inputEl.value?.value ?? '')
121149
if (isNaN(currentInputValue)) {
122-
modelValue.value = min.value ?? 0
123-
}
124-
else {
125-
if (type === 'increase')
126-
modelValue.value = clampInputValue(currentInputValue + ((step.value ?? 1) * multiplier))
127-
else
128-
modelValue.value = clampInputValue(currentInputValue - ((step.value ?? 1) * multiplier))
150+
// Route the fallback through clampInputValue so the min/max contract still holds
151+
// (e.g. a negative max would otherwise be violated by the bare 0 fallback).
152+
modelValue.value = clampInputValue(min.value ?? 0)
153+
return
129154
}
155+
156+
modelValue.value = getNextValue(type, currentInputValue, multiplier)
130157
}
131158
132159
function handleIncrease(multiplier = 1) {

0 commit comments

Comments
 (0)