Skip to content

fix: trigger onChange twice when inputting using the input method - #60

Closed
Adrian-Zephyr-Liao wants to merge 4 commits into
react-component:masterfrom
Adrian-Zephyr-Liao:fix-twice-onchange
Closed

fix: trigger onChange twice when inputting using the input method#60
Adrian-Zephyr-Liao wants to merge 4 commits into
react-component:masterfrom
Adrian-Zephyr-Liao:fix-twice-onchange

Conversation

@Adrian-Zephyr-Liao

@Adrian-Zephyr-LiaoAdrian-Zephyr-Liao commented Dec 25, 2023•
edited by yoyo837
Loading

Copy link
Copy Markdown

The onCompositionEnd event will be called when the user input method combination ends, triggering an onChange event, but it only needs to be called after the count method of Input is successfully cropped.

fixant-design/ant-design#46587
close#61

@Adrian-Zephyr-Liao

Adrian-Zephyr-Liao commented Dec 25, 2023

Copy link
Copy Markdown
Author

ant-design/ant-design#46587

yoyo837
yoyo837 reviewed Dec 25, 2023
View reviewed changes
Comment threadsrc/Input.tsx Outdated
@yoyo837

yoyo837 commented Dec 25, 2023

Copy link
Copy Markdown
Member

需要添加测试用例

@codecov

codecovBot commented Dec 25, 2023•
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9640649) 97.31% compared to head (a8a9b4e) 97.40%.

Additional details and impacted files
@@ Coverage Diff @@## master #60 +/- ##
==========================================
+ Coverage 97.31% 97.40% +0.09% 
==========================================
Files 5 5 Lines 186 193 +7 Branches 68 69 +1 ==========================================
+ Hits 181 188 +7 
Misses 5 5

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

@Adrian-Zephyr-Liao

Adrian-Zephyr-Liao commented Dec 25, 2023

Copy link
Copy Markdown
Author

需要添加测试用例

已添加测试用例，主要有两点：

  1. 在未发生count裁剪时，onChange触发一回
  2. 在发生裁剪且裁剪后的字符小于max时，compositionEnd中会触发onChange，总共两次

yoyo837
yoyo837 reviewed Dec 26, 2023
View reviewed changes
Comment threadsrc/Input.tsx
cutValue = countConfig.exceedFormatter(currentValue, {
max: countConfig.max,
if (isExceed(currentValue)) {
cutValue = countConfig.exceedFormatter!(currentValue, {

@yoyo837yoyo837Dec 26, 2023

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.

这里不要使用!

@yoyo837yoyo837Dec 26, 2023

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.

isExceed 里确实判断，看有没有更好的代替？

@Adrian-Zephyr-LiaoAdrian-Zephyr-LiaoDec 26, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

我目前能想到的还有一个思路是：
将formatter相关的代码抽离出来，返回一个cutValue和isExceed
const { cutValue, isExceed } = onInternalFormatter(e.currentTarget.value);
在onInternalChange的事件中，formatter后直接触发triggerChange
在onInternalCompositionEnd的事件中，判断是否isExceed，如果为true就触发triggerChange

@yoyo837yoyo837Dec 26, 2023

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.

感觉这样会容易混淆逻辑 🤔

@Adrian-Zephyr-LiaoAdrian-Zephyr-LiaoDec 26, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

我认为fomatter是一个独立的事情，isExceed和cutValue都是和formatter相关的内容，表示格式化后是否超出限制和最终裁剪的值。目前的处境是在change的时候需要格式化，在compositionEnd的时候需要格式化并且条件调用change

@yoyo837yoyo837Dec 26, 2023

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.

改改看

yoyo837
yoyo837 reviewed Dec 27, 2023
View reviewed changes
Comment threadsrc/Input.tsx
});

if (currentValue !== cutValue) {
setSelection([

@yoyo837yoyo837Dec 27, 2023

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.

一个看似计算的方法，内部确在调用setSelection, 有点奇怪

@yoyo837yoyo837Dec 27, 2023

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.

有副作用的意思。

@yoyo837

yoyo837 commented Dec 27, 2023•
edited
Loading

Copy link
Copy Markdown
Member

image

要不要这样改，还简单一点, diff 也少一些

@yoyo837yoyo837 mentioned this pull request Dec 27, 2023
Merged
@yoyo837

yoyo837 commented Dec 27, 2023

Copy link
Copy Markdown
Member

#61 你觉得怎样？

@Adrian-Zephyr-Liao

Adrian-Zephyr-Liao commented Dec 27, 2023

Copy link
Copy Markdown
Author

#61 你觉得怎样？

emmm，其实我一开始也想在triggerChange上加参数，这确实是最简单的改法。但往triggerChange这种语义化的函数中堆除了e和value之外的参数总感觉怪怪的😂如果后续再来个类似的需求，又得加参数了。
看怎么考虑吧，这里也不是对外暴露的API，这么写也没啥关系

@Adrian-Zephyr-Liao

Adrian-Zephyr-Liao commented Dec 27, 2023

Copy link
Copy Markdown
Author

#61 你觉得怎样？

纯函数有些时候也不一定能保证，无非是把set值的操作再放到另外一个函数中去

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yoyo837yoyo837yoyo837 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Input onChange trigger two times when input chinese

2 participants

@Adrian-Zephyr-Liao@yoyo837