Skip to content
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

fix(VNumberInput): prevent NaN & properly handle js number quirks #20211

Merged
merged 14 commits into from
Jul 30, 2024
Merged
10 changes: 10 additions & 0 deletions packages/docs/src/pages/en/components/number-inputs.md
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
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
yuwu9145 marked this conversation as resolved.
Show resolved Hide resolved
},
})

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
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