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 19798 tweaks #20230

Closed
wants to merge 2 commits into from
Closed

Conversation

J-Sek
Copy link
Contributor

@J-Sek J-Sek commented Jul 27, 2024

PR consists of 2 independent parts:

  • refactoring to make it clear none of the update:model-value emits from VTextField are accepted (because all of them provide type string, they were already filtered out by the _model wrapper)
  • test cases (mostly failing) to guide further implementation tweaks
    • scenarios for mixed usage of typing (adding/removing digits) and incrementing value (up/down arrows)
    • additional scenarios to ensure consistent state after blur

There is one more advanced scenario that works around auto-cleanup on blur (initial typing for -0.21 is actually optional)

  1. focus input with existing value and change text without changing effective number
    • e.g. -.21, -0000.21, -0.2100
  2. after blur the text stays the same
    • it might not be a critical bug, but it is inconsistent with behavior when we type a new value, developers might be bothered by their QA team about this

Supports #20211

@yuwu9145
Copy link
Member

As it has been mentioned before, this change breaks external v-model value enforcement

In this example, expected behaviour should be input value fall back to min 5

number-input-v-model

<template>
  <v-app>
    <v-container>
      <h1>{{ typeof typed }}</h1>
      <h1>{{ typed  }}</h1>
      <v-number-input
        label=""
        clearable 
        v-model="typed"
        :hideInput="false"
        :inset="false"
        :min="5"
        :max="125"
        :step="0.32"
      ></v-number-input>
    </v-container>
  </v-app>
</template>

<script setup>
  import { ref } from 'vue'
  const typed = ref(-5)
</script>

@yuwu9145 yuwu9145 closed this Jul 28, 2024
@J-Sek
Copy link
Contributor Author

J-Sek commented Jul 28, 2024

@yuwu9145 It does, you are right... weird I missed it. My focus was on the behavior of unrestricted field.

  • shouldn't we have some test cases for it?
  • wouldn't it be more clear to put clam in onBlur to enforce it rather than relying on some magic side-effect of @update:model-value?

Edit: It seems that missing @update:model-value makes it possible for model to become out-of-sync with VTextField, hence onBlur fixes nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants