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

[yuwu9145]

fixes #19798
fixes #20171

Description

Markup:

<template>
  <v-app>
    <v-container>
      <h1>{{ typed }}</h1>
      <v-number-input
        label=""
        clearable 
        v-model="typed"
        :hideInput="false"
        :inset="false"
        :min="5"
        :max="125"
        :step="0.32"
        validate-on="lazy blur"
        :messages="`What I typed: ${typed}`"
      ></v-number-input>
    </v-container>
  </v-app>
</template>

<script setup>
  import { ref } from 'vue'
  // function onKeydown(event) {
  //   if (event.key === 'Backspace') {
  //     typed.value = typed.value.slice(0, -1)
  //   } else {
  //     typed.value += event.key
  //   }
  // }
  const typed = ref(-5)
</script>

[J-Sek]

I can see number of test cases failing. In particular min/max enforcement. Did you mean to move it inside onBeforeinput? If so, the tests need to be updated as they check range enforcement even without user interaction.

There is also a bit of inconsistency in the emitted value type. Up/Down emit number while typing emits string. I am not sure which way is intended for this component.

• string model makes it somehow consistent with <input type='number' /> when interacting through vanilla JS
• once native <input type='number' /> is used with Vue v-model the value is of type number and that's what developers may expect when binding to v-number-input

The trajectory of the changes suggests we cannot go back to emitting number.

• users can use v-model.number="..." to transform values
  • It is not ideal though, as I get - as string out of it despite the modifier
• some onBlur action to clear leading and trailing zeros could be welcomed

Issues:

• - gets removed after typing 0 right after (when I use v-model.number=...)
• leading and trailing 0 get removed if model changes (when I use v-model.number=...) (maybe irrelevant, but caught my eye regardless, because it interrupts typing)
• 0 is required to type . (maybe irrelevant)
• user cannot paste any value that has even the smallest divergence from underlying regex, so numbers copied with locale formatting, or spaces surrounding the value - may lead to "this input is broken" from user's perspective

[J-Sek]

I am glad to see some progress :) We don't need the .number modifier, but it is a bit of a trade nonetheless.

Couple of weird behaviors I think are not exactly obvious, but may be important:

• (observation) component does not emit while typing - only on blur or when using up/down arrows
• (bug) null values get replaced with 0 - clearable does not work
• (bug) when I change value and use up/down arrows right after, the step is modifying last emitted value (e.g. 55{up} » 1)

[J-Sek]

<v-number-input v-model="value" clearable />
<code>value: {{ value ?? "(empty)" }} | {{ typeof(value) }}</code>
---
const value = ref<number | null>(null)

Clearable works, but I still get initial 0 instead of empty field.

There is one new bug

• typing emits string and if I use up arrow (keyboard), the action has no effect, while down decrements normally

While can see it is very easy to patch with a single + to cast model before adding step, I am curious if emits with string type should be allowed... Developers might be tempted to use v-model.number= and will get problems mentioned earlier (especially problems when typing small negative float values).

[yuwu9145]

typing emits string

Fixed, it should only emit number or null

[J-Sek]

All VTextField's v-model={ model.value } emits are strings, so the setter for computed model seems rather pointless. The intent could be more clear with going back to modelValue={ model.value }, because only onChange handler and toggleUpDown actually emit numbers.

Regardless, I think internals will change yet again, but I don't want to stress about uncovered use case that bounces between broken and working states. Could you include following test scenarios? 😃

{ text: '123{uparrow}', expected: '124' },
{ text: '521{downarrow}', expected: '520' },

[yuwu9145]

All VTextField's v-model={ model.value } emits are strings, so the setter for computed model seems rather pointless. The intent could be more clear with going back to modelValue={ model.value }, because only onChange handler and toggleUpDown actually emit numbers.

Regardless, I think internals will change yet again, but I don't want to stress about uncovered use case that bounces between broken and working states. Could you include following test scenarios? 😃

{ text: '123{uparrow}', expected: '124' },
{ text: '521{downarrow}', expected: '520' },

The reason of double way binding is: in the case of v-model=5, min=5 & max=10 and they type "-1", I want input to be enforced back to 5. Single way binding lose that enforcement because external v-model isn't mutated

Could be a nice feature in useProxiedModel

https://play.vuetifyjs.com/#eNpdUEFugzAQ/MrIlyRSKOIakaj9Q3sqPbiwBKvGtsyaNkL8vTYkUZTLyjs7np3Zz0kMvs7fnHsZA4mDKJl6pyXTqTJAOWbSueW5NLU1LJUhf4Ui2BWn94sjSHPhTpkzLHfkwZ00KPZQjF+lNVoZy7esf8AWRZnHb3eJMWP646xVpBscetuQzkapAx0r0Q/nSuA1uCZ6ep6l7mOZRE5+s5k/+0zIGqPMH+LFdqi9coyBOKwpVe+sZ0zw1GJG622PTbzMJtGBqDswoiccE2Nb7Fa8DaZmZQ0eLG13mOa0cl0S5cW8N0HrpXz9A1y2f1E=

[J-Sek]

The reason of double way binding is: in the case of v-model=5, min=5 & max=10 and they type "-1", I want input to be enforced back to 5.

And you achieve this with onChange handler. Check the example below. By removing @change='toNumber' you will get back to the same behavior as :model-value='...', proving it is not v-model that helps you run clampModel, but onChange handler.

https://play.vuetifyjs.com/#eNpdkMFy4jAQRH+l4wumAri4spjd/YDltLeQg2LGoIosqeSxkhTlf8/ICgnJRR71jHqe++FS9KGp/nq/igMVm2LL1HmjmHYHC2zjUnk/ldOlcZaVthQ+JBHP693/N09Q9o3P2p7g+EwBfFYW6wU040Ubg1bJ8aSaZ7DDelvJs0+LuGR65WWryRwRl507kqkPRdef/qXyUOBPI3YnEpHdfuieKIhYXbmqn2BJydzb6uZ/5No3QXtGTzx4GPFMlv2hmJ7pzrvAuCBQu0DjOj8wHTGiDa7DTAKaJRNIy/YM4UOdZsv1/Js6UUvr6lBeMuiJeINyjnqXplZRmYEWuSVAZZzjY1BQWpQssboWEXd1jVnPQdKd3czgy0WWxas85mKcnEchS992sA1rZyX9nF9JkT+9MnkUF1FXrIKQZuPcv91zp/u92pf3Qvsb9xEb2MGYX9OylHdOWPIsxkXqTMfjO2KHxeo=

[yuwu9145]

@J-Sek feel free to make a tweaking PR to fix-19798-origin, I will check it out tomorrow in my time

[yuwu9145]

If no more comments, ready to merge in

[J-Sek]

Edit: I checked out the wrong branch, sorry min and max are not enforced in some scenarios because model can become out-of-sync with VTextField's value

There is still a problem of using up/down while typing. 510{uparrow} does not become 511. I hope it can be addressed later.