-
Notifications
You must be signed in to change notification settings - Fork 267
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
feat(Picker): 增加可选项禁用配置 #2804
base: next
Are you sure you want to change the base?
feat(Picker): 增加可选项禁用配置 #2804
Conversation
Walkthrough此次更改主要涉及 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (11)
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2804 +/- ##
=======================================
Coverage 84.02% 84.02%
=======================================
Files 220 220
Lines 17907 17909 +2
Branches 2625 2627 +2
=======================================
+ Hits 15046 15048 +2
Misses 2856 2856
Partials 5 5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (11)
src/packages/picker/demos/h5/demo1.tsx (4)
25-25
: 代码实现正确,建议增强示例代码正确展示了禁用选项的使用方法,但建议考虑以下改进:
- 添加多个禁用选项的示例,以更好地展示该功能
- 考虑在不同列表层级中添加禁用示例
建议增加更多示例:
const listData1 = [ [ { value: 1, text: '南京市' }, - { value: 2, text: '无锡市' }, + { value: 2, text: '无锡市', disabled: true }, { value: 3, text: '海北藏族自治区' }, { value: 4, text: '北京市' }, - { value: 5, text: '连云港市' }, + { value: 5, text: '连云港市', disabled: true }, { value: 8, text: '大庆市' }, { value: 9, text: '绥化市' }, { value: 10, text: '潍坊市' }, { value: 12, text: '乌鲁木齐市', disabled: true }, ], ]
Line range hint
29-31
: 建议增加禁用选项的处理逻辑在
changePicker
回调函数中,建议添加对禁用选项的处理逻辑,以便更好地展示如何处理禁用项的交互。建议添加以下处理逻辑:
const changePicker = (list: any[], option: any, columnIndex: number) => { - console.log(columnIndex, option) + if (option.disabled) { + console.log('当前选项已禁用:', option.text) + return + } + console.log('选中项:', option.text, '列索引:', columnIndex) }
Line range hint
32-41
: 建议优化确认回调的类型定义在
confirmPicker
函数中,建议使用更严格的类型定义,避免使用any
类型。建议修改如下:
const confirmPicker = ( options: PickerOption[], values: (string | number)[] ) => { let description = '' - options.forEach((option: any) => { + options.forEach((option: PickerOption) => { description += ` ${option.text}` }) setBaseDesc(description) }
禁用状态功能验证结果
经过代码分析,Picker 组件的禁用功能实现存在以下问题:
- 虽然在 PickerOption 接口中定义了 disabled 属性,但在组件实现中对禁用状态的处理不够完整
- 仅在 pickerpanel.tsx 中实现了基础的禁用判断,但缺乏完整的交互反馈
- 禁用项的样式类名已定义,但未找到相应的样式实现
建议:
- 需要完善禁用状态下的样式定义
- 在选择器滚动和点击时需要增加禁用状态的处理逻辑
- 建议添加禁用状态下的视觉反馈
🔗 Analysis chain
Line range hint
47-56
: 验证禁用功能的实现需要确保 Picker 组件正确处理了禁用状态的交互逻辑。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查 Picker 组件是否正确实现了禁用状态的处理 # 搜索 Picker 组件中与禁用状态相关的实现 echo "检查 Picker 组件实现..." ast-grep --pattern 'disabled' src/packages/picker/ # 检查样式文件中是否包含禁用状态的样式定义 echo "检查样式实现..." rg -l "disabled" src/packages/picker/**/*.scssLength of output: 310
Script:
#!/bin/bash # 检查 Picker 组件的实现文件 echo "查找 Picker 组件文件..." fd -e tsx -e ts "picker" src/packages/picker # 检查组件属性定义 echo "检查组件属性定义..." ast-grep --pattern 'interface $_ { $$$ }' src/packages/picker # 检查样式文件 echo "查找样式文件..." fd -e scss -e css . src/packages/picker # 检查组件实现中的条件渲染 echo "检查条件渲染逻辑..." ast-grep --pattern 'const $_ = ($$$) => { $$$ }' src/packages/pickerLength of output: 102073
Script:
#!/bin/bash # 检查 Picker 组件的实现文件中的禁用状态处理 echo "检查 Picker 组件实现..." ast-grep --pattern 'if ($_.disabled) { $$$ }' src/packages/picker/ # 检查 Picker 组件的主文件 echo "检查 Picker 主文件..." rg -A 5 "disabled" src/packages/picker/picker.tsx # 检查 PickerPanel 组件中的禁用状态处理 echo "检查 PickerPanel 组件..." rg -A 5 "disabled" src/packages/picker/pickerpanel.tsxLength of output: 735
src/packages/picker/demos/taro/demo1.tsx (1)
25-25
: 代码实现正确,建议增强示例代码正确展示了禁用选项的基本用法。
建议考虑以下优化建议来增强示例的完整性:
- 添加多个禁用选项以展示不同场景
- 在
changePicker
回调中增加对禁用选项的处理逻辑演示- 在界面上通过文案说明禁用状态的表现
const listData1 = [ [ { value: 1, text: '南京市' }, - { value: 2, text: '无锡市' }, + { value: 2, text: '无锡市', disabled: true }, { value: 3, text: '海北藏族自治区' }, { value: 4, text: '北京市' }, - { value: 5, text: '连云港市' }, + { value: 5, text: '连云港市', disabled: true }, { value: 8, text: '大庆市' }, { value: 9, text: '绥化市' }, { value: 10, text: '潍坊市' }, { value: 12, text: '乌鲁木齐市', disabled: true }, ], ]src/packages/picker/doc.zh-TW.md (1)
106-106
: 文档更新清晰完整!
disabled
属性的文档说明简洁明了,类型和默认值定义准确。建议在示例代码部分添加一个展示禁用选项的使用案例,以帮助用户更好地理解此新功能。例如:
const options = [ { text: '北京市', value: '010' }, { text: '上海市', value: '021' }, { text: '乌鲁木齐市', value: '0991', disabled: true } ]src/packages/picker/doc.taro.md (1)
106-106
: 文档更新清晰完整
disabled
属性的文档说明简洁明了,类型和默认值的定义也很清晰。建议在示例代码部分添加一个展示禁用选项的使用案例,以帮助开发者更好地理解此功能。src/packages/picker/doc.en-US.md (2)
106-106
: 建议补充禁用选项的使用说明建议在文档中补充以下内容:
- 禁用选项的具体表现效果
- 使用场景示例
- 与其他属性的组合使用说明
-| disabled | Whether to disable | `boolean` | `false` | +| disabled | Whether to disable the option. When true, the option cannot be selected and appears grayed out | `boolean` | `false` |
Line range hint
1-1
: 建议添加禁用选项的演示代码为了帮助用户更好地理解禁用选项的功能,建议在 Demo 部分添加新的示例,展示:
- 单列选择器中的禁用选项
- 多列选择器中的禁用选项
- 级联选择器中的禁用选项
建议在 "Basic Usage" 之后添加新的示例部分:
### Disabled Options Use the `disabled` property to disable specific options. :::demo <CodeBlock src='h5/demo-disabled.tsx'></CodeBlock> :::需要我帮您生成相应的演示代码吗?
src/packages/picker/picker.scss (1)
122-125
: 样式实现符合组件库规范新增的禁用状态样式实现简洁明确,并且使用了主题变量
$picker-item-text-color-disabled
来保持样式的一致性和可配置性。建议考虑添加以下额外的样式属性来增强禁用状态的视觉反馈:
&-disabled { color: $picker-item-text-color-disabled; + cursor: not-allowed; + opacity: 0.6; }src/styles/variables.scss (1)
Line range hint
1-1000
: 建议优化变量分组和注释文件结构建议:
- 考虑按功能对变量进行分组,如颜色、字体、间距等
- 为每个分组添加清晰的中文注释
- 保持变量命名的一致性,特别是在前缀使用上
建议参考以下组织结构:
// 主题色 $color-primary: ... // 文字颜色 $color-text: ... // 字体大小 $font-size-base: ... // 间距 $spacing-base: ... // 组件变量 // Picker 组件 $picker-*: ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
src/packages/picker/demos/h5/demo1.tsx
(1 hunks)src/packages/picker/demos/taro/demo1.tsx
(1 hunks)src/packages/picker/doc.en-US.md
(1 hunks)src/packages/picker/doc.md
(1 hunks)src/packages/picker/doc.taro.md
(1 hunks)src/packages/picker/doc.zh-TW.md
(1 hunks)src/packages/picker/picker.scss
(1 hunks)src/packages/picker/pickerpanel.taro.tsx
(2 hunks)src/packages/picker/pickerpanel.tsx
(2 hunks)src/styles/variables-jmapp.scss
(1 hunks)src/styles/variables.scss
(1 hunks)
🔇 Additional comments (6)
src/packages/picker/pickerpanel.tsx (2)
117-119
: 禁用选项的处理逻辑正确实现
代码正确地实现了禁用选项的功能,通过提前返回来阻止选择被禁用的选项。实现简单明了且有效。
273-273
: 样式类的条件渲染实现恰当
使用条件渲染添加禁用状态的样式类,实现符合 React 最佳实践。建议在相关的样式文件中确保 nut-picker-roller-item-disabled
类定义了适当的视觉反馈。
src/packages/picker/pickerpanel.taro.tsx (2)
121-123
: 禁用选项的处理逻辑正确实现
setChooseValue
函数中的禁用选项检查实现简洁有效,能够正确阻止用户选择被禁用的选项。
283-283
: UI 样式集成合理
禁用状态的类名 nut-picker-roller-item-disabled
与现有的条件类名集成得当,保持了代码的一致性。建议确认相关的 CSS 样式是否已正确实现。
✅ Verification successful
样式定义已正确实现
在 picker.scss
文件中已找到对应的禁用状态样式定义:
&-item,
&-roller-item {
// ...
&-disabled {
color: $picker-item-text-color-disabled;
}
}
样式通过 CSS 变量 $picker-item-text-color-disabled
控制禁用状态的文本颜色,实现方式规范且合理。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否存在对应的禁用状态样式定义
rg -l "nut-picker-roller-item-disabled" --type scss
Length of output: 80
Script:
#!/bin/bash
# 使用更广泛的搜索来查找样式定义,包括其他可能的样式文件类型
rg -l "nut-picker-roller-item-disabled"
# 查找所有可能的样式文件
fd -e css -e scss -e less -e sass -e stylus
Length of output: 6158
Script:
#!/bin/bash
# 检查 picker.scss 文件中的样式定义
cat src/packages/picker/picker.scss
# 检查相关的主题文件中是否包含该样式
cat src/styles/theme-default.scss | rg -A 5 -B 5 "picker"
Length of output: 3441
src/styles/variables.scss (1)
553-556
: 变量命名和值设置合理
新增的 $picker-item-text-color-disabled
变量:
- 命名符合项目规范,清晰表达了其用途
- 复用了系统中已有的
$color-text-disabled
变量,保持了设计一致性 - 正确使用了 CSS 变量和
!default
标志,支持主题定制
src/styles/variables-jmapp.scss (1)
587-590
: 变量定义正确且符合规范!
新增的 $picker-item-text-color-disabled
变量定义合理:
- 遵循了组件变量命名规范
- 正确使用了 CSS 自定义属性作为首选值
- 合理使用了
$color-text-disabled
作为默认值,保持了设计token的一致性
@@ -103,6 +103,7 @@ import { Picker } from '@nutui/nutui-react' | |||
| --- | --- | --- | --- | | |||
| text | 选项的文字内容 | `string` \| `number` | `-` | | |||
| value | 选项对应的值,且唯一 | `string` \| `number` | `-` | | |||
| disabled | 是否禁用 | `boolean` | `false` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
需要补充禁用状态的使用示例说明
示例代码中已经包含了 disabled
属性的使用,但文档中缺少对应的使用说明和示例代码展示。建议在文档中添加以下内容:
- 在"基础用法"或"禁用选项"小节中展示如何使用
disabled
属性 - 添加示例代码,说明如何禁用特定选项
🔗 Analysis chain
文档更新清晰完整!
disabled
属性的文档说明简洁明了,类型定义和默认值的描述符合规范。
运行以下脚本验证示例代码中是否正确展示了禁用选项的用法:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查示例代码中是否包含 disabled 属性的使用示例
# 搜索示例代码中的 disabled 属性使用
fd -e tsx -e jsx . src/packages/picker/demos -x rg -l "disabled.*true"
Length of output: 152
Script:
#!/bin/bash
# 检查示例代码中 disabled 属性的具体使用情况
rg -A 5 "disabled.*true" src/packages/picker/demos/h5/demo1.tsx src/packages/picker/demos/taro/demo1.tsx
# 检查文档中是否包含禁用状态的示例说明
rg -A 5 "disabled" src/packages/picker/doc.md
Length of output: 1094
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
需要关注 confirm 的时候,会返回其他的选项值,导致错误。
H5 可复现。
@@ -118,7 +118,9 @@ const InternalPickerPanel: ForwardRefRenderFunction< | |||
} | |||
|
|||
const setChooseValue = (move: number) => { | |||
chooseItem?.(options?.[Math.round(-move / lineSpacing.current)], keyIndex) | |||
const option = options?.[Math.round(-move / lineSpacing.current)] | |||
if (option.disabled) return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
选择的时候不可以选,也要体现在 confirm 的时候 不要选其他的值。
在 confirm 的时候 最好可以增加回调,告知该项 disabled。
31ccb51
to
bff8f63
Compare
🤔 这个变动的性质是?
🔗 相关 Issue
#2575
💡 需求背景和解决方案
实现 Picker 的 options 增加
disabled
配置项☑️ 请求合并前的自查清单
Summary by CodeRabbit
新特性
disabled
属性的说明。样式
文档
disabled
属性的类型和默认值说明。