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: form #705

Merged
merged 1 commit into from
Apr 26, 2023
Merged

fix: form #705

merged 1 commit into from
Apr 26, 2023

Conversation

jc9702507
Copy link
Collaborator

@github-actions
Copy link
Contributor

github-actions bot commented Apr 24, 2023

🎊 PR Preview af2e1d9 has been successfully built and deployed to https://ant-design-ant-design-mini-preview-pr-705.surge.sh

🕐 Build time: 248.141s

🤖 By surge-preview

this.ref.setFormData({
required: this.required,
});
}
let validateTriggerList: ValidateTrigger[] | ValidateTrigger = validateTrigger || 'onChange';
if (!Array.isArray(validateTriggerList)) {
validateTriggerList = [validateTriggerList];
Copy link
Contributor

Choose a reason for hiding this comment

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

这段代码是一个类 Field 的部分修改,以下是我对这段代码的评价和建议:

  • 在第 3 行中新增一个名为 update 的参数来提高可读性是很不错的做法。
  • 第 11 行,在第八个参数上的添加了一个布尔值。考虑到在创建字段时该标记值默认为假,这里修改它的值为真作用不大,可以将其去掉。
  • 第 16 - 24 行是处理数据更新逻辑的关键部分。通过判断 update 这一标记值是否为真,确定需不需要调用 reset 方法。这部分逻辑看起来没有问题,但我们还需要注意代码最后两行加入的关于 validateTriggerList 的赋值,因为此处会直接覆盖方法中参数 validateTrigger 的值,可能会影响代码的其他部分。

我的建议是:如果需要更新表单数据,则在方法 create 中加入更新数据时的相关操作,可以避免引用过多外部变量生成的机制使得更新代码难以维护。另外建议针对数据更新逻辑进行单元测试,确保代码正确性。

@codecov
Copy link

codecov bot commented Apr 24, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.03 ⚠️

Comparison is base (e8ed1ad) 24.02% compared to head (af2e1d9) 24.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #705      +/-   ##
==========================================
- Coverage   24.02%   24.00%   -0.03%     
==========================================
  Files         158      158              
  Lines        3334     3337       +3     
  Branches      873      875       +2     
==========================================
  Hits          801      801              
- Misses       2202     2205       +3     
  Partials      331      331              
Impacted Files Coverage Δ
src/Form/form.ts 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jc9702507 jc9702507 merged commit 7f8c424 into master Apr 26, 2023
@github-actions github-actions bot mentioned this pull request Apr 26, 2023
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.

1 participant