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(selector): 修复选中判断 #707

Merged
merged 1 commit into from
Apr 26, 2023
Merged

fix(selector): 修复选中判断 #707

merged 1 commit into from
Apr 26, 2023

Conversation

wyj580231
Copy link
Collaborator

No description provided.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 26, 2023

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

🕐 Build time: 804.759s

🤖 By surge-preview


### SelectorItem
| 属性 | 说明 | 类型 | 默认值 |
| -----|-----|-----|----- |
| disabled | 是否禁用 | boolean | - |
| subText | 副文案 | string | - |
| text | 文案 | string | - |
| value | 当前项value | number \| string | - |
| value | 当前项value | string \| number | - |


Copy link
Contributor

Choose a reason for hiding this comment

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

这段代码的主要变更是修改了defaultValue和value属性类型,由原本只支持单选项的string、number类型扩展为支持多选项的string、number类型数组;同时onChange回调函数的参数也进行了相应的调整,增加了支持多选项情况下,传递选中项的数组形式。

在功能上看起来比较完整,但是从代码本身的角度没有发现特别明显的bug风险。如果后续有需要改进的地方,可以考虑增加对数据类型的判断和错误处理,以及简化部分重复代码等方面进行优化。

@@ -99,7 +91,7 @@ Component({
if (onChange) {
onChange(
value,
options.find((v) => v.value === value) as any,
options.find((v) => v.value === value),
fmtEvent(this.props)
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

这份代码看起来只是对旧有的代码做了一些小的修改。以下是我对修改后的代码可能存在的问题和建议的总结:

  1. 第 4 行代码中 mixins 对象被修改为 mixinValue() 调用,但是该函数并没有传递任何参数。如果该函数需要传递参数,应当进行相应的修改。
  2. 第 37 行代码中 currentValue 变量被赋值时,使用了逻辑或运算符 '||',但是它可能带来未预期的问题。如果 getValue() 返回一个值为 0, '', false 等,在进行逻辑或运算时就会将其转换为 false 而选择后面赋值的数组 []。如果这不是期望的行为,可以使用三目运算符代替逻辑或运算符。
  3. 第 66、71 行代码中使用了 as any。如果类型已经明确,应避免使用此类断言,因为在编译期间不能提供足够的类型安全性,导致潜在错误。
  4. 建议考虑在所有出现的事件处理程序中显示地检查 onChange 和 onSelectMin 是否定义,如果不存在相应的回调函数,可能会引起异常。
  5. 是否可以修改该部分代码以提高可扩展性。最好的方法是将此代码重构为支持钩子函数的通用组件,使得用户可以绑定回调函数来实现自己的业务逻辑,并将其鉴别为更好的可读、可测试和可扩展代码。

希望我的回答能对您有所帮助。

/**
* @description 触发最小限制
*/
onSelectMin(value: string, item: ISelectorItem, e: Record<string, any>): void;
onSelectMin(value: Value, item: ISelectorItem, e: Record<string, any>): void;
}

export const SelectorDefaultProps: Partial<ISelectorProps> = {
Copy link
Contributor

Choose a reason for hiding this comment

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

这段代码的基本功能是实现一个选择器组件,其中包括以下几个方面的内容:

  • 增加 Value 类型:增加了 Value 类型用于表示选择项的值,可以是字符串或数字类型。
  • 修改 ISelectorItem 接口中:value 的类型为 Value.
  • 新增 defaultValue 属性:新增默认选择项的属性。
  • 修改 value 属性的类型:将 value 属性的值类型变为 Value | Value[]
  • 增加最大选中数和禁用状态的属性。
  • 修改 onChangeonSelectMaxonSelectMin 函数的参数类型,使其能够支持新的 Value 类型。

在代码上没有明显的 bug 风险,但是建议增加注释,特别是解释一下每个函数和接口的作用和参数含义。此外,还可以进一步考虑优化组件的可维护性和可复用性。

@codecov
Copy link

codecov bot commented Apr 26, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.02 🎉

Comparison is base (e8ed1ad) 24.02% compared to head (0101e45) 24.04%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #707      +/-   ##
==========================================
+ Coverage   24.02%   24.04%   +0.02%     
==========================================
  Files         158      158              
  Lines        3334     3331       -3     
  Branches      873      873              
==========================================
  Hits          801      801              
+ Misses       2202     2199       -3     
  Partials      331      331              
Impacted Files Coverage Δ
src/Selector/index.ts 0.00% <0.00%> (ø)
src/Selector/props.ts 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 62d21b2 into master Apr 26, 2023
@github-actions github-actions bot mentioned this pull request Apr 26, 2023
@DiamondYuan DiamondYuan deleted the fix/Selector_0426 branch October 27, 2023 06:34
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.

2 participants