Skip to content

Conversation

JserWang
Copy link

@JserWang JserWang commented Aug 8, 2018

Please make sure these boxes are checked before submitting your PR, thank you!

  • Make sure you follow Element's contributing guide (中文 | English | Español).
  • Make sure you are merging your commits to dev branch.
  • Add some descriptions and refer relative issues for you PR.
<template>
  <div class="hello">
    <el-input-number
      v-model="number"
      @change="handleChange"
      :setp="5"
    ></el-input-number>
  </div>
</template>

<script>
export default {
  data() {
    return {
      number: 5,
    };
  },
  methods: {
    handleChange(newVal) {
      const step = 5;
      if (newVal % step !== 0) {
        this.number = (Math.floor(newVal / step) * step) + step;
      }
    },
  },
};
</script>

模拟一个使用场景,用户期望文本框显示值恒为5的倍数时,虽然在源码中inputchange写的早,但是在实际场景中,changethis.numbersetter会早于input的触发,导致用户在change中设置的值,会被input触发的newVal所覆盖掉。

如果不修改此BUG,用户需要手动在handelChange中写setTimeoutnextTick,这明显不是一个比较好的做法。

@element-bot
Copy link
Member

element-bot commented Aug 8, 2018

Deploy preview for element ready!

Built with commit 4842b22

https://deploy-preview-12291--element.netlify.com

@wacky6
Copy link
Contributor

wacky6 commented Aug 8, 2018

这种情况直接 @input="handleChange" 不就行了吗。

@JserWang
Copy link
Author

JserWang commented Aug 8, 2018

默认值为5,当在页面中将5删掉,在失去去焦点的情况下,将值变为1234中任意一个值时,此时在vue内部会发生什么情况?

由于使用了v-modelvue通过transformModel将回调函数拼接。

如果直接使用@inputinvoker会循环遍历上述拼接的回调,依次执行:
(1)number会先变为你输入的数字,触发setter调用dep.notify()
(2)number变为handleChange处理后的数字,同样触发setter调用dep.notify()

但是需要注意的一点是在watcher中存储的value5,在后续watcher.run()时,updateChildComponent方法中重新为props赋值时,value5的数,再次被赋值给el-input-number中,所以在此不会触发其中的watch: { value },从而影响了后续逻辑,导致了this.currentInputValuev-model中的值不符。

@@ -223,7 +223,9 @@
return;
}
this.$emit('input', newVal);
this.$emit('change', newVal, oldVal);
setTimeout(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

这里可以用 this.$nextTick 替代

@JserWang JserWang force-pushed the input-number-bugfix branch from 1f20774 to 4842b22 Compare August 9, 2018 08:12
@JserWang
Copy link
Author

JserWang commented Aug 9, 2018

@jikkai 已更改为this.$nextTick的方式,重新提交。

@wacky6
Copy link
Contributor

wacky6 commented Aug 9, 2018

看了一下代码。其实还是input和input-number内部实现的问题。没有严格遵守单向数据流,内部另外维护了显示值。彻底修要和input一起重构。

@jikkai
Copy link
Contributor

jikkai commented Aug 9, 2018

@wacky6 我其实比较倾向这个 PR 的解决方案是个 workaround。现在这个版本里做这个修改的意义也不是特别大,最好是能在下一个主版本中彻底重构。或者 @JserWang @ziyoung 有什么其他建议吗?

@ziyoung
Copy link
Contributor

ziyoung commented Aug 9, 2018

没有严格遵守单向数据流,内部另外维护了显示值

这句话,我不是特别理解。😂InputNumber 的实现大致如下:

<el-input :value="currentInputValue" @change="handleInputChange"></el-input>
  • 在 handleInputChange 回调函数中,改变 currentInputValue,然后通过触发事件传递到外部
  • watch value 的变化,把 currentInputValue 跟 value 做一次同步

我感觉内部维护一个显示值是不可少的,如果不需要这个值,那要怎么去做?

@wacky6
Copy link
Contributor

wacky6 commented Aug 9, 2018

@ziyoung

我倾向于直接把组件的 value 显示出来。currentInputValue 其实是想获得用户输入的值,但应该显示的值是根据 value 和 currentInputValue 算出来的。改完以后有点像 date-picker 里我以前改过的 userInput + (computed) displayValue 那种写法 https://github.com/ElemeFE/element/blob/dev/packages/date-picker/src/picker.vue#L492-L508。

watch 的坏处是「难以弄清方法调用顺序和关系」,维护起来很麻烦。容易导致timing的问题(比如某些操作必须 nextTick 等更新)


input 的话,就更不需要维护一个 currentValue 了(现在是watch value再更新currentValue)。额外内部状态会导致 https://jsfiddle.net/60d1cm5j/ 里面 input 的值可以修改,但是代码却刻意无视了input事件,这种情况下 input 显示的内容无法被修改才比较合理(和下面的date-picker对比)。

另外,可以考虑把 textarea 和 input 分成两个子组件,两者的逻辑分开来会比现在的写法更清晰。

@JserWang
Copy link
Author

JserWang commented Aug 9, 2018

@jikkai 这方案确实是个 workaround,在维护两个值的情况下,就需要处理timing的问题,这点我认可 @wacky6 的观点。既然你们有后续的规划,那我再继续关注下 next 分支的情况吧。

@wacky6
Copy link
Contributor

wacky6 commented Aug 9, 2018

这个的 nextTick 和 input 的内部实现也有关联:#12171

@stale
Copy link

stale bot commented Jun 20, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 20, 2020
@stale stale bot closed this Jun 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants