Skip to content

fix(TimePicker): fix the 'modelValue' watch defect#13095

Open
pany-ang wants to merge 1 commit intoyouzan:mainfrom
pany-ang:fix-time-picker-watch-modelValue
Open

fix(TimePicker): fix the 'modelValue' watch defect#13095
pany-ang wants to merge 1 commit intoyouzan:mainfrom
pany-ang:fix-time-picker-watch-modelValue

Conversation

@pany-ang
Copy link
Copy Markdown
Contributor

解决该 issue 中提到的第 2 个问题: #13084

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 30, 2024

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.68%. Comparing base (867e612) to head (51d89eb).
⚠️ Report is 313 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13095      +/-   ##
==========================================
+ Coverage   89.66%   89.68%   +0.01%     
==========================================
  Files         257      257              
  Lines        6987     6988       +1     
  Branches     1724     1725       +1     
==========================================
+ Hits         6265     6267       +2     
+ Misses        382      380       -2     
- Partials      340      341       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pany-ang
Copy link
Copy Markdown
Contributor Author

同理,逻辑相似的 DatePicker 组件也有该问题。如果这个 PR 通过了,到时候我会再提交一个修复 DatePicker 的 PR

@inottn
Copy link
Copy Markdown
Collaborator

inottn commented Aug 30, 2024

可以补充一下单元测试

@pany-ang
Copy link
Copy Markdown
Contributor Author

该 PR 不太好用单测来测试,暂时没找到通过单测触发新增逻辑的办法。参考 issue 中提到的手动复现办法,也是需要两个组件 + 人工按照一定逻辑操作 4 次界面。

如果 CR 能通过,也许这里并不需要单测也行。

const _newValues = formatValueRange(newValues, columns.value);
if (
!isSameValue(_newValues, currentValues.value) ||
!isSameValue(_newValues, newValues)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这里为什么需要加这个判断呢,从 issue 中没有看到解释

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  1. v-model 修改为 ['00', '00'] 时,此时 UI 上没有 ['00', '00'] 选项,UI 被迫选中最接近的一个值,但是 v-model 没有同步更新为该值,依旧是 ['00', '00'](UI 变了,但 v-model 没变)(可能是一个 BUG)

对应的是 issue 中这段话

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

对于新加的这个 if 判断,似乎只有在 currentValues.value 和 _newValues 相等的时候会触发,那下面的 currentValues.value = _newValues; 赋值有什么意义呢?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

该赋值为了触发这段侦听器:

    watch(currentValues, (newValues) => {
      if (!isSameValue(newValues, props.modelValue)) {
        emit('update:modelValue', newValues);
      }
    });

目的是让内部的 currentValues 同步到外部的 v-model 绑定的变量

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.

4 participants