feat(checkbox): v14 #2802
feat(checkbox): v14 #2802
Conversation
Walkthrough此次变更涉及对复选框组件的样式和结构进行重组。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Checkbox
participant Icon
User->>Checkbox: 点击复选框
Checkbox->>Icon: 渲染图标状态
Icon-->>Checkbox: 返回图标
Checkbox-->>User: 更新复选框状态
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (5)
src/packages/checkbox/icon.tsx (2)
10-22: 建议优化组件性能和类名处理
- 考虑使用
React.memo来避免不必要的重渲染- 建议使用类名工具库（如 classnames）来处理类名组合，以提高可维护性
建议按照以下方式修改：
+import classNames from 'classnames' + -const Icon = (props: Partial<IconProps>) => { +const Icon = React.memo((props: Partial<IconProps>) => { const { tag, classPrefix, name } = mergeProps( { tag: 'div', classPrefix: 'nut-checkbox-self', name: 'normal', }, props ) return createElement<{ className: string }>(tag, { - className: `${classPrefix} ${classPrefix}-${name}`, + className: classNames(classPrefix, `${classPrefix}-${name}`), }) -} +})
1-24: 建议完善组件文档和测试为了提高组件的可维护性和可靠性，建议：
- 添加组件的 JSDoc 文档，包括：
- 组件的用途说明
- 属性的详细描述
- 使用示例
- 添加单元测试，覆盖：
- 不同状态的渲染结果
- 属性的默认值处理
- 自定义标签的渲染
需要我帮助生成相关的文档模板或测试用例吗？
src/packages/checkbox/checkbox.scss (2)
3-14: 建议优化边框宽度以提高可见度
border: 0.5px在某些显示设备上可能显示不清晰，建议考虑使用1px或使用transform: scale()实现更细的边框。- border: 0.5px solid $color-text-disabled; + border: 1px solid $color-text-disabled;
Line range hint
1-200: 整体评估：代码结构清晰，但建议进行性能优化代码结构组织良好，状态管理清晰，但有以下建议：
- 考虑使用 CSS 变量统一管理尺寸和颜色值
- 可以通过 CSS 选择器优化来减少样式规则的重复
- 建议添加响应式断点支持
建议考虑以下优化方向：
- 将重复的样式值抽取为主题变量
- 添加媒体查询以支持不同设备尺寸
- 考虑使用 CSS Grid 优化布局结构
src/packages/checkbox/checkbox.tsx (1)
105-125: 建议重构图标渲染逻辑以提高可读性当前的条件渲染逻辑较为复杂，建议进行优化以提高代码的可维护性。
建议采用以下方式重构：
const renderIcon = () => { - if (innerDisabled) { - if (innerIndeterminate) { - return <Icon name="disabled" /> - } - if (innerChecked) { - return <Icon name="checked-disabled" /> - } - return <Icon name="disabled" /> - } - if (!innerChecked) { - return React.isValidElement(icon) ? icon : <Icon name="normal" /> - } - if (innerIndeterminate) { - return React.isValidElement(indeterminateIcon) ? ( - indeterminateIcon - ) : ( - <Icon name="disabled" /> - ) - } - return React.isValidElement(activeIcon) ? ( - activeIcon - ) : ( - <Icon name="checked" /> - ) + const getIconName = () => { + if (innerDisabled) { + if (innerChecked) return 'checked-disabled' + return 'disabled' + } + if (innerIndeterminate) return 'disabled' + if (innerChecked) return 'checked' + return 'normal' + } + + const customIcon = innerChecked ? activeIcon : icon + return React.isValidElement(customIcon) ? customIcon : <Icon name={getIconName()} /> }优化建议：
- 将图标名称的确定逻辑独立出来，使用单独的函数处理
- 减少嵌套的条件判断，使逻辑更清晰
- 统一处理自定义图标的逻辑
这样的重构可以：
- 降低代码复杂度
- 提高可读性
- 使维护更容易
- 减少潜在的错误
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/packages/checkbox/checkbox.scss(1 hunks)src/packages/checkbox/checkbox.taro.tsx(2 hunks)src/packages/checkbox/checkbox.tsx(2 hunks)src/packages/checkbox/icon.tsx(1 hunks)
🔇 Additional comments (3)
src/packages/checkbox/icon.tsx (1)
24-24: 导出语句符合规范
默认导出方式适合单组件文件，符合最佳实践。
src/packages/checkbox/checkbox.tsx (1)
13-13: 导入更改提升了代码组织性！
将多个具体的图标组件替换为统一的 Icon 组件是个很好的改进，这样可以：
- 减少重复代码
- 提高可维护性
- 使图标管理更加集中和统一
src/packages/checkbox/checkbox.taro.tsx (1)
8-8: 导入更改看起来不错！
将图标处理逻辑集中到本地组件是一个很好的架构决策，这样可以：
- 集中管理图标逻辑
- 减少外部依赖
- 便于维护和修改图标行为
| &-checked, | ||
| &-disabled, | ||
| &-checked-disabled { | ||
| &:after { | ||
| display: block; | ||
| position: absolute; | ||
| top: 50%; | ||
| left: 50%; | ||
| transform: translate(-50%, -50%); | ||
| content: ' '; | ||
| background: $color-primary-text; | ||
| } | ||
| } | ||
|
|
||
| &-icon-checked { | ||
| color: $color-primary; | ||
| transition-duration: 0.3s; | ||
| transition-property: color, border-color, background-color; | ||
| &-checked { | ||
| background: linear-gradient( | ||
| 90deg, | ||
| $color-primary-stop-1 0%, | ||
| $color-primary-stop-2 100% | ||
| ); | ||
| box-shadow: 0px 2px 4px 0px rgba(255, 15, 35, 0.2); | ||
|
|
||
| &:after { | ||
| width: 8px; | ||
| height: 8px; | ||
| border-radius: 50%; | ||
| } | ||
| } | ||
|
|
||
| &-disabled { | ||
| background: $color-text-disabled; | ||
|
|
||
| &.nut-checkbox-icon-disabled { | ||
| color: $color-primary-disabled-special; | ||
| &:after { | ||
| width: 8px; | ||
| height: 1.5px; | ||
| } | ||
| } | ||
|
|
||
| &-checked-disabled { | ||
| background: $color-primary-disabled-special; | ||
|
|
||
| &:after { | ||
| width: 8px; | ||
| height: 8px; | ||
| border-radius: 50%; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议将固定数值抽取为 CSS 变量
代码中存在多个魔法数字（如 8px、1.5px 等），建议将这些值抽取为 CSS 变量以提高可维护性和一致性。
+ $checkbox-indicator-size: 8px;
+ $checkbox-disabled-height: 1.5px;
&-checked {
&:after {
- width: 8px;
- height: 8px;
+ width: $checkbox-indicator-size;
+ height: $checkbox-indicator-size;
border-radius: 50%;
}
}Committable suggestion skipped: line range outside the PR's diff.
| color: $checkbox-label-color; | ||
| white-space: nowrap; | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
建议使用文本截断 mixin 替代单独的 white-space: nowrap
代码库中已有成熟的文本截断处理方案，建议使用 @include text-ellipsis() 或 @include oneline-ellipsis() 来处理标签文本。其他组件（如 Tabs、NoticeBar）在使用 white-space: nowrap 时都配合了文本截断样式。
建议修改为：
.nut-checkbox-label {
margin-left: $checkbox-label-margin-left;
font-size: $checkbox-label-font-size;
color: $checkbox-label-color;
@include text-ellipsis();
}🔗 Analysis chain
建议重新考虑 white-space: nowrap 的使用
强制不换行可能导致长文本标签溢出容器。建议添加文本截断或允许适当换行的样式。
.nut-checkbox-label {
margin-left: $checkbox-label-margin-left;
font-size: $checkbox-label-font-size;
color: $checkbox-label-color;
- white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ word-break: break-word;
}🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有其他组件使用类似的标签处理方式
rg -l 'white-space:\s*nowrap' 'src/packages/*/*.scss'
Length of output: 117
Script:
#!/bin/bash
# 使用 fd 查找所有 scss 文件并使用 rg 搜索 white-space: nowrap
fd -e scss -x rg -l 'white-space:\s*nowrap' {}
Length of output: 525
Script:
#!/bin/bash
# 检查其他组件中 white-space: nowrap 的具体使用场景和相关样式
rg 'white-space:\s*nowrap' -B 2 -A 2 ./src/packages/tour/tour.scss ./src/packages/tabs/tabs.scss ./src/packages/noticebar/noticebar.scss ./src/packages/navbar/navbar.scss
# 检查是否有文本截断相关的 mixin 定义
cat ./src/styles/mixins/text-ellipsis.scss
Length of output: 2322
| @@ -97,31 +97,31 @@ export const Checkbox: FC< | |||
| const renderIcon = () => { | |||
| if (innerDisabled) { | |||
| if (innerIndeterminate) { | |||
| return <CheckDisabled className={color()} /> | |||
| return <Icon tag={View} name="disabled" /> | |||
There was a problem hiding this comment.
建议区分不同的禁用状态图标
目前在不同的禁用状态下使用了相同的图标名称 "disabled"，这可能会导致用户体验问题。建议为不同的禁用状态使用独特的图标。
建议修改如下：
- return <Icon tag={View} name="disabled" />
+ return <Icon tag={View} name="indeterminate-disabled" />- return <Icon tag={View} name="disabled" />
+ return <Icon tag={View} name="unchecked-disabled" />Also applies to: 105-105
| return <Icon tag={View} name="disabled" /> | ||
| } | ||
| if (innerChecked) { | ||
| return <Checked className={color()} /> | ||
| return <Icon tag={View} name="checked-disabled" /> | ||
| } | ||
| return <CheckDisabled className={color()} /> | ||
| return <Icon tag={View} name="disabled" /> | ||
| } | ||
| if (!innerChecked) { | ||
| return React.isValidElement(icon) ? ( | ||
| icon | ||
| ) : ( | ||
| <CheckNormal className={color()} /> | ||
| <Icon tag={View} name="normal" /> | ||
| ) | ||
| } | ||
| if (innerIndeterminate) { | ||
| return React.isValidElement(indeterminateIcon) ? ( | ||
| indeterminateIcon | ||
| ) : ( | ||
| <CheckDisabled className={color()} /> | ||
| <Icon tag={View} name="disabled" /> | ||
| ) | ||
| } | ||
| return React.isValidElement(activeIcon) ? ( | ||
| activeIcon | ||
| ) : ( | ||
| <Checked className={color()} /> | ||
| <Icon tag={View} name="checked" /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议添加图标名称的类型检查
为了提高代码的可维护性和类型安全性，建议为 Icon 组件的 name 属性添加类型定义。
建议在 Icon 组件中添加以下类型定义：
type IconName = 'disabled' | 'checked-disabled' | 'normal' | 'checked' | 'indeterminate' | 'indeterminate-disabled' | 'unchecked-disabled';
interface IconProps {
tag: typeof View;
name: IconName;
}| ) | ||
| } | ||
| if (innerIndeterminate) { | ||
| return React.isValidElement(indeterminateIcon) ? ( | ||
| indeterminateIcon | ||
| ) : ( | ||
| <CheckDisabled className={color()} /> | ||
| <Icon tag={View} name="disabled" /> |
There was a problem hiding this comment.
修复 indeterminate 状态下的图标显示
当前 indeterminate 状态使用了 "disabled" 图标，这似乎是不正确的。
建议修改如下：
- return <Icon tag={View} name="disabled" />
+ return <Icon tag={View} name="indeterminate" />Committable suggestion skipped: line range outside the PR's diff.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #2802 +/- ##
==========================================
Coverage 83.98% 83.99%
==========================================
Files 220 221 +1
Lines 17882 17877 -5
Branches 2676 2666 -10
==========================================
- Hits 15019 15015 -4
+ Misses 2858 2857 -1
Partials 5 5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
src/packages/checkbox/__test__/__snapshots__/checkbox.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
src/packages/checkbox/__test__/checkbox.spec.tsx(3 hunks)
🔇 Additional comments (3)
src/packages/checkbox/__test__/checkbox.spec.tsx (3)
78-79: 选择器更新符合新的命名规范！
测试用例中的类选择器已更新为新的命名规范，同时保持了原有的测试逻辑完整性。
127-129: 选择器正确反映了组合状态！
测试用例中的类选择器 .nut-checkbox-self-checked-disabled 准确地反映了复选框的选中且禁用状态。
182-182: 选择器更新保持一致性！
列表模式测试中的类选择器更新与之前的更改保持一致，符合新的命名规范。
| expect(container.querySelector('.nut-checkbox-self-disabled')).toBeTruthy() | ||
| // expect( | ||
| // container.querySelector('.nut-checkbox-self-indeterminate') | ||
| // ).toHaveClass('nut-checkbox-self-disabled') |
There was a problem hiding this comment.
建议完善组合状态的测试覆盖
当前测试用例只验证了禁用状态，但根据测试名称，应该同时验证 disabled 和 indeterminate 两种状态。建议取消注释并完善测试断言。
建议修改为：
expect(container.querySelector('.nut-checkbox-self-disabled')).toBeTruthy()
- // expect(
- // container.querySelector('.nut-checkbox-self-indeterminate')
- // ).toHaveClass('nut-checkbox-self-disabled')
+ expect(container.querySelector('.nut-checkbox-self-indeterminate')).toBeTruthy()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(container.querySelector('.nut-checkbox-self-disabled')).toBeTruthy() | |
| // expect( | |
| // container.querySelector('.nut-checkbox-self-indeterminate') | |
| // ).toHaveClass('nut-checkbox-self-disabled') | |
| expect(container.querySelector('.nut-checkbox-self-disabled')).toBeTruthy() | |
| expect(container.querySelector('.nut-checkbox-self-indeterminate')).toBeTruthy() |
| expect( | ||
| container.querySelector('.nut-checkbox-icon-indeterminate') | ||
| ).toBeTruthy() | ||
| expect(container.querySelector('.nut-checkbox-self-disabled')).toBeTruthy() |
There was a problem hiding this comment.
测试用例与测试名称不符
该测试用例命名为测试 indeterminate 状态，但实际上在验证 disabled 状态。建议修改测试用例以确保正确验证 indeterminate 状态。
建议修改为：
- expect(container.querySelector('.nut-checkbox-self-disabled')).toBeTruthy()
+ expect(container.querySelector('.nut-checkbox-self-indeterminate')).toBeTruthy()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(container.querySelector('.nut-checkbox-self-disabled')).toBeTruthy() | |
| expect(container.querySelector('.nut-checkbox-self-indeterminate')).toBeTruthy() |
|
等待Icon，所以转 Draft |
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
Icon组件，简化复选框图标的渲染逻辑。