Skip to content

Commit

Permalink
fix(VNumberInput): prevent NaN & handle js number quirks (#20211)
Browse files Browse the repository at this point in the history 
fixes #19798
fixes #20171

Co-authored-by: Kael <kaelwd@gmail.com>
  • Loading branch information
@yuwu9145 @KaelWD
yuwu9145 and KaelWD committed Jul 30, 2024
1 parent 0a31bf8 commit 3a31086
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 42 deletions.
10 changes: 10 additions & 0 deletions packages/docs/src/pages/en/components/number-inputs.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ Here we display a list of settings that could be applied within an application.

<ApiInline hide-links />

## Caveats

::: warning
**v-number-input** is designed for simple numeric input usage. It has limitations with very long integers and highly precise decimal arithmetic due to JavaScript number precision issues:

- For integers, **v-model** is restricted within [Number.MIN_SAFE_INTEGER](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MIN_SAFE_INTEGER) and [Number.MAX_SAFE_INTEGER](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER) to ensure precision is not lost.

- To cope with JavaScript floating-point issues (e.g. 0.1 + 0.2 === 0.30000000000000004), Vuetify's internal logic uses **toFixed()** with the maximum number of decimal places between v-model and step. If accurate arbitrary-precision decimal arithmetic is required, consider working with strings using [decimal.js](https://github.com/MikeMcl/decimal.js) and [v-text-field](/components/text-fields) instead.
:::

## Guide

The `v-number-input` component is built upon the `v-field` and `v-input` components. It is used as a replacement for `<input type="number">`, accepting numeric values from the user.
Expand Down
113 changes: 72 additions & 41 deletions packages/vuetify/src/labs/VNumberInput/VNumberInput.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { useForm } from '@/composables/form'
import { useProxiedModel } from '@/composables/proxiedModel'

// Utilities
import { computed, watchEffect } from 'vue'
import { computed, nextTick, onMounted, ref } from 'vue'
import { clamp, genericComponent, getDecimals, omit, propsFactory, useRender } from '@/util'

// Types
Expand All @@ -37,20 +37,24 @@ const makeVNumberInputProps = propsFactory({
},
inset: Boolean,
hideInput: Boolean,
modelValue: {
type: Number as PropType<Number | null>,
default: null,
},
min: {
type: Number,
default: -Infinity,
default: Number.MIN_SAFE_INTEGER,
},
max: {
type: Number,
default: Infinity,
default: Number.MAX_SAFE_INTEGER,
},
step: {
type: Number,
default: 1,
},

...omit(makeVTextFieldProps(), ['appendInnerIcon', 'prependInnerIcon']),
...omit(makeVTextFieldProps({}), ['appendInnerIcon', 'modelValue', 'prependInnerIcon']),
}, 'VNumberInput')

