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

feat(VNumberInput): support control holding down #20987

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

yuwu9145
Copy link
Member

Description

Markup:

<template>
  <v-container>
    <v-row>
      <v-col cols="12" md="4" sm="4">
        <h5>Default</h5>

        <v-number-input 
          v-if="showNumberInput"
          control-variant="default"
        ></v-number-input>
      </v-col>

      <v-btn
        @click="remove"
      >Remove Number Input</v-btn>
    </v-row>
  </v-container>
</template>
<script setup>
import { ref } from 'vue'

  const showNumberInput = ref(true)
  function remove (event) {
    showNumberInput.value = false
  }
</script>

@yuwu9145 yuwu9145 requested a review from a team February 15, 2025 07:29
Copy link
Contributor

@J-Sek J-Sek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only see potential for improvement around variable naming and maybe avoiding boolean as direction argument. Other than this, it delivers the functionality and I was not able to break it or come up with edge case scenario.

let hldDwnTimeout = -1

function controlHoldingDownWatcher (time: number) {
hldDwnReq = requestAnimationFrame(controlHoldingDownWatcher)
Copy link
Contributor

@J-Sek J-Sek Feb 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • hldDwnReq » holdDownRaf or holdActionRaf

I actually don't care much about the suffix. It can be *Raf, *Frame, *Req. My issue is with saving 2 vowels in hldDwn for no obvious gain. IMO it drags attention contributing to cognitive load.

@KaelWD KaelWD force-pushed the feat-number-input-holding-control branch from af78942 to e2ad8a7 Compare February 17, 2025 05:41
@KaelWD KaelWD changed the base branch from dev to master February 17, 2025 05:41
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.

3 participants