export const VNumberInput = genericComponent<VNumberInputSlots>()({
Expand All @@ -64,11 +68,20 @@ export const VNumberInput = genericComponent<VNumberInputSlots>()({
'update:modelValue': (val: number) => true,
},

setup (props, { attrs, emit, slots }) {
const model = useProxiedModel(props, 'modelValue')
setup (props, { slots }) {
const _model = useProxiedModel(props, 'modelValue')

const model = computed({
get: () => _model.value,
set (val) {
if (typeof val !== 'string') _model.value = val
},
})

const vTextFieldRef = ref<VTextField | undefined>()

const stepDecimals = computed(() => getDecimals(props.step))
const modelDecimals = computed(() => model.value != null ? getDecimals(model.value) : 0)
const modelDecimals = computed(() => typeof model.value === 'number' ? getDecimals(model.value) : 0)

const form = useForm()
const controlsDisabled = computed(() => (
Expand All @@ -77,20 +90,11 @@ export const VNumberInput = genericComponent<VNumberInputSlots>()({

const canIncrease = computed(() => {
if (controlsDisabled.value) return false
if (model.value == null) return true
return model.value + props.step <= props.max
return (model.value ?? 0) as number + props.step <= props.max
})
const canDecrease = computed(() => {
if (controlsDisabled.value) return false
if (model.value == null) return true
return model.value - props.step >= props.min
})

watchEffect(() => {
if (controlsDisabled.value) return
if (model.value != null && (model.value < props.min || model.value > props.max)) {
model.value = clamp(model.value, props.min, props.max)
}
return (model.value ?? 0) as number - props.step >= props.min
})

const controlVariant = computed(() => {
Expand All @@ -106,18 +110,24 @@ export const VNumberInput = genericComponent<VNumberInputSlots>()({

const decrementSlotProps = computed(() => ({ click: onClickDown }))

onMounted(() => {
if (!props.readonly && !props.disabled) {
clampModel()
}
})

function toggleUpDown (increment = true) {
if (controlsDisabled.value) return
if (model.value == null) {
model.value = 0
model.value = clamp(0, props.min, props.max)
return
}

const decimals = Math.max(modelDecimals.value, stepDecimals.value)
if (increment) {
if (canIncrease.value) model.value = +(((model.value + props.step).toFixed(decimals)))
if (canIncrease.value) model.value = +((((model.value as number) + props.step).toFixed(decimals)))
} else {
if (canDecrease.value) model.value = +(((model.value - props.step).toFixed(decimals)))
if (canDecrease.value) model.value = +((((model.value as number) - props.step).toFixed(decimals)))
}
}

Expand All @@ -131,37 +141,56 @@ export const VNumberInput = genericComponent<VNumberInputSlots>()({
toggleUpDown(false)
}

function onKeydown (e: KeyboardEvent) {
function onBeforeinput (e: InputEvent) {
if (!e.data) return
const existingTxt = (e.target as HTMLInputElement)?.value
const selectionStart = (e.target as HTMLInputElement)?.selectionStart
const selectionEnd = (e.target as HTMLInputElement)?.selectionEnd
const potentialNewInputVal =
existingTxt
? existingTxt.slice(0, selectionStart as number | undefined) + e.data + existingTxt.slice(selectionEnd as number | undefined)
: e.data
// Only numbers, "-", "." are allowed
// AND "-", "." are allowed only once
// AND "-" is only allowed at the start
if (!/^-?(\d+(\.\d*)?|(\.\d+)|\d*|\.)$/.test(potentialNewInputVal)) {
e.preventDefault()
}
}

async function onKeydown (e: KeyboardEvent) {
if (
['Enter', 'ArrowLeft', 'ArrowRight', 'Backspace', 'Delete', 'Tab'].includes(e.key) ||
e.ctrlKey
) return

if (['ArrowDown'].includes(e.key)) {
e.preventDefault()
toggleUpDown(false)
return
}
if (['ArrowUp'].includes(e.key)) {
e.preventDefault()
toggleUpDown()
return
}

// Only numbers, +, - & . are allowed
if (!/^[0-9\-+.]+$/.test(e.key)) {
if (['ArrowDown', 'ArrowUp'].includes(e.key)) {
e.preventDefault()
clampModel()
// _model is controlled, so need to wait until props['modelValue'] is updated
await nextTick()
if (e.key === 'ArrowDown') {
toggleUpDown(false)
} else {
toggleUpDown()
}
}
}

function onModelUpdate (v: string) {
model.value = v ? +(v) : undefined
}

function onControlMousedown (e: MouseEvent) {
e.stopPropagation()
}

function clampModel () {
if (!vTextFieldRef.value) return
const inputText = vTextFieldRef.value.value
if (inputText && !isNaN(+inputText)) {
model.value = clamp(+(inputText), props.min, props.max)
} else {
model.value = null
}
}

useRender(() => {
const { modelValue: _, ...textFieldProps } = VTextField.filterProps(props)

Expand Down Expand Up @@ -277,8 +306,10 @@ export const VNumberInput = genericComponent<VNumberInputSlots>()({

return (
<VTextField
modelValue={ model.value }
onUpdate:modelValue={ onModelUpdate }
ref={ vTextFieldRef }
v-model={ model.value }
onBeforeinput={ onBeforeinput }
onChange={ clampModel }
onKeydown={ onKeydown }
class={[
'v-number-input',
Expand Down
36 changes: 35 additions & 1 deletion packages/vuetify/src/labs/VNumberInput/__tests__/VNumberInput.spec.cy.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,40 @@ import { VForm } from '@/components/VForm'
import { ref } from 'vue'

describe('VNumberInput', () => {
it('should prevent NaN from arbitrary input', () => {
const scenarios = [
{ typing: '---', expected: '-' }, // "-" is only allowed once
{ typing: '1-', expected: '1' }, // "-" is only at the start
{ typing: '.', expected: '.' }, // "." is allowed at the start
{ typing: '..', expected: '.' }, // "." is only allowed once
{ typing: '1...0', expected: '1.0' }, // "." is only allowed once
{ typing: '123.45.67', expected: '123.4567' }, // "." is only allowed once
{ typing: 'ab-c8+.iop9', expected: '-8.9' }, // Only numbers, "-", "." are allowed to type in
]
scenarios.forEach(({ typing, expected }) => {
cy.mount(() => <VNumberInput />)
.get('.v-number-input input').focus().realType(typing)
.get('.v-number-input input').should('have.value', expected)
})
})

it('should reset v-model to null when click:clear is triggered', () => {
const model = ref(5)

cy.mount(() => (
<>
<VNumberInput
clearable
v-model={ model.value }
readonly
/>
</>
))
.get('.v-field__clearable .v-icon--clickable').click()
.then(() => {
expect(model.value).equal(null)
})
})
describe('readonly', () => {
it('should prevent mutation when readonly applied', () => {
const value = ref(1)
Expand Down Expand Up @@ -98,7 +132,7 @@ describe('VNumberInput', () => {
class="disabled-input-2"
v-model={ value4.value }
min={ 0 }
max={ 10 }
max={ 10 }
disabled
/>
</>
Expand Down

0 comments on commit 3a31086

Please sign in to comment.