Conversation
Walkthrough此次变更在配置文件中新增了 PickerView 组件条目，并为其引入了测试用例、演示组件（适用于 H5 和 Taro 平台）、文档、样式、类型定义以及工具函数。同时，还添加了支持触摸滚动效果的 PickerRoller 组件，并在入口文件中导出相应类型，使整个 PickerView 组件的功能得到完善和对外暴露。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 用户
participant PV as PickerView
participant PR as PickerRoller
participant Callback as onChange
User->>PV: 开始触摸与选择操作
PV->>PR: 转发触摸事件
PR->>PR: 处理滚动、惯性计算
PR-->>PV: 更新组件选中值
PV->>Callback: 触发 onChange 回调
Suggested reviewers
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 (
|
github-actions
bot
added
3.x
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🔭 Outside diff range comments (1)
src/packages/pickerview/demos/taro/demo6.tsx (1)
1-81: 🛠️ Refactor suggestion建议重构以减少代码重复
H5 和 Taro 版本的 Demo6 代码几乎完全相同，建议创建一个共享的基础组件来减少代码重复。
建议采用以下方式重构：
- 创建共享的数据文件：
+// src/packages/pickerview/demos/data/cities.ts +export const citiesData = [...]
- 创建共享的基础组件：
+// src/packages/pickerview/demos/base/demo6.base.tsx +import React from 'react' +import { citiesData } from '../data/cities' + +export const createDemo6 = (PickerView, Cell) => { + return () => { + return ( + <> + <Cell> + <PickerView + defaultValue={[1]} + options={citiesData} + onChange={({ value, selectedOptions }) => { + if (process.env.NODE_ENV === 'development') { + console.log('onChange', value, selectedOptions) + } + }} + /> + </Cell> + </> + ) + } +}
- 在 H5 和 Taro 版本中复用：
+// H5 version +import { PickerView, Cell } from '@nutui/nutui-react' +import { createDemo6 } from '../base/demo6.base' + +export default createDemo6(PickerView, Cell) +// Taro version +import { PickerView, Cell } from '@nutui/nutui-react-taro' +import { createDemo6 } from '../base/demo6.base' + +export default createDemo6(PickerView, Cell)
🧹 Nitpick comments (42)
src/packages/pickerview/pickerview.tsx (3)
82-83: 使用可选链精简条件判断
可以将columnOptions && columnOptions.children简化为可选链写法，增强可读性并避免潜在的空对象解引用。- while (columnOptions && columnOptions.children) { + while (columnOptions?.children) { ... }🧰 Tools
🪛 Biome (1.9.4)
[error] 83-83: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
116-119: 避免对数组直接进行引用比较
此处的比较props.options !== innerOptions可能在引用不同而内容相同的场景下被误触发。可考虑使用深度比较或针对更新条件进行更精细的处理，以避免重复渲染。
127-166: 拆分级联和多列选择逻辑
handleSelect方法中针对级联和多列的处理逻辑相对冗长，可将其中的级联部分提取为独立函数，减少分支内的嵌套。这样有助于后续维护与测试，也提高代码可读性。src/packages/pickerview/pickerview.taro.tsx (3)
83-84: 考虑使用可选链优化循环条件
与非 Taro 版本类似，这里columnOptions && columnOptions.children建议改为可选链形式，以减少不必要的空值判断。- while (columnOptions && columnOptions.children) { + while (columnOptions?.children) { ... }🧰 Tools
🪛 Biome (1.9.4)
[error] 84-84: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
116-119: 避免对数组直接进行引用比较
此处props.options !== innerOptions的比较条件，在 React 中可能带来额外的渲染开销。考虑改为使用深度比较或其他特定比较方案解决。
128-166: 简化级联与多列逻辑
handleSelect方法容纳了两套逻辑，代码量较大，可将级联逻辑单独提取，以方便后续维护与单元测试。src/packages/pickerview/pickerroller.tsx (2)
60-65: 通过减少远距离元素渲染提升性能
isHidden方法仅对大于/小于当前索引 ±8 的条目返回true。在渲染内容较多时，可进一步优化判断，加快组件渲染效率，例如通过分段渲染或虚拟列表技术来避免过长的 DOM 节点。
82-91: 校验滚动边界时提升可读性
对updateMove的边界判断虽然逻辑正确，但可考虑通过函数封装或命名常量来提升可读性，比如minMove与maxMove的定义，让阅读逻辑更直观明了。src/packages/pickerview/pickerroller.taro.tsx (3)
29-39: 建议在 ref 与状态之间保持一致性，以便减少潜在的逻辑混淆。
目前对moving、lineSpacing等变量混合使用useRef和useState，需要检查它们在渲染周期里的更新频率和用法场景，以保证逻辑持续一致，避免意外的渲染或状态无法同步。
224-260: 每次渲染都重复添加与移除事件监听器，可能造成性能开销。
该useEffect没有指定依赖数组，组件每次重新渲染都会执行事件绑定与解绑；另外，使用{ once: true }会在首次触发后移除监听器，需确认是否满足滚动需求。若需多次滚动事件，建议去除once: true并在依赖数组中仅执行一次绑定。- const eventOptions = passiveSupported ? { passive: false, once: true } : false + const eventOptions = passiveSupported ? { passive: false } : false useEffect(() => { ... - }) + }, [])
195-200: 可在惯性滚动结束后集中处理滚动结果。
stopMomentum将moving设置为false并调用setChooseValue，逻辑合理。但若需额外处理收尾事件（如回调队列或埋点数据）可以在此处统一集中处理，减少各钩子之间的耦合。src/packages/pickerview/demos/h5/demo1.tsx (3)
5-17: 建议将列表数据提取为共享常量由于多个演示组件使用相同的数据结构，建议将
listData提取到一个共享的常量文件中以提高代码复用性。建议创建
src/packages/pickerview/demos/data.ts文件：export const cityList = [ { value: 1, label: '南京市' }, { value: 2, label: '无锡市' }, // ...其他城市 ] export const defaultDemo = { options: [[...cityList]], defaultValue: [1] }
25-27: 移除生产环境中的 console.log在生产环境中使用 console.log 不是最佳实践。建议使用 proper 的日志系统或移除此代码。
-onChange={({ value, selectedOptions }) => { - console.log('onChange', value, selectedOptions) -}} +onChange={({ value, selectedOptions }) => { + // 在这里处理选择变更 +}}
22-28: 增加无障碍属性为了提高组件的可访问性，建议添加 ARIA 属性。
<PickerView defaultValue={[1]} options={listData} + aria-label="城市选择器" + role="listbox" onChange={({ value, selectedOptions }) => { console.log('onChange', value, selectedOptions) }} />src/packages/pickerview/demos/taro/demo1.tsx (2)
5-17: 建议将列表数据提取为共享常量Taro 和 H5 版本使用相同的数据结构，建议提取共享数据以确保跨平台一致性。
参考上述 H5 版本的数据提取建议。
25-27: 优化 onChange 处理函数在 Taro 环境中，建议使用 Taro 的日志系统替代 console.log。
-onChange={({ value, selectedOptions }) => { - console.log('onChange', value, selectedOptions) -}} +onChange={({ value, selectedOptions }) => { + // 使用 Taro.showToast 或其他适合的反馈方式 +}}src/packages/pickerview/demos/h5/demo2.tsx (1)
23-23: 建议将样式配置移至主题文件内联样式建议移至主题配置文件中，以便统一管理和复用。
建议创建主题配置文件，将样式变量集中管理：
// src/packages/pickerview/styles/variables.ts export const pickerViewVariables = { itemHeight: '28px', // 其他样式变量 }src/packages/pickerview/demos/taro/demo2.tsx (2)
23-23: 建议使用主题变量而不是直接设置CSS变量建议使用NutUI的主题系统来设置picker项的高度，这样可以保持与其他组件的一致性。
- style={{ '--nutui-picker-item-height': '28px' }} + style={{ height: 'var(--nutui-picker-item-height, 28px)' }}
26-28: 建议增加错误处理onChange事件处理器目前只打印日志，建议添加错误处理并通知用户选择结果。
onChange={({ value, selectedOptions }) => { - console.log('onChange', value, selectedOptions) + try { + console.log('onChange', value, selectedOptions) + // 添加用户通知或状态更新 + } catch (error) { + console.error('选择出错:', error) + } }}src/packages/pickerview/demos/h5/demo5.tsx (1)
27-29: 建议增加事件处理的类型定义onChange回调函数的参数建议添加类型定义，以提高代码的可维护性。
- onChange={({ value, selectedOptions }) => { + onChange={({ value, selectedOptions }: { value: number[], selectedOptions: Array<{value: number, label: string}> }) => { console.log('onChange', value, selectedOptions) }}src/packages/pickerview/demos/h5/demo4.tsx (2)
29-31: 建议优化选择值变更的逻辑当选择值为3时直接设置为1的逻辑不够清晰，建议添加注释说明业务逻辑，并考虑使用更具描述性的条件判断。
- if (value[0] === 3) { - setValue([1]) - } + // 当选择"海北藏族自治区"时，自动切换到"南京市" + const AUTO_SWITCH_FROM_VALUE = 3 + const DEFAULT_FALLBACK_VALUE = 1 + if (value[0] === AUTO_SWITCH_FROM_VALUE) { + setValue([DEFAULT_FALLBACK_VALUE]) + }
19-19: 建议添加初始值的类型定义useState的初始值建议添加明确的类型定义，以提高代码的可维护性。
- const [value, setValue] = useState([2]) + const [value, setValue] = useState<number[]>([2])src/packages/pickerview/demos/taro/demo4.tsx (2)
19-19: 建议使用常量定义初始值避免在 useState 中使用魔法数字。建议将初始值定义为常量，提高代码可维护性。
+ const DEFAULT_CITY_VALUE = 2 - const [value, setValue] = useState([2]) + const [value, setValue] = useState([DEFAULT_CITY_VALUE])
27-32: 建议优化 onChange 处理逻辑
- 生产环境代码中不应该保留 console.log
- 建议将硬编码的条件值提取为常量
- 建议添加注释说明值变更的业务逻辑
+ const TRIGGER_VALUE = 3 + const RESET_VALUE = 1 onChange={({ value, selectedOptions }) => { - console.log('onChange', value, selectedOptions) - if (value[0] === 3) { - setValue([1]) + // 当选择海北藏族自治区时重置为南京市 + if (value[0] === TRIGGER_VALUE) { + setValue([RESET_VALUE]) } }}src/packages/pickerview/demos/h5/demo3.tsx (1)
28-33: 建议优化 onChange 回调函数
- 移除生产环境中的 console.log
- 建议将条件判断值提取为常量，增加代码可读性
- 建议添加注释说明此处的业务逻辑
+ const TRIGGER_VALUES = ['Tuesday', 'Afternoon'] + const RESET_VALUES = ['Monday', 'Evening'] onChange={({ value, selectedOptions }) => { - console.log('onChange', value, selectedOptions) - if (isEqual(value, ['Tuesday', 'Afternoon'])) { - setValue(['Monday', 'Evening']) + // 当选择周二下午时重置为周一晚上 + if (isEqual(value, TRIGGER_VALUES)) { + setValue(RESET_VALUES) } }}src/packages/pickerview/types.ts (3)
30-30: 建议优化 setRefs 的类型定义当前 setRefs 使用 any 类型，建议使用更具体的类型定义以提供更好的类型安全性。
- setRefs?: (ref: any) => any + setRefs?: (ref: HTMLElement | null) => void
5-11: 建议增强 PickerOptionItem 接口定义
- 建议添加 readonly 修饰符防止意外修改
- 建议为接口添加 JSDoc 注释说明用途
+ /** + * PickerView 选项项接口 + */ export interface PickerOptionItem { - label: string | number - value: string | number - children?: PickerOptionItem[] + readonly label: string | number + readonly value: string | number + readonly children?: readonly PickerOptionItem[] }
23-27: 建议完善回调参数接口的类型定义建议为回调参数接口添加详细的 JSDoc 注释，说明每个字段的用途。
+ /** + * PickerView onChange 回调参数接口 + * @property value - 当前选中的值数组 + * @property index - 当前改变的列索引 + * @property selectedOptions - 当前选中的选项数组 + */ export interface PickerOnChangeCallbackParameter { value: PickerValue[] index: number selectedOptions: PickerOptionItem[] }src/packages/pickerview/demo.tsx (1)
38-51: 建议优化 Demo 展示顺序当前的 Demo 展示顺序（基础用法 -> 受控 -> 自适应高度 -> 多列 -> 平铺 -> 级联）可以调整以提供更好的学习曲线。建议按照以下顺序排列：
- 基础用法
- 自适应高度
- 多列
- 级联
- 平铺
- 受控
这样的顺序从简单到复杂，更有利于用户理解组件的功能。
src/packages/pickerview/demos/h5/demo6.tsx (2)
5-64: 建议将数据结构抽离到单独的文件中当前的城市和区域数据直接定义在组件中，建议将其抽离到单独的数据文件中以提高代码的可维护性和复用性。
+// src/packages/pickerview/demos/data/cities.ts +export const citiesData = [ + [ + { + value: 1, + label: '北京', + children: [ + // ... existing children + ], + }, + // ... existing cities + ], +] -const listData = [...] +import { citiesData } from '../data/cities' +const listData = citiesData
72-74: 移除生产环境中的 console.log在生产环境中应该移除 console.log 语句。建议使用条件编译或开发工具来处理这个问题。
-onChange={({ value, selectedOptions }) => { - console.log('onChange', value, selectedOptions) -}} +onChange={({ value, selectedOptions }) => { + if (process.env.NODE_ENV === 'development') { + console.log('onChange', value, selectedOptions) + } +}}src/packages/pickerview/pickerview.scss (4)
1-9: 布局结构合理，但建议优化变量复用布局结构使用 flex 布局合理，但建议将重复使用的高度计算提取为变量以提高可维护性。
+$picker-list-height: calc($picker-item-height * 7); .nut-pickerview { --nutui-picker-item-height: 36px; position: relative; display: flex; width: 100%; - height: $picker-list-height; + height: $picker-list-height; $pickerview-top: calc(($picker-list-height - $picker-item-height) / 2); overflow: hidden;
10-27: 遮罩层实现合理，建议添加浏览器前缀遮罩层和指示器的定位和层级设置合理，但建议为 transform 属性添加浏览器前缀以提高兼容性。
&-mask { top: 0; bottom: 0; background-image: $picker-mask-background; background-position: top, bottom; background-size: 100% $pickerview-top; background-repeat: no-repeat; + -webkit-transform: translateZ(0); + -moz-transform: translateZ(0); transform: translateZ(0); }
46-65: 3D 效果实现完善，建议优化性能3D 转换和背面可见性的处理合理，但建议添加 will-change 属性以优化动画性能。
&-roller { position: absolute; top: $pickerview-top; width: 100%; height: $picker-item-height; z-index: 1; transform-style: preserve-3d; + will-change: transform; }
67-78: 文本样式处理合理，建议增加多语言支持文本溢出和对齐的处理合理，但建议考虑为不同语言的文本长度添加自适应处理。
&-roller-item, &-roller-item-title { width: 100%; height: $picker-item-height; line-height: $picker-item-height; color: $picker-item-text-color; font-size: $picker-item-text-font-size; text-align: center; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; + word-break: break-word; }src/packages/pickerview/doc.md (3)
1-60: 文档结构完整，建议补充更多使用场景文档包含了基础用法、受控模式等示例，建议补充更多实际应用场景的示例代码。
建议添加以下场景的示例：
- 自定义选项高亮样式
- 动态加载选项
- 异步更新数据
- 错误处理
61-80: API 文档完善，建议增加事件说明Props 的描述清晰，但建议补充事件触发时机和参数的详细说明。
建议在 onChange 事件中补充：
- 参数的详细类型定义
- 触发时机说明
- 返回值示例
81-94: 主题定制说明完整，建议补充设计指南CSS 变量的说明完整，建议补充设计规范和最佳实践指南。
建议添加：
- 设计规范说明
- 自定义主题示例
- 常见定制场景
src/config.json (1)
693-705: 配置完整，建议补充依赖说明组件配置包含了必要的元数据，但建议补充组件依赖关系说明。
建议在配置中补充：
{ "version": "3.0.0", "name": "PickerView", "type": "component", "cName": "选择器视图", "desc": "PickerView 是 Picker 的内容区域。", "sort": 15, "show": true, "taro": true, "v15": false, "dd": true, - "author": "songsong" + "author": "songsong", + "dependencies": ["Picker"] }src/styles/variables.scss (1)
562-582: 变量定义合理，建议补充暗黑模式支持变量定义和默认值设置合理，但建议补充暗黑模式下的变量值。
建议添加以下变量：
$picker-item-height: var(--nutui-picker-item-height, 36px) !default; $picker-list-height: calc($picker-item-height * 7) !default; +$picker-item-text-color-dark: var(--nutui-picker-item-text-color-dark, $color-text-dark) !default; +$picker-item-active-line-border-dark: var(--nutui-picker-item-active-line-border-dark, 1px solid $color-border-dark) !default; +$picker-mask-background-dark: var(--picker-mask-background-dark, linear-gradient(180deg, var(--nutui-black-12), var(--nutui-black-7))) !default;src/packages/pickerview/doc.taro.md (1)
67-72: Props 文档需要补充说明建议为以下属性添加更详细的说明：
options的具体数据结构示例value和selectedOptions的具体数据格式onChange回调函数的参数类型定义src/packages/pickerview/doc.zh-TW.md (1)
89-93: 部分繁體中文術語翻譯建議優化以下術語建議調整：
- "面闆" 應改為 "面板"
- "數據" 可考慮改為 "資料"
- "字色" 建議改為 "文字顏色"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
src/config.json(1 hunks)src/packages/pickerview/__test__/pickerview.spec.tsx(1 hunks)src/packages/pickerview/demo.taro.tsx(1 hunks)src/packages/pickerview/demo.tsx(1 hunks)src/packages/pickerview/demos/h5/demo1.tsx(1 hunks)src/packages/pickerview/demos/h5/demo2.tsx(1 hunks)src/packages/pickerview/demos/h5/demo3.tsx(1 hunks)src/packages/pickerview/demos/h5/demo4.tsx(1 hunks)src/packages/pickerview/demos/h5/demo5.tsx(1 hunks)src/packages/pickerview/demos/h5/demo6.tsx(1 hunks)src/packages/pickerview/demos/taro/demo1.tsx(1 hunks)src/packages/pickerview/demos/taro/demo2.tsx(1 hunks)src/packages/pickerview/demos/taro/demo3.tsx(1 hunks)src/packages/pickerview/demos/taro/demo4.tsx(1 hunks)src/packages/pickerview/demos/taro/demo5.tsx(1 hunks)src/packages/pickerview/demos/taro/demo6.tsx(1 hunks)src/packages/pickerview/doc.en-US.md(1 hunks)src/packages/pickerview/doc.md(1 hunks)src/packages/pickerview/doc.taro.md(1 hunks)src/packages/pickerview/doc.zh-TW.md(1 hunks)src/packages/pickerview/index.taro.ts(1 hunks)src/packages/pickerview/index.ts(1 hunks)src/packages/pickerview/pickerroller.taro.tsx(1 hunks)src/packages/pickerview/pickerroller.tsx(1 hunks)src/packages/pickerview/pickerview.scss(1 hunks)src/packages/pickerview/pickerview.taro.tsx(1 hunks)src/packages/pickerview/pickerview.tsx(1 hunks)src/packages/pickerview/types.ts(1 hunks)src/styles/variables.scss(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/packages/pickerview/doc.en-US.md
🧰 Additional context used
🪛 GitHub Actions: CI
src/packages/pickerview/__test__/pickerview.spec.tsx
[error] 61-61: TypeError: undefined is not iterable (cannot read property Symbol(Symbol.iterator)).
🪛 Biome (1.9.4)
src/packages/pickerview/pickerview.tsx
[error] 83-83: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/pickerview/pickerview.taro.tsx
[error] 84-84: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 markdownlint-cli2 (0.17.2)
src/packages/pickerview/doc.taro.md
85-85: Link fragments should be valid
null
(MD051, link-fragments)
src/packages/pickerview/doc.zh-TW.md
85-85: Link fragments should be valid
null
(MD051, link-fragments)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (4)
src/packages/pickerview/pickerroller.tsx (1)
212-216: 在修改选中值后，补充边界保护
modifyStatus(false)内部如果传入的props.value未匹配到任意options，会将索引重置为 1，可能需要额外处理当options本身为空或传入值不在其索引范围时的场景。src/packages/pickerview/index.ts (1)
1-10: 导出文件结构清晰，类型和主组件分离可读性好。
当前的导出方式有助于用户在使用时快速定位PickerView及相关类型，整体实现合理。src/packages/pickerview/index.taro.ts (1)
1-10: 与标准版保持一致的导出结构，方便跨平台使用。
这种做法与普通index.ts相同，让 Taro 版本的组件保持一致的使用方式，很恰当。src/packages/pickerview/demo.taro.tsx (1)
43-56: 建议优化 Demo 展示顺序与 H5 版本相同，建议调整 Demo 的展示顺序以提供更好的学习曲线，从基础功能逐步过渡到高级特性。
| const [firstColumn] = props.options as PickerOptions[] | ||
| if (Array.isArray(firstColumn) && 'children' in firstColumn[0]) { | ||
| return 'cascade' | ||
| } | ||
| return 'multiple' | ||
| }, [props.options]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
在判定级联类型时增加空数组保护逻辑
如果外部传递的 options 为空数组，firstColumn[0] 会抛出错误，建议在访问之前增加数组为空的判断或使用可选链，提升健壮性。
| import React from 'react' | ||
| import { PickerView, Cell } from '@nutui/nutui-react' | ||
|
|
||
| const Demo1 = () => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
组件命名需要修正
文件名为 demo2.tsx 但组件名为 Demo1，这可能导致混淆。
-const Demo1 = () => {
+const Demo2 = () => {📝 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.
| const Demo1 = () => { | |
| const Demo2 = () => { |
|
|
||
| ### 样式变量 | ||
|
|
||
| 组件提供了下列 CSS 变量，可用于自定义样式，使用方法请参考 [ConfigProvider 组件](#/zh-CN/component/configprovider)。 |
There was a problem hiding this comment.
ConfigProvider 组件链接需要更新
链接片段 #/zh-CN/component/configprovider 格式不正确，这可能导致导航失败。
建议修改为：
-[ConfigProvider 组件](#/zh-CN/component/configprovider)
+[ConfigProvider 组件](/zh-CN/component/configprovider)📝 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.
| 组件提供了下列 CSS 变量，可用于自定义样式，使用方法请参考 [ConfigProvider 组件](#/zh-CN/component/configprovider)。 | |
| 组件提供了下列 CSS 变量，可用于自定义样式，使用方法请参考 [ConfigProvider 组件](/zh-CN/component/configprovider)。 |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
85-85: Link fragments should be valid
null
(MD051, link-fragments)
|
|
||
| ### 樣式變量 | ||
|
|
||
| 組件提供了下列 CSS 變量，可用於自定義樣式，使用方法請參考 [ConfigProvider 組件](#/zh-CN/component/configprovider)。 |
There was a problem hiding this comment.
ConfigProvider 組件鏈接需要更新
鏈接片段 #/zh-CN/component/configprovider 格式不正確，這可能導致導航失敗。
建議修改為：
-[ConfigProvider 組件](#/zh-CN/component/configprovider)
+[ConfigProvider 組件](/zh-CN/component/configprovider)📝 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.
| 組件提供了下列 CSS 變量，可用於自定義樣式，使用方法請參考 [ConfigProvider 組件](#/zh-CN/component/configprovider)。 | |
| 組件提供了下列 CSS 變量，可用於自定義樣式，使用方法請參考 [ConfigProvider 組件](/zh-CN/component/configprovider)。 |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
85-85: Link fragments should be valid
null
(MD051, link-fragments)
github-actions
bot
added
action:review
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #2986 +/- ##
=============================================
+ Coverage 85.75% 85.79% +0.03%
=============================================
Files 277 281 +4
Lines 18110 18527 +417
Branches 2735 2810 +75
=============================================
+ Hits 15531 15896 +365
- Misses 2574 2626 +52
Partials 5 5 ☔ View full report in Codecov by Sentry. |
github-actions
bot
added
action:review
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/packages/pickerview/pickerroller.tsx (1)
223-233:⚠️ Potential issue建议修复事件监听器的内存泄漏问题
当前的事件监听器实现存在以下问题：
- useEffect 缺少必要的依赖项
- 可能导致重复绑定事件
- 清理函数可能在组件卸载时无法正确执行
useEffect(() => { const options = passiveSupported ? { passive: false } : false + const currentRef = pickerRollerRef.current + - pickerRollerRef.current?.addEventListener('touchstart', touchStart, options) - pickerRollerRef.current?.addEventListener('touchmove', touchMove, options) - pickerRollerRef.current?.addEventListener('touchend', touchEnd, options) + if (currentRef) { + currentRef.addEventListener('touchstart', touchStart, options) + currentRef.addEventListener('touchmove', touchMove, options) + currentRef.addEventListener('touchend', touchEnd, options) + } + return () => { - pickerRollerRef.current?.removeEventListener('touchstart', touchStart) - pickerRollerRef.current?.removeEventListener('touchmove', touchMove) - pickerRollerRef.current?.removeEventListener('touchend', touchEnd) + if (currentRef) { + currentRef.removeEventListener('touchstart', touchStart) + currentRef.removeEventListener('touchmove', touchMove) + currentRef.removeEventListener('touchend', touchEnd) + } } -}, []) +}, [touchStart, touchMove, touchEnd])src/packages/pickerview/pickerroller.taro.tsx (1)
92-94:⚠️ Potential issue需要处理空数组边界情况
当
options.length为 0 时，计算边界值可能会产生意外结果。建议添加空数组检查。+ if (options.length === 0) { + updateMove = 0; + return; + } if (updateMove < -(options.length - 1) * lineSpacing.current) { updateMove = -(options.length - 1) * lineSpacing.current }
🧹 Nitpick comments (12)
src/packages/pickerview/__test__/pickerview.spec.tsx (2)
6-18: 测试数据建议使用更具代表性的示例当前测试数据中的城市列表过于简单，建议：
- 添加不同长度的城市名称
- 包含特殊字符的城市名称
- 添加空值和边界值测试用例
const listData = [ [ + { value: '', label: '' }, // 空值测试 { value: 1, label: '南京市' }, { value: 2, label: '无锡市' }, { value: 3, label: '海北藏族自治区' }, + { value: 4, label: '北京市(首都)' }, // 特殊字符 - { value: 4, label: '北京市' }, { value: 5, label: '连云港市' }, { value: 8, label: '大庆市' }, { value: 9, label: '绥化市' }, { value: 10, label: '潍坊市' }, { value: 12, label: '乌鲁木齐市' }, + { value: null, label: '测试市' }, // null值测试 ], ]
27-36: 建议完善平铺模式的测试用例当前平铺模式的测试用例仅验证了渲染，建议增加：
- 样式验证
- 布局验证
- 交互验证
src/packages/pickerview/pickerview.tsx (2)
132-171: 建议优化 handleSelect 函数的复杂度当前 handleSelect 函数逻辑较为复杂，建议：
- 拆分为多个子函数
- 添加详细的注释
- 增加错误处理
const handleSelect = useCallback( (option: PickerOptionItem, index: number) => { const newValue = option?.value if (!newValue || innerValue[index] === newValue) return changeIndex.current = index + + // 处理多列选择模式 if (columnsType === 'multiple') { - setInnerValue((prev) => { - const next = [...prev] - next[index] = newValue - return next - }) + handleMultipleSelect(newValue, index) } else { - const startIndex = index - const values: PickerValue[] = [] - values[index] = option.value - while (option?.children?.[0]) { - values[index + 1] = option.children[0].value - index++ - option = option.children[0] - } - if (option?.children?.length) { - values[index + 1] = '' - } - const combineResult = [ - ...innerValue.slice(0, startIndex), - ...values.splice(startIndex), - ] - setInnerValue([...combineResult]) + handleCascadeSelect(option, index) } }, [innerValue, props.options, columnsType, innerOptions] ) + // 处理多列选择 + const handleMultipleSelect = (newValue: PickerValue, index: number) => { + setInnerValue((prev) => { + const next = [...prev] + next[index] = newValue + return next + }) + } + + // 处理级联选择 + const handleCascadeSelect = (option: PickerOptionItem, index: number) => { + try { + const startIndex = index + const values: PickerValue[] = [] + values[index] = option.value + + // 递归获取子选项的值 + while (option?.children?.[0]) { + values[index + 1] = option.children[0].value + index++ + option = option.children[0] + } + + // 处理空子选项 + if (option?.children?.length) { + values[index + 1] = '' + } + + const combineResult = [ + ...innerValue.slice(0, startIndex), + ...values.splice(startIndex), + ] + setInnerValue([...combineResult]) + } catch (error) { + console.error('级联选择处理失败:', error) + } + }
187-193: 建议优化 onChange 回调的依赖项当前 useEffect 的依赖项可能导致不必要的重渲染，建议：
- 使用 useCallback 包装 onChange 回调
- 考虑使用 useMemo 缓存回调参数
+const callbackParams = useMemo(() => ({ + value: innerValue, + index: changeIndex.current, + selectedOptions, +}), [innerValue, selectedOptions]) + useEffect(() => { - onChange?.({ - value: innerValue, - index: changeIndex.current, - selectedOptions, - }) + onChange?.(callbackParams) }, [innerValue, selectedOptions, onChange])src/packages/pickerview/pickerview.taro.tsx (1)
195-213: 建议增加 Taro 平台特定的处理当前实现与 React 版本过于相似，建议：
- 添加 Taro 平台特有的性能优化
- 处理小程序的触摸事件差异
- 考虑小程序的渲染限制
return ( <View className={cls} style={style}> + {/* 使用 Taro 的虚拟列表优化性能 */} + <VirtualList + height={300} + itemData={innerOptions} + itemCount={innerOptions.length} + itemSize={44} + > - {innerOptions.map((item, index) => ( <PickerRoller ref={props?.setRefs?.(index)} key={index} keyIndex={index} value={innerValue[index] as PickerValue} options={item} renderLabel={renderLabel} onSelect={handleSelect} duration={duration} threeDimensional={threeDimensional} /> - ))} + </VirtualList> <View className="nut-pickerview-mask" /> <View className="nut-pickerview-indicator" /> </View> )src/packages/pickerview/pickerroller.tsx (1)
130-138: 建议优化触摸事件处理逻辑当前的触摸事件处理逻辑可以改进：
- 添加防抖处理
- 优化性能
- 增加错误处理
+const debouncedMove = useCallback( + debounce((move: number) => { + setMove(move) + }, 16), + [] +) + const touchMove = (event: React.TouchEvent<HTMLDivElement>) => { + try { touch.move(event) if ((touch as any).isVertical) { moving.current = true preventDefault(event, true) } const move = touch.deltaY.current - startY - setMove(move) + debouncedMove(move) + } catch (error) { + console.error('触摸事件处理失败:', error) + } }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 131-138: src/packages/pickerview/pickerroller.tsx#L131-L138
Added lines #L131 - L138 were not covered by testssrc/packages/pickerview/pickerroller.taro.tsx (3)
20-20: 建议为 options 添加更具体的类型定义当前
options = []的类型推断过于宽泛。建议明确指定类型以提高代码的可维护性。- options = [], + options: PickerOptionItem[] = [],
170-193: 建议优化值查找逻辑以提高性能当前实现中存在多次数组遍历，可以通过单次遍历优化性能。
const modifyStatus = (type?: boolean, val?: string | number) => { const value = val || props.value - let index = -1 - if (value) { - options.some((item, idx) => { - if (item.value === value) { - index = idx - return true - } - return false - }) - } else { - options.forEach((item, i) => { - if (item.value === props.value) { - index = i - } - }) - } + const index = options.findIndex(item => + item.value === (value || props.value) + ) setCurrIndex(index === -1 ? 1 : index + 1) const move = index * lineSpacing.current type && setChooseValue(-move) setMove(-move) }
224-260: 建议优化事件监听器的清理逻辑可以将事件监听器的添加和移除逻辑抽象为一个辅助函数，以减少代码重复并提高可维护性。
+ const setupEventListener = ( + element: HTMLElement, + eventName: string, + handler: (event: TouchEvent) => void, + options: boolean | AddEventListenerOptions + ) => { + element?.addEventListener(eventName, handler, options) + return () => element?.removeEventListener(eventName, handler, options) + } useEffect(() => { const eventOptions = passiveSupported ? { passive: false, once: true } : false - pickerRollerRef.current?.addEventListener('touchstart', touchStart, eventOptions) - pickerRollerRef.current?.addEventListener('touchmove', touchMove, eventOptions) - pickerRollerRef.current?.addEventListener('touchend', touchEnd, eventOptions) + const cleanupFns = [ + setupEventListener(pickerRollerRef.current, 'touchstart', touchStart, eventOptions), + setupEventListener(pickerRollerRef.current, 'touchmove', touchMove, eventOptions), + setupEventListener(pickerRollerRef.current, 'touchend', touchEnd, eventOptions) + ] return () => { - pickerRollerRef.current?.removeEventListener('touchstart', touchStart, eventOptions) - pickerRollerRef.current?.removeEventListener('touchmove', touchMove, eventOptions) - pickerRollerRef.current?.removeEventListener('touchend', touchEnd, eventOptions) + cleanupFns.forEach(cleanup => cleanup()) } }, [])src/packages/pickerview/doc.md (1)
85-85: ConfigProvider 组件链接需要更新当前链接格式不正确，可能导致导航失败。
-[ConfigProvider 组件](#/zh-CN/component/configprovider) +[ConfigProvider 组件](/zh-CN/component/configprovider)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
85-85: Link fragments should be valid
null(MD051, link-fragments)
src/packages/pickerview/doc.taro.md (1)
67-72: 建议完善 Props 类型说明为了提高文档的准确性和可用性，建议补充以下内容：
options的具体类型定义value和defaultValue数组元素的类型说明onChange回调参数的详细类型定义建议修改为：
-| options | 列表数据 | `Array` | `[]` | +| options | 列表数据 | `Array<{label: string \| number, value: string \| number, children?: Array}>` | `[]` | -| value | 选中值，受控 | `Array` | `[]` | +| value | 选中值，受控 | `Array<string \| number>` | `[]` | -| defaultValue | 默认选中 | `Array` | `[]` | +| defaultValue | 默认选中 | `Array<string \| number>` | `[]` |src/packages/pickerview/doc.zh-TW.md (1)
67-72: 建議完善 Props 類型說明為了提高文檔的準確性和可用性，建議補充以下內容：
options的具體類型定義value和defaultValue數組元素的類型說明onChange回調參數的詳細類型定義建議修改為：
-| options | 列錶數據 | `Array` | `[]` | +| options | 列錶數據 | `Array<{label: string \| number, value: string \| number, children?: Array}>` | `[]` | -| value | 選中值，受控 | `Array` | `[]` | +| value | 選中值，受控 | `Array<string \| number>` | `[]` | -| defaultValue | 默認選中 | `Array` | `[]` | +| defaultValue | 默認選中 | `Array<string \| number>` | `[]` |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/packages/pickerview/__test__/__snapshots__/pickerview.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
src/packages/pickerview/__test__/pickerview.spec.tsx(1 hunks)src/packages/pickerview/demo.taro.tsx(1 hunks)src/packages/pickerview/doc.md(1 hunks)src/packages/pickerview/doc.taro.md(1 hunks)src/packages/pickerview/doc.zh-TW.md(1 hunks)src/packages/pickerview/pickerroller.taro.tsx(1 hunks)src/packages/pickerview/pickerroller.tsx(1 hunks)src/packages/pickerview/pickerview.taro.tsx(1 hunks)src/packages/pickerview/pickerview.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/pickerview/demo.taro.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/pickerview/pickerview.tsx
[error] 87-87: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/pickerview/pickerview.taro.tsx
[error] 88-88: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Check: codecov/patch
src/packages/pickerview/pickerview.tsx
[warning] 67-68: src/packages/pickerview/pickerview.tsx#L67-L68
Added lines #L67 - L68 were not covered by tests
[warning] 73-76: src/packages/pickerview/pickerview.tsx#L73-L76
Added lines #L73 - L76 were not covered by tests
[warning] 78-83: src/packages/pickerview/pickerview.tsx#L78-L83
Added lines #L78 - L83 were not covered by tests
[warning] 85-88: src/packages/pickerview/pickerview.tsx#L85-L88
Added lines #L85 - L88 were not covered by tests
[warning] 90-91: src/packages/pickerview/pickerview.tsx#L90-L91
Added lines #L90 - L91 were not covered by tests
[warning] 93-94: src/packages/pickerview/pickerview.tsx#L93-L94
Added lines #L93 - L94 were not covered by tests
[warning] 96-102: src/packages/pickerview/pickerview.tsx#L96-L102
Added lines #L96 - L102 were not covered by tests
[warning] 104-107: src/packages/pickerview/pickerview.tsx#L104-L107
Added lines #L104 - L107 were not covered by tests
[warning] 111-115: src/packages/pickerview/pickerview.tsx#L111-L115
Added lines #L111 - L115 were not covered by tests
src/packages/pickerview/pickerroller.tsx
[warning] 23-24: src/packages/pickerview/pickerroller.tsx#L23-L24
Added lines #L23 - L24 were not covered by tests
[warning] 68-80: src/packages/pickerview/pickerroller.tsx#L68-L80
Added lines #L68 - L80 were not covered by tests
[warning] 86-91: src/packages/pickerview/pickerroller.tsx#L86-L91
Added lines #L86 - L91 were not covered by tests
[warning] 94-98: src/packages/pickerview/pickerroller.tsx#L94-L98
Added lines #L94 - L98 were not covered by tests
[warning] 100-101: src/packages/pickerview/pickerroller.tsx#L100-L101
Added lines #L100 - L101 were not covered by tests
[warning] 112-114: src/packages/pickerview/pickerroller.tsx#L112-L114
Added lines #L112 - L114 were not covered by tests
[warning] 119-120: src/packages/pickerview/pickerroller.tsx#L119-L120
Added lines #L119 - L120 were not covered by tests
[warning] 124-128: src/packages/pickerview/pickerroller.tsx#L124-L128
Added lines #L124 - L128 were not covered by tests
[warning] 131-138: src/packages/pickerview/pickerroller.tsx#L131-L138
Added lines #L131 - L138 were not covered by tests
[warning] 141-143: src/packages/pickerview/pickerroller.tsx#L141-L143
Added lines #L141 - L143 were not covered by tests
[warning] 145-145: src/packages/pickerview/pickerroller.tsx#L145
Added line #L145 was not covered by tests
[warning] 147-155: src/packages/pickerview/pickerroller.tsx#L147-L155
Added lines #L147 - L155 were not covered by tests
[warning] 159-159: src/packages/pickerview/pickerroller.tsx#L159
Added line #L159 was not covered by tests
[warning] 161-161: src/packages/pickerview/pickerroller.tsx#L161
Added line #L161 was not covered by tests
[warning] 163-165: src/packages/pickerview/pickerroller.tsx#L163-L165
Added lines #L163 - L165 were not covered by tests
[warning] 176-176: src/packages/pickerview/pickerroller.tsx#L176
Added line #L176 was not covered by tests
[warning] 179-184: src/packages/pickerview/pickerroller.tsx#L179-L184
Added lines #L179 - L184 were not covered by tests
[warning] 194-197: src/packages/pickerview/pickerroller.tsx#L194-L197
Added lines #L194 - L197 were not covered by tests
[warning] 200-205: src/packages/pickerview/pickerroller.tsx#L200-L205
Added lines #L200 - L205 were not covered by tests
[warning] 207-210: src/packages/pickerview/pickerroller.tsx#L207-L210
Added lines #L207 - L210 were not covered by tests
[warning] 219-220: src/packages/pickerview/pickerroller.tsx#L219-L220
Added lines #L219 - L220 were not covered by tests
🪛 markdownlint-cli2 (0.17.2)
src/packages/pickerview/doc.taro.md
85-85: Link fragments should be valid
null
(MD051, link-fragments)
src/packages/pickerview/doc.md
85-85: Link fragments should be valid
null
(MD051, link-fragments)
src/packages/pickerview/doc.zh-TW.md
85-85: Link fragments should be valid
null
(MD051, link-fragments)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (2)
src/packages/pickerview/doc.taro.md (1)
85-85: ConfigProvider 组件链接需要更新链接格式不正确，建议修改为相对路径格式。
-[ConfigProvider 组件](#/zh-CN/component/configprovider) +[ConfigProvider 组件](/zh-CN/component/configprovider)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
85-85: Link fragments should be valid
null(MD051, link-fragments)
src/packages/pickerview/doc.zh-TW.md (1)
85-85: ConfigProvider 組件鏈接需要更新鏈接格式不正確，建議修改為相對路徑格式。
-[ConfigProvider 組件](#/zh-CN/component/configprovider) +[ConfigProvider 組件](/zh-CN/component/configprovider)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
85-85: Link fragments should be valid
null(MD051, link-fragments)
| test('should match snapshot', () => { | ||
| const { container } = render( | ||
| <PickerView defaultValue={[1]} options={listData} /> | ||
| ) | ||
| expect(container).toMatchSnapshot() | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议增加更多测试场景
当前仅有快照测试，建议添加以下测试场景：
- 选项切换时的回调测试
- 默认值渲染测试
- 错误处理测试
- 用户交互测试
describe('PickerView', () => {
it('应该正确触发 onChange 事件', () => {
const onChange = jest.fn()
const { container } = render(
<PickerView
defaultValue={[1]}
options={listData}
onChange={onChange}
/>
)
// 模拟选择操作
// 验证回调是否被正确触发
})
it('应该正确处理无效的默认值', () => {
const { container } = render(
<PickerView
defaultValue={[999]} // 无效值
options={listData}
/>
)
// 验证是否回退到第一个选项
})
})
github-actions
bot
added
action:review
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/packages/pickerview/pickerroller.taro.tsx (2)
85-119:⚠️ Potential issue建议增加边界处理和简化计算逻辑
当前代码在处理空选项时可能出现问题，且计算逻辑可以简化：
const setMove = (move: number, type?: string, time?: number) => { + if (options.length === 0) { + return + } let updateMove = move + transformY.current if (type === 'end') { // 限定滚动距离 if (updateMove > 0) { updateMove = 0 } - if (updateMove < -(options.length - 1) * lineSpacing.current) { - updateMove = -(options.length - 1) * lineSpacing.current - } + const maxMove = -(options.length - 1) * lineSpacing.current + updateMove = Math.max(updateMove, maxMove) // 设置滚动距离为lineSpacing.current的倍数值 - const endMove = - Math.round(updateMove / lineSpacing.current) * lineSpacing.current + const endMove = Math.round(updateMove / lineSpacing.current) * lineSpacing.current
121-123:⚠️ Potential issue建议增加空值处理
在设置选中值时需要处理空值情况：
const setChooseValue = (move: number) => { + if (options.length === 0) return + const selectedIndex = Math.round(-move / lineSpacing.current) + if (selectedIndex < 0 || selectedIndex >= options.length) return - onSelect?.(options?.[Math.round(-move / lineSpacing.current)], keyIndex) + onSelect?.(options[selectedIndex], keyIndex) }
🧹 Nitpick comments (9)
src/packages/pickerview/pickerroller.tsx (5)
27-47: 建议优化状态管理和常量定义
- 建议将相关状态组合成对象，减少 useState 的使用次数
- 建议将魔法数字（如 36、20）提取为命名常量，提高代码可维护性
+const CONSTANTS = { + LINE_SPACING: 36, + ROTATION_ANGLE: 20, + DEFAULT_DURATION: 200, + INERTIA_TIME: 300, + INERTIA_DISTANCE: 15, +} as const + +interface TouchState { + time: number; + deg: string; + distance: number; +} + -const lineSpacing = useRef(36) -const [touchTime, setTouchTime] = useState(0) -const [touchDeg, setTouchDeg] = useState('0deg') -const rotation = 20 +const lineSpacing = useRef(CONSTANTS.LINE_SPACING) +const [touchState, setTouchState] = useState<TouchState>({ + time: 0, + deg: '0deg', + distance: 0 +}) +const rotation = CONSTANTS.ROTATION_ANGLE
122-156: 触摸事件处理逻辑缺少测试覆盖触摸事件处理是组件的核心功能，但目前缺少测试覆盖。建议添加以下测试场景：
- 基本的触摸事件（开始、移动、结束）
- 惯性滚动触发条件
- 边界条件处理
需要我帮您生成这些测试用例的实现代码吗？
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 124-128: src/packages/pickerview/pickerroller.tsx#L124-L128
Added lines #L124 - L128 were not covered by tests
[warning] 131-138: src/packages/pickerview/pickerroller.tsx#L131-L138
Added lines #L131 - L138 were not covered by tests
[warning] 141-143: src/packages/pickerview/pickerroller.tsx#L141-L143
Added lines #L141 - L143 were not covered by tests
[warning] 145-145: src/packages/pickerview/pickerroller.tsx#L145
Added line #L145 was not covered by tests
[warning] 147-155: src/packages/pickerview/pickerroller.tsx#L147-L155
Added lines #L147 - L155 were not covered by tests
157-165: 建议优化惯性滚动计算逻辑
- 建议将惯性系数 0.003 提取为可配置参数
- 建议添加注释说明惯性计算的物理原理
- 考虑添加减速度因子，使滚动更自然
+interface MomentumConfig { + coefficient: number; + deceleration: number; +} + +const DEFAULT_MOMENTUM_CONFIG: MomentumConfig = { + coefficient: 0.003, + deceleration: 0.98, +}; + -const momentum = (distance: number, duration: number) => { +// 基于物理模型计算惯性滚动距离 +// distance: 滑动距离 +// duration: 滑动时间 +// config: 惯性参数配置 +const momentum = ( + distance: number, + duration: number, + config: MomentumConfig = DEFAULT_MOMENTUM_CONFIG +) => { let nDistance = distance const speed = Math.abs(nDistance / duration) - nDistance = (speed / 0.003) * (nDistance < 0 ? -1 : 1) + nDistance = (speed / config.coefficient) * + (nDistance < 0 ? -1 : 1) * + config.deceleration return nDistance }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 159-159: src/packages/pickerview/pickerroller.tsx#L159
Added line #L159 was not covered by tests
[warning] 161-161: src/packages/pickerview/pickerroller.tsx#L161
Added line #L161 was not covered by tests
[warning] 163-165: src/packages/pickerview/pickerroller.tsx#L163-L165
Added lines #L163 - L165 were not covered by tests
167-190: 建议简化值更新逻辑当前值查找和更新逻辑较为复杂，建议重构以提高可读性：
const modifyStatus = (type?: boolean, val?: string | number) => { const value = val || props.value - let index = -1 - if (value) { - options.some((item, idx) => { - if (item.value === value) { - index = idx - return true - } - return false - }) - } else { - options.forEach((item, i) => { - if (item.value === props.value) { - index = i - } - }) - } + const index = options.findIndex( + (item) => item.value === (value || props.value) + ) + const normalizedIndex = index === -1 ? 1 : index + 1 - setCurrIndex(index === -1 ? 1 : index + 1) + setCurrIndex(normalizedIndex) const move = index * lineSpacing.current type && setChooseValue(-move) setMove(-move) }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 176-176: src/packages/pickerview/pickerroller.tsx#L176
Added line #L176 was not covered by tests
[warning] 179-184: src/packages/pickerview/pickerroller.tsx#L179-L184
Added lines #L179 - L184 were not covered by tests
256-292: 建议优化渲染性能
- 建议使用 useMemo 缓存样式计算结果：
+const memoizedRollerStyle = useMemo( + () => touchRollerStyle(), + [touchTime, touchDeg] +) + +const memoizedTileStyle = useMemo( + () => touchTileStyle(), + [touchTime, scrollDistance] +) return ( <div className="nut-pickerview-list" ref={pickerRollerRef}> <div className="nut-pickerview-roller" ref={rollerRef} - style={threeDimensional ? touchRollerStyle() : touchTileStyle()} + style={threeDimensional ? memoizedRollerStyle : memoizedTileStyle}
- 建议使用 React.memo 优化选项渲染：
+const PickerItem = React.memo( + ({ item, index, isHidden, rollerStyle, renderLabel }) => ( + <div + className={`nut-pickerview-roller-item ${ + isHidden(index + 1) && 'nut-pickerview-roller-item-hidden' + }`} + style={rollerStyle(index)} + key={item.value ?? index} + > + {renderLabel(item)} + </div> + ) +)src/packages/pickerview/pickerroller.taro.tsx (4)
14-28: 建议加强类型定义为了提高代码的类型安全性，建议：
- 为
keyIndex添加更具体的类型约束- 为
duration添加最小值限制- 为
renderLabel的返回值添加明确的类型注解const InternalPickerRoller: ForwardRefRenderFunction< { stopMomentum: () => void; moving: boolean }, Partial<PickerRollerProps> > = (props, ref) => { const { - keyIndex = 0, + keyIndex = 0 as const, - duration = 1000, + duration = Math.max(1000, 0), options = [], threeDimensional = true, - renderLabel = (item: PickerOptionItem) => { + renderLabel = (item: PickerOptionItem): string => { return item.label }, } = props
29-49: 建议优化状态管理当前状态变量较为分散，建议：
- 使用 reducer 统一管理相关状态
- 将魔法数字提取为命名常量
- 考虑使用 TypeScript 的 const assertions
+ const ROTATION_ANGLE = 20 + const DEFAULT_CURR_INDEX = 1 + const LINE_SPACING = 36 + const touchState = { + time: 0, + deg: '0deg', + distance: 0, + } - const [currIndex, setCurrIndex] = useState(1) - const lineSpacing = useRef(36) - const [touchTime, setTouchTime] = useState(0) - const [touchDeg, setTouchDeg] = useState('0deg') - const rotation = 20 + const [currIndex, setCurrIndex] = useState(DEFAULT_CURR_INDEX) + const lineSpacing = useRef(LINE_SPACING) + const rotation = ROTATION_ANGLE as const
224-260: 建议优化事件处理当前事件处理的实现可以通过以下方式优化：
- 使用 useCallback 缓存事件处理函数
- 优化 useEffect 的依赖数组
- 提取事件选项配置
+ const eventOptions = useMemo( + () => (passiveSupported ? { passive: false, once: true } : false), + [] + ) + const handleTouchStart = useCallback((event: React.TouchEvent<HTMLDivElement>) => { + touchStart(event) + }, [touchStart]) + const handleTouchMove = useCallback((event: React.TouchEvent<HTMLDivElement>) => { + touchMove(event) + }, [touchMove]) + const handleTouchEnd = useCallback(() => { + touchEnd() + }, [touchEnd]) useEffect(() => { - const eventOptions = passiveSupported - ? { passive: false, once: true } - : false pickerRollerRef.current?.addEventListener( 'touchstart', - touchStart, + handleTouchStart, eventOptions ) // ... 其他事件监听器 - }, [pickerRollerRef.current, touchStart, touchMove, touchEnd]) + }, [handleTouchStart, handleTouchMove, handleTouchEnd, eventOptions])
262-315: 建议优化样式计算和渲染性能当前样式计算和渲染逻辑可以通过以下方式优化：
- 使用 useMemo 缓存样式计算
- 优化 key 的生成
- 提取样式常量
+ const touchRollerStyle = useMemo(() => ({ + transition: `transform ${touchTime}ms cubic-bezier(0.17, 0.89, 0.45, 1)`, + transform: `rotate3d(1, 0, 0, ${touchDeg})`, + }), [touchTime, touchDeg]) + const getItemKey = useCallback((item: PickerOptionItem, index: number) => ( + item.value?.toString() ?? `index-${index}` + ), []) {threeDimensional && options.map((item, index) => ( <View className={`nut-pickerview-roller-item ${ isHidden(index + 1) && 'nut-pickerview-roller-item-hidden' }`} style={rollerStyle(index)} - key={item.value ?? index} + key={getItemKey(item, index)} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/packages/pickerview/pickerroller.taro.tsx(1 hunks)src/packages/pickerview/pickerroller.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/pickerview/pickerroller.tsx
[warning] 23-24: src/packages/pickerview/pickerroller.tsx#L23-L24
Added lines #L23 - L24 were not covered by tests
[warning] 68-80: src/packages/pickerview/pickerroller.tsx#L68-L80
Added lines #L68 - L80 were not covered by tests
[warning] 86-91: src/packages/pickerview/pickerroller.tsx#L86-L91
Added lines #L86 - L91 were not covered by tests
[warning] 94-98: src/packages/pickerview/pickerroller.tsx#L94-L98
Added lines #L94 - L98 were not covered by tests
[warning] 100-101: src/packages/pickerview/pickerroller.tsx#L100-L101
Added lines #L100 - L101 were not covered by tests
[warning] 112-114: src/packages/pickerview/pickerroller.tsx#L112-L114
Added lines #L112 - L114 were not covered by tests
[warning] 119-120: src/packages/pickerview/pickerroller.tsx#L119-L120
Added lines #L119 - L120 were not covered by tests
[warning] 124-128: src/packages/pickerview/pickerroller.tsx#L124-L128
Added lines #L124 - L128 were not covered by tests
[warning] 131-138: src/packages/pickerview/pickerroller.tsx#L131-L138
Added lines #L131 - L138 were not covered by tests
[warning] 141-143: src/packages/pickerview/pickerroller.tsx#L141-L143
Added lines #L141 - L143 were not covered by tests
[warning] 145-145: src/packages/pickerview/pickerroller.tsx#L145
Added line #L145 was not covered by tests
[warning] 147-155: src/packages/pickerview/pickerroller.tsx#L147-L155
Added lines #L147 - L155 were not covered by tests
[warning] 159-159: src/packages/pickerview/pickerroller.tsx#L159
Added line #L159 was not covered by tests
[warning] 161-161: src/packages/pickerview/pickerroller.tsx#L161
Added line #L161 was not covered by tests
[warning] 163-165: src/packages/pickerview/pickerroller.tsx#L163-L165
Added lines #L163 - L165 were not covered by tests
[warning] 176-176: src/packages/pickerview/pickerroller.tsx#L176
Added line #L176 was not covered by tests
[warning] 179-184: src/packages/pickerview/pickerroller.tsx#L179-L184
Added lines #L179 - L184 were not covered by tests
[warning] 194-197: src/packages/pickerview/pickerroller.tsx#L194-L197
Added lines #L194 - L197 were not covered by tests
[warning] 200-205: src/packages/pickerview/pickerroller.tsx#L200-L205
Added lines #L200 - L205 were not covered by tests
[warning] 207-210: src/packages/pickerview/pickerroller.tsx#L207-L210
Added lines #L207 - L210 were not covered by tests
[warning] 219-220: src/packages/pickerview/pickerroller.tsx#L219-L220
Added lines #L219 - L220 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
src/packages/pickerview/pickerroller.tsx (2)
1-26: 组件结构设计合理！组件使用 ForwardRef 转发引用，props 接口定义清晰，默认值设置合理。特别赞赏 renderLabel 的灵活设计，允许自定义标签渲染。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 23-24: src/packages/pickerview/pickerroller.tsx#L23-L24
Added lines #L23 - L24 were not covered by tests
223-233: 事件监听器依赖项问题仍然存在之前的审查意见仍然有效，当前 useEffect 的依赖数组包含了可能导致无限重渲染的依赖项。
src/packages/pickerview/pickerroller.taro.tsx (1)
321-326: 组件导出实现正确组件导出使用了 forwardRef 包装，符合 React 最佳实践。
| useEffect(() => { | ||
| const element = pickerRollerRef.current | ||
| if (element && web()) { | ||
| const computedStyle = getComputedStyle(element) | ||
| const currentLineSpacing = computedStyle.getPropertyValue( | ||
| '--nutui-picker-item-height' | ||
| ) | ||
| currentLineSpacing && | ||
| (lineSpacing.current = parseFloat(currentLineSpacing)) | ||
| } | ||
| }, [pickerRollerRef.current]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议增加错误处理和优化依赖
当前代码在获取 CSS 变量时缺少错误处理，且 useEffect 的依赖数组可以优化：
useEffect(() => {
const element = pickerRollerRef.current
if (element && web()) {
const computedStyle = getComputedStyle(element)
const currentLineSpacing = computedStyle.getPropertyValue(
'--nutui-picker-item-height'
)
- currentLineSpacing &&
- (lineSpacing.current = parseFloat(currentLineSpacing))
+ if (!currentLineSpacing) {
+ console.warn('未找到 --nutui-picker-item-height CSS 变量')
+ return
+ }
+ const parsedValue = parseFloat(currentLineSpacing)
+ if (isNaN(parsedValue)) {
+ console.warn('CSS 变量值格式无效')
+ return
+ }
+ lineSpacing.current = parsedValue
}
- }, [pickerRollerRef.current])
+ }, [])📝 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.
| useEffect(() => { | |
| const element = pickerRollerRef.current | |
| if (element && web()) { | |
| const computedStyle = getComputedStyle(element) | |
| const currentLineSpacing = computedStyle.getPropertyValue( | |
| '--nutui-picker-item-height' | |
| ) | |
| currentLineSpacing && | |
| (lineSpacing.current = parseFloat(currentLineSpacing)) | |
| } | |
| }, [pickerRollerRef.current]) | |
| useEffect(() => { | |
| const element = pickerRollerRef.current | |
| if (element && web()) { | |
| const computedStyle = getComputedStyle(element) | |
| const currentLineSpacing = computedStyle.getPropertyValue( | |
| '--nutui-picker-item-height' | |
| ) | |
| if (!currentLineSpacing) { | |
| console.warn('未找到 --nutui-picker-item-height CSS 变量') | |
| return | |
| } | |
| const parsedValue = parseFloat(currentLineSpacing) | |
| if (isNaN(parsedValue)) { | |
| console.warn('CSS 变量值格式无效') | |
| return | |
| } | |
| lineSpacing.current = parsedValue | |
| } | |
| }, []) |
github-actions
bot
added
action:review
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (13)
src/packages/pickerview/demos/h5/demo5.tsx (1)
27-29: 建议改进事件处理方式当前 onChange 事件仅打印日志，建议：
- 在生产环境中移除 console.log
- 添加实际的数据处理逻辑
- onChange={({ value, selectedOptions }) => { - console.log('onChange', value, selectedOptions) - }} + onChange={({ value, selectedOptions }) => { + // 添加实际的数据处理逻辑 + handleValueChange(value, selectedOptions) + }}src/packages/pickerview/demos/taro/demo5.tsx (2)
5-17: 建议将数据提取到常量文件建议将城市列表数据移至单独的常量文件中：
- 提高代码可维护性
- 方便数据复用
- 便于后续数据更新和管理
+// src/packages/pickerview/demos/constants.ts +export const CITY_LIST = [ + [ + { value: 1, label: '南京市' }, + { value: 2, label: '无锡市' }, + // ... 其他城市 + ], +] -const listData = [ - [ - { value: 1, label: '南京市' }, - // ... 其他城市 - ], -] +import { CITY_LIST } from '../constants'
19-34: 优化组件实现
- 建议移除生产环境中的 console.log
- 建议添加错误处理
- 考虑添加 onError 回调处理选择异常情况
<PickerView defaultValue={[1]} options={listData} threeDimensional={false} duration={500} onChange={({ value, selectedOptions }) => { - console.log('onChange', value, selectedOptions) + try { + // 处理选择变更 + handleChange(value, selectedOptions) + } catch (error) { + console.error('选择处理失败:', error) + } }} + onError={(error) => { + console.error('选择器错误:', error) + }} />src/packages/pickerview/demos/taro/demo2.tsx (3)
5-17: 建议优化数据结构的设计当前的数据结构有以下几点可以改进：
- value 值的序列不连续，建议使用连续的值以便维护
- 可以考虑将城市数据抽离到单独的配置文件中，提高代码的可维护性
- 可以为城市对象添加更多有用的元数据，如城市代码、所属省份等
建议重构数据结构如下：
// cities.ts export const cityList = [ { value: 1, label: '南京市', code: '320100', province: '江苏省' }, { value: 2, label: '无锡市', code: '320200', province: '江苏省' }, // ... 其他城市 ]
22-29: 需要移除生产环境中的 console.logonChange 事件处理器中的 console.log 应该仅用于开发调试，建议在生产环境中移除。
onChange={({ value, selectedOptions }) => { - console.log('onChange', value, selectedOptions) + // 在这里处理选中值的变化 }}
23-23: 建议使用 TypeScript 常量定义样式变量为了提高代码的可维护性和类型安全性，建议将 CSS 自定义属性定义为常量。
const PICKER_ITEM_HEIGHT = '28px'; // 在组件中使用 style={{ '--nutui-picker-item-height': PICKER_ITEM_HEIGHT }}src/packages/pickerview/demos/h5/demo4.tsx (3)
5-17: 建议优化数据结构的组织方式为了提高代码的可维护性和复用性，建议：
- 将城市数据抽离到单独的常量文件中
- 考虑添加更多的城市信息（如区号、邮编等）以支持未来可能的需求
建议的代码结构如下：
+// src/packages/pickerview/demos/h5/constants.ts +export const CITY_OPTIONS = [ + [ + { value: 1, label: '南京市', areaCode: '025' }, + // ... 其他城市 + ] +] +// src/packages/pickerview/demos/h5/demo4.tsx -const listData = [ - [ - { value: 1, label: '南京市' }, - // ... 其他城市 - ], -] +import { CITY_OPTIONS } from './constants' +const listData = CITY_OPTIONS
28-28: 建议移除生产环境中的 console.log在生产环境中保留 console.log 语句可能会影响性能，建议使用适当的日志系统或在生产构建时移除。
-console.log('onChange', value, selectedOptions) +// 使用适当的日志系统或条件性日志 +if (process.env.NODE_ENV === 'development') { + console.log('onChange', value, selectedOptions) +}
29-31: 建议优化选择逻辑，提升用户体验当用户选择"海北藏族自治区"时直接切换到"南京市"的逻辑可能会让用户感到困惑。建议：
- 添加适当的用户提示
- 考虑使用更平滑的过渡效果
if (value[0] === 3) { + Toast.show('由于业务限制，已自动切换到南京市') setValue([1]) }src/packages/pickerview/demos/h5/demo2.tsx (1)
5-17: 建议优化数据结构设计
- 当前数据结构使用嵌套数组，但实际只使用单列数据，建议简化为单层数组。
- value 值不连续（1-12之间有跳跃），建议使用连续的值以便于维护和扩展。
建议修改为：
const listData = [ - [ { value: 1, label: '南京市' }, { value: 2, label: '无锡市' }, { value: 3, label: '海北藏族自治区' }, { value: 4, label: '北京市' }, { value: 5, label: '连云港市' }, - { value: 8, label: '大庆市' }, - { value: 9, label: '绥化市' }, - { value: 10, label: '潍坊市' }, - { value: 12, label: '乌鲁木齐市' }, + { value: 6, label: '大庆市' }, + { value: 7, label: '绥化市' }, + { value: 8, label: '潍坊市' }, + { value: 9, label: '乌鲁木齐市' }, - ], ]src/packages/pickerview/demos/taro/demo4.tsx (3)
1-4: 建议优化组件命名以更好地反映其用途当前的组件名称
Demo4过于简单，建议改为更具描述性的名称，例如PickerViewCityDemo或CityPickerViewExample，以便更好地表达组件的功能和用途。-const Demo4 = () => { +const PickerViewCityDemo = () => {
5-17: 建议优化数据结构定义当前的数据结构可以通过以下方式改进：
- 添加 TypeScript 接口定义以增强类型安全性
- 对城市数据进行更有意义的分组（如按省份分组）
建议添加以下类型定义：
interface CityOption { value: number; label: string; } const listData: CityOption[][] = [ [ { value: 1, label: '南京市' }, // ... 其他城市 ], ]
19-19: 建议优化状态管理当前的状态变量命名和初始值设置可以更清晰：
- 建议使用更具描述性的状态变量名称
- 添加注释说明初始值的选择原因
-const [value, setValue] = useState([2]) +// 默认选中"无锡市" +const [selectedCityValue, setSelectedCityValue] = useState([2])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/packages/pickerview/demos/h5/demo2.tsx(1 hunks)src/packages/pickerview/demos/h5/demo4.tsx(1 hunks)src/packages/pickerview/demos/h5/demo5.tsx(1 hunks)src/packages/pickerview/demos/taro/demo2.tsx(1 hunks)src/packages/pickerview/demos/taro/demo4.tsx(1 hunks)src/packages/pickerview/demos/taro/demo5.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (11)
src/packages/pickerview/demos/h5/demo5.tsx (3)
1-4: 代码结构清晰，导入规范！组件的导入和结构定义符合 React 最佳实践。
5-17: 建议将城市数据提取为共享常量为保持代码的可维护性，建议将 listData 移至单独的常量文件中。
35-35: 导出语句规范！组件的导出方式符合 React 组件的标准做法。
src/packages/pickerview/demos/taro/demo5.tsx (1)
1-4: 建议重构以减少代码重复当前的实现方式可能导致H5和Taro版本之间存在重复代码。建议参考之前的建议，创建一个共享的基础组件。
src/packages/pickerview/demos/taro/demo2.tsx (2)
1-2: 导入语句看起来没有问题！导入了必要的组件和依赖项。
34-34: 导出语句正确！组件的默认导出符合常见的最佳实践。
src/packages/pickerview/demos/h5/demo4.tsx (3)
1-4: 代码结构清晰，导入合理！组件的导入和声明遵循了 React 最佳实践。
19-19: 状态管理实现恰当！使用 useState 管理选中值，初始值设置合理。
38-38: 导出语句正确！组件导出方式符合规范。
src/packages/pickerview/demos/h5/demo2.tsx (2)
1-4: 代码结构清晰，命名规范！组件命名与文件名保持一致，导入声明简洁明确。
34-34: 导出语句规范！默认导出符合 React 组件的常见实践。
| return ( | ||
| <> | ||
| <Cell> | ||
| <PickerView | ||
| style={{ '--nutui-picker-item-height': '28px' }} | ||
| options={listData} | ||
| defaultValue={[1]} | ||
| onChange={({ value, selectedOptions }) => { | ||
| console.log('onChange', value, selectedOptions) | ||
| }} | ||
| /> | ||
| </Cell> | ||
| </> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议增强组件的健壮性和可访问性
- 生产环境代码中不应该保留 console.log
- 缺少错误处理机制
- 需要增加无障碍访问属性
建议修改为：
return (
<>
<Cell>
<PickerView
style={{ '--nutui-picker-item-height': '28px' }}
options={listData}
defaultValue={[1]}
+ aria-label="城市选择器"
+ role="listbox"
onChange={({ value, selectedOptions }) => {
- console.log('onChange', value, selectedOptions)
+ try {
+ // 处理选择变更
+ onCityChange?.(value, selectedOptions)
+ } catch (error) {
+ console.error('城市选择发生错误:', error)
+ }
}}
/>
</Cell>
</>
)📝 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.
| return ( | |
| <> | |
| <Cell> | |
| <PickerView | |
| style={{ '--nutui-picker-item-height': '28px' }} | |
| options={listData} | |
| defaultValue={[1]} | |
| onChange={({ value, selectedOptions }) => { | |
| console.log('onChange', value, selectedOptions) | |
| }} | |
| /> | |
| </Cell> | |
| </> | |
| ) | |
| } | |
| return ( | |
| <> | |
| <Cell> | |
| <PickerView | |
| style={{ '--nutui-picker-item-height': '28px' }} | |
| options={listData} | |
| defaultValue={[1]} | |
| aria-label="城市选择器" | |
| role="listbox" | |
| onChange={({ value, selectedOptions }) => { | |
| try { | |
| // 处理选择变更 | |
| onCityChange?.(value, selectedOptions) | |
| } catch (error) { | |
| console.error('城市选择发生错误:', error) | |
| } | |
| }} | |
| /> | |
| </Cell> | |
| </> | |
| ) | |
| } |
| return ( | ||
| <> | ||
| <Cell> | ||
| <PickerView | ||
| value={value} | ||
| options={listData} | ||
| onChange={({ value, selectedOptions }) => { | ||
| console.log('onChange', value, selectedOptions) | ||
| if (value[0] === 3) { | ||
| setValue([1]) | ||
| } | ||
| }} | ||
| /> | ||
| </Cell> | ||
| </> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议优化事件处理逻辑
当前的事件处理逻辑存在以下可改进之处：
- 生产环境代码中不应该保留 console.log
- 特殊情况处理（value[0] === 3）需要添加注释说明原因
- 建议添加错误处理
建议按如下方式修改：
<PickerView
- value={value}
+ value={selectedCityValue}
options={listData}
onChange={({ value, selectedOptions }) => {
- console.log('onChange', value, selectedOptions)
+ try {
+ // 当选择"海北藏族自治区"时，自动切换到"南京市"
+ if (value[0] === 3) {
+ setSelectedCityValue([1])
+ } else {
+ setSelectedCityValue(value)
+ }
+ } catch (error) {
+ console.error('选择城市时发生错误:', error)
+ }
}}
/>📝 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.
| return ( | |
| <> | |
| <Cell> | |
| <PickerView | |
| value={value} | |
| options={listData} | |
| onChange={({ value, selectedOptions }) => { | |
| console.log('onChange', value, selectedOptions) | |
| if (value[0] === 3) { | |
| setValue([1]) | |
| } | |
| }} | |
| /> | |
| </Cell> | |
| </> | |
| ) | |
| } | |
| return ( | |
| <> | |
| <Cell> | |
| <PickerView | |
| value={selectedCityValue} | |
| options={listData} | |
| onChange={({ value, selectedOptions }) => { | |
| try { | |
| // 当选择"海北藏族自治区"时，自动切换到"南京市" | |
| if (value[0] === 3) { | |
| setSelectedCityValue([1]) | |
| } else { | |
| setSelectedCityValue(value) | |
| } | |
| } catch (error) { | |
| console.error('选择城市时发生错误:', error) | |
| } | |
| }} | |
| /> | |
| </Cell> | |
| </> | |
| ) | |
| } |
github-actions
bot
added
action:review
github-actions
bot
added
action:review
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
src/packages/pickerview/pickerroller.tsx (2)
124-167: 建议增加触摸事件的错误处理当前触摸事件处理逻辑实现完整，但建议增加以下改进：
- 添加 try-catch 块处理可能的事件异常
- 处理触摸事件可能的空值情况
- 添加适当的错误日志
建议参考以下示例添加错误处理：
const touchStart = (event: React.TouchEvent<HTMLDivElement>) => { + try { touch.start(event) setStartY(touch.deltaY.current) setStartTime(Date.now()) transformY.current = scrollDistance + } catch (error) { + console.error('触摸事件处理失败:', error) + } }
258-293: 建议优化渲染性能当前渲染逻辑可以通过以下方式优化：
- 使用 useMemo 缓存 options 的映射结果
- 优化 key 的选择，避免使用索引作为后备值
- 考虑使用虚拟滚动处理大量选项
建议参考以下示例优化渲染逻辑：
+const renderedOptions = useMemo(() => + options.map((item, index) => ( + <div + className={`nut-pickerview-roller-item ${ + isHidden(index + 1) && 'nut-pickerview-roller-item-hidden' + }`} + style={rollerStyle(index)} + key={item.value} + > + {renderLabel(item)} + </div> + )), [options, isHidden, rollerStyle, renderLabel]) return ( <div className="nut-pickerview-list" ref={pickerRollerRef}> {/* ... */} - {threeDimensional && options.map((item, index) => (/* ... */))} + {threeDimensional && renderedOptions} {/* ... */} </div> )src/packages/pickerview/doc.en-US.md (3)
1-4: 建议增强组件概述部分当前概述比较简单，建议补充以下内容：
- 组件的主要用途和应用场景
- 与 Picker 组件的关系和区别
- 核心特性和优势
13-59: 建议为每个示例添加功能说明为了帮助用户更好地理解每个示例的用途，建议在每个示例前添加简要说明：
- 基础用法：说明最基本的使用方式
- 受控组件：解释如何通过外部状态控制组件
- 高度调整：说明如何自定义面板高度
- 多列展示：展示多列数据的配置方式
- 平铺展示：说明平铺模式的特点
- 级联选择：解释级联数据的使用场景
63-80: 建议完善属性文档为了提供更完整的使用指导，建议：
- 为每个属性添加示例值
- 补充
onChange回调参数的详细类型说明- 说明
value和defaultValue的数据格式要求- 为
PickerOptionItem添加示例代码示例格式：
interface PickerOptionItem { label: string | number; // 例如：'选项1' 或 1 value: string | number; // 例如：'value1' 或 1 children?: PickerOptionItem[]; // 用于级联数据，例如：[{label: '子选项1', value: 'child1'}] }src/packages/pickerview/pickerroller.taro.tsx (3)
14-28: 建议加强类型安全性当前的类型定义可以进一步优化：
keyIndex可以使用更具体的类型而不是默认的 numberrenderLabel的返回值类型应该显式声明- 建议为
PickerOptionItem添加更严格的类型约束建议按如下方式优化类型定义：
+ type KeyIndex = number; + type RenderLabelReturn = string | JSX.Element; const InternalPickerRoller: ForwardRefRenderFunction< { stopMomentum: () => void; moving: boolean }, Partial<PickerRollerProps> > = (props, ref) => { const { - keyIndex = 0, + keyIndex: KeyIndex = 0, options = [], threeDimensional = true, duration = 1000, onSelect, - renderLabel = (item: PickerOptionItem) => { + renderLabel = (item: PickerOptionItem): RenderLabelReturn => { return item.label }, } = props
161-168: 优化惯性滚动实现当前的惯性滚动实现可以进行以下优化：
- 添加最大速度限制，防止滚动过快
- 考虑设备性能，添加帧率控制
- 增加边界情况处理
建议按如下方式优化实现：
const momentum = (distance: number, duration: number) => { let nDistance = distance // 惯性滚动的速度 const speed = Math.abs(nDistance / duration) + // 限制最大速度 + const MAX_SPEED = 2.5 + const limitedSpeed = Math.min(speed, MAX_SPEED) // 惯性滚动的距离 - nDistance = (speed / 0.003) * (nDistance < 0 ? -1 : 1) + nDistance = (limitedSpeed / 0.003) * (nDistance < 0 ? -1 : 1) + // 确保不超出边界 + const maxDistance = lineSpacing.current * (options.length - 1) + return Math.max(Math.min(nDistance, maxDistance), -maxDistance) return nDistance }
283-318: 优化渲染性能当前的渲染实现可以进行以下优化：
- 使用
useMemo缓存样式计算- 优化条件渲染逻辑
- 减少不必要的重渲染
建议按如下方式优化：
+ const memoizedRollerStyle = useMemo(() => + threeDimensional ? touchRollerStyle() : touchTileStyle(), + [threeDimensional, touchTime, touchDeg, scrollDistance] + ) + const memoizedOptions = useMemo(() => + options.map((item, index) => ({ + ...item, + style: rollerStyle(index), + hidden: isHidden(index + 1) + })), + [options, rotation, lineSpacing.current] + ) return ( <View className="nut-pickerview-list" ref={pickerRollerRef}> <View className="nut-pickerview-roller" ref={rollerRef} - style={threeDimensional ? touchRollerStyle() : touchTileStyle()} + style={memoizedRollerStyle} onTransitionEnd={stopMomentum} > {threeDimensional ? ( - options.map((item, index) => ( + memoizedOptions.map(({ style, hidden, ...item }, index) => ( <View className={`nut-pickerview-roller-item ${ - isHidden(index + 1) && 'nut-pickerview-roller-item-hidden' + hidden && 'nut-pickerview-roller-item-hidden' }`} - style={rollerStyle(index)} + style={style} key={item.value ?? index} > {renderLabel(item)} </View> )) ) : ( // 平铺模式保持不变... )} </View> </View> )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/packages/pickerview/__test__/__snapshots__/pickerview.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
src/packages/pickerview/__test__/pickerview.spec.tsx(1 hunks)src/packages/pickerview/doc.en-US.md(1 hunks)src/packages/pickerview/pickerroller.taro.tsx(1 hunks)src/packages/pickerview/pickerroller.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/pickerview/test/pickerview.spec.tsx
🧰 Additional context used
🪛 GitHub Check: build
src/packages/pickerview/pickerroller.tsx
[failure] 8-8:
Cannot find module '@/utils/use-touch' or its corresponding type declarations.
src/packages/pickerview/pickerroller.taro.tsx
[failure] 9-9:
Cannot find module '@/utils/use-touch' or its corresponding type declarations.
🪛 GitHub Actions: E2E Tests
src/packages/pickerview/pickerroller.tsx
[error] 19-19: Failed to resolve import '@/utils/use-touch'. Does the file exist?
🪛 markdownlint-cli2 (0.17.2)
src/packages/pickerview/doc.en-US.md
86-86: Link fragments should be valid
null
(MD051, link-fragments)
🪛 GitHub Actions: CI
src/packages/pickerview/pickerroller.taro.tsx
[error] 9-9: Cannot find module '@/utils/use-touch' or its corresponding type declarations.
🔇 Additional comments (2)
src/packages/pickerview/pickerroller.tsx (2)
12-26: 代码结构清晰，类型定义完善组件初始化的实现很规范：
- 使用 ForwardRefRenderFunction 正确处理 ref 转发
- 默认属性设置合理
- Props 接口设计完整
🧰 Tools
🪛 GitHub Actions: E2E Tests
[error] 19-19: Failed to resolve import '@/utils/use-touch'. Does the file exist?
225-235: 事件监听器管理优化到位useEffect 的实现已经得到改进：
- 正确设置了依赖数组
- 清理函数移除了所有事件监听器
- 避免了重复绑定事件的问题
github-actions
bot
added
action:review
github-actions
bot
added
action:review
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
src/packages/pickerview/pickerview.tsx (1)
182-182: 清理注释代码建议移除被注释的代码
// return selectedOption || columnOptions[0]，如果这是一个重要的备选方案，建议添加说明注释解释为什么不使用这个方案。src/packages/pickerview/pickerview.taro.tsx (1)
224-260: 事件监听设置合理，但可以优化事件监听的设置和清理实现得当，使用了 passive 事件监听器提升性能。建议考虑：
- 将事件选项对象提取为常量，避免重复创建
- 考虑使用 Taro 的事件系统而不是原生事件
+ const EVENT_OPTIONS = passiveSupported ? { passive: false, once: true } : false; useEffect(() => { pickerRollerRef.current?.addEventListener( 'touchstart', touchStart, - eventOptions + EVENT_OPTIONS ) // ... 其他事件监听器同样更新 }, [pickerRollerRef.current, touchStart, touchMove, touchEnd])src/packages/pickerview/pickerroller.tsx (1)
170-193: 简化 modifyStatus 方法的实现当前实现包含重复的查找逻辑和复杂的条件判断。建议使用数组方法简化：
const modifyStatus = (type?: boolean, val?: string | number) => { const value = val || props.value - let index = -1 - if (value) { - options.some((item, idx) => { - if (item.value === value) { - index = idx - return true - } - return false - }) - } else { - options.forEach((item, i) => { - if (item.value === props.value) { - index = i - } - }) - } + const index = options.findIndex(item => + item.value === (value || props.value) + ) setCurrIndex(index === -1 ? 1 : index + 1) const move = index * lineSpacing.current type && setChooseValue(-move) setMove(-move) }src/packages/pickerview/pickerroller.taro.tsx (1)
8-12: 确保 Taro 平台兼容性当前实现使用了 Taro 特定的组件和工具：
- 使用
@tarojs/components的View组件- 引入
platform-taro的web方法- 使用平台特定的钩子和工具
建议添加以下测试和验证：
- 在不同小程序平台上的兼容性测试
- 触摸事件在各平台的表现验证
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/packages/pickerview/pickerroller.taro.tsx(1 hunks)src/packages/pickerview/pickerroller.tsx(1 hunks)src/packages/pickerview/pickerview.taro.tsx(1 hunks)src/packages/pickerview/pickerview.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/pickerview/pickerview.taro.tsx
[error] 88-88: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/pickerview/pickerview.tsx
[error] 87-87: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Check: build
src/packages/pickerview/pickerview.taro.tsx
[failure] 13-13:
Cannot find module '@/utils/use-props-value' or its corresponding type declarations.
[failure] 139-139:
Parameter 'prev' implicitly has an 'any' type.
src/packages/pickerview/pickerview.tsx
[failure] 12-12:
Cannot find module '@/utils/use-props-value' or its corresponding type declarations.
[failure] 138-138:
Parameter 'prev' implicitly has an 'any' type.
🪛 GitHub Actions: CI
src/packages/pickerview/pickerview.taro.tsx
[error] 13-13: Cannot find module '@/utils/use-props-value' or its corresponding type declarations.
src/packages/pickerview/pickerview.tsx
[error] 12-12: Failed to resolve import '@/utils/use-props-value'. Does the file exist?
🪛 GitHub Actions: E2E Tests
src/packages/pickerview/pickerview.tsx
[error] 12-12: Failed to resolve import '@/utils/use-props-value'. Does the file exist?
[warning] 12-12: Invalid deprecation 'global-builtin'.
[warning] 12-12: Future import deprecation is not yet active, so silencing it is unnecessary.
🔇 Additional comments (2)
src/packages/pickerview/pickerview.tsx (1)
60-70: 代码实现正确且健壮已正确实现了空数组保护，避免了潜在的运行时错误。同时类型判断逻辑清晰。
src/packages/pickerview/pickerroller.tsx (1)
8-8:Details
✅ Verification successful
验证模块导入路径
建议确认
@/hooks/use-touch模块的导入路径是否正确配置。需要检查：
- 项目的路径别名配置
- 模块是否存在于正确位置
- 类型声明文件是否完整
🏁 Script executed:
#!/bin/bash # 验证模块是否存在 fd -t f "use-touch.{ts,tsx}" hooks/Length of output: 131
请验证模块导入路径配置
当前在尝试寻找
@/hooks/use-touch模块时，之前的搜索定位到了错误路径（即使用了hooks/而非src/hooks/）。请手动确认以下事项：
- 是否在
src/hooks目录下存在文件名为use-touch.ts或use-touch.tsx的模块；- 项目的路径别名
@是否正确映射到src目录；- 类型声明文件是否完整无误。
请开发者执行上面的 shell 脚本并验证文件是否存在，然后根据检测结果调整导入路径或别名配置。
🏁 Script executed:
[run_scripts] #!/bin/bash # 查找 use-touch 模块文件在 src/hooks 目录下 fd -t f -e ts -e tsx "use-touch" src/hooks/Length of output: 188
模块导入路径已验证
经确认，项目中确实存在文件
src/hooks/use-touch.ts，因此当前的模块导入路径@/hooks/use-touch是有效的。不过，为确保项目整体配置一致，请检查以下内容：
- 确认路径别名
@是否在 tsconfig.json（或相关构建配置）中正确映射至src目录。- 检查该模块的类型声明文件是否完善，满足项目的使用需求。
如果上述配置都正常，则无需进一步修改导入路径。
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/packages/pickerview/pickerview.taro.tsx (1)
61-71: 🛠️ Refactor suggestion建议增加空值保护
在判断列类型时，建议增加更完善的空值检查：
const columnsType = useMemo(() => { - const firstColumn = (props.options as PickerOptions[])[0] || [] + const firstColumn = (props.options as PickerOptions[])?.at(0) ?? [] if ( Array.isArray(firstColumn) && firstColumn.length > 0 && - 'children' in firstColumn[0] + firstColumn[0] && 'children' in firstColumn[0] ) { return 'cascade' } return 'multiple' }, [props.options])
🧹 Nitpick comments (2)
src/packages/pickerview/pickerview.taro.tsx (2)
73-108: 优化级联选项格式化逻辑建议对
formatCascadeOptions函数进行以下优化：
- 使用可选链简化空值判断
- 添加类型断言提升类型安全性
const formatCascadeOptions = ( options: PickerOptions, value: PickerValue[] ) => { if (!options.length) return [] const formatted: PickerOptions[] = [] let columnOptions: PickerOptionItem = { label: '', value: '', children: options, } let columnIndex = 0 - while (columnOptions && columnOptions.children) { + while (columnOptions?.children) { const currentOptions: PickerOptions = columnOptions.children formatted.push(currentOptions) const currentValue = value?.[columnIndex] if (currentValue === 0) { columnOptions = currentOptions[0] } else if (currentValue) { const index = currentOptions.findIndex( (columnItem) => columnItem.value === currentValue ) columnOptions = currentOptions[index === -1 ? 0 : index] } else { break } columnIndex++ } return formatted }🧰 Tools
🪛 Biome (1.9.4)
[error] 88-88: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
133-174: 建议拆分选择处理逻辑当前
handleSelect函数同时处理多列和级联两种情况，建议拆分为独立函数以提高可维护性：+ const handleMultipleSelect = ( + option: PickerOptionItem, + index: number, + prev: PickerValue[] + ) => { + const next = [...prev] + next[index] = option.value + return next + } + + const handleCascadeSelect = ( + option: PickerOptionItem, + index: number, + innerValue: PickerValue[] + ) => { + const values: PickerValue[] = [] + let currentOption = option + values[index] = option.value + + while (currentOption?.children?.[0]) { + values[index + 1] = currentOption.children[0].value + index++ + currentOption = currentOption.children[0] + } + + if (currentOption?.children?.length) { + values[index + 1] = '' + } + + return [...innerValue.slice(0, index), ...values.splice(index)] + } const handleSelect = useCallback( (option: PickerOptionItem, index: number) => { const newValue = option?.value if (!newValue || innerValue[index] === newValue) return changeIndex.current = index if (columnsType === 'multiple') { - setInnerValue((prev) => { - const next = [...prev] - next[index] = newValue - return next - }) + setInnerValue((prev) => handleMultipleSelect(option, index, prev)) } else { - const startIndex = index - const values: PickerValue[] = [] - // ... existing cascade logic ... + const combineResult = handleCascadeSelect(option, index, innerValue) setInnerValue(combineResult) const optionFirst = props?.options?.[0] as PickerOptionItem[] if (!isEqual( formatCascadeOptions(optionFirst, combineResult), innerOptions )) { setInnerOptions(formatCascadeOptions(optionFirst, combineResult)) } } }, [innerValue, props.options, columnsType, innerOptions] )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/pickerview/pickerview.taro.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/pickerview/pickerview.taro.tsx
[error] 88-88: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (3)
src/packages/pickerview/pickerview.taro.tsx (3)
9-13: 请确保所有导入路径正确配置以下导入路径需要验证：
@/utils/typings@/hooks/use-props-value请确保：
- 这些模块在正确的路径下
- Taro 项目配置中的路径别名设置正确
22-29: 默认属性设置合理默认属性的设置遵循了最佳实践，包括了必要的默认值和类型定义。
196-219: 渲染逻辑实现正确已正确使用 Taro 的 View 组件替代原生 div，渲染逻辑清晰且实现合理。
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/packages/pickerview/doc.md (3)
1-4: 建议补充组件介绍内容当前介绍过于简单，建议补充以下内容：
- 组件的主要使用场景
- 与 Picker 组件的关系和区别
- 核心功能特点
13-59: 建议为每个示例添加说明文字为了帮助开发者更好地理解每个示例的用途，建议在每个示例代码块前添加简要说明，描述该示例的具体使用场景和实现效果。
72-72: 建议完善 onChange 事件说明建议补充 onChange 回调函数参数的详细说明：
- value: 当前选中项的值数组
- index: 当前变更列的索引
- selectedOptions: 当前选中项的完整选项数据
src/packages/pickerview/pickerview.tsx (1)
147-151: 优化代码安全性建议使用可选链操作符来优化代码安全性。
- while (option?.children?.[0]) { - values[index + 1] = option.children[0].value + while (option?.children?.[0]?.value) { + values[index + 1] = option.children?.[0].value index++ - option = option.children[0] + option = option.children?.[0] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/packages/pickerview/doc.en-US.md(1 hunks)src/packages/pickerview/doc.md(1 hunks)src/packages/pickerview/doc.taro.md(1 hunks)src/packages/pickerview/doc.zh-TW.md(1 hunks)src/packages/pickerview/pickerroller.taro.tsx(1 hunks)src/packages/pickerview/pickerroller.tsx(1 hunks)src/packages/pickerview/pickerview.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/packages/pickerview/doc.en-US.md
- src/packages/pickerview/doc.zh-TW.md
- src/packages/pickerview/pickerroller.taro.tsx
- src/packages/pickerview/doc.taro.md
- src/packages/pickerview/pickerroller.tsx
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
src/packages/pickerview/doc.md
86-86: Link fragments should be valid
null
(MD051, link-fragments)
🪛 Biome (1.9.4)
src/packages/pickerview/pickerview.tsx
[error] 87-87: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (6)
src/packages/pickerview/pickerview.tsx (6)
11-12: 修复模块导入路径建议检查并修正模块导入路径，确保与项目的路径别名配置一致。
建议执行以下操作：
- 确认
@/utils/typings和@/hooks/use-props-value模块存在- 检查项目中
@别名的配置是否正确- 验证模块是否包含必要的类型声明文件
21-28: 默认属性设置合理默认属性的设置符合最佳实践，提供了必要的默认值。
60-70: 列类型判断逻辑完善已经添加了空数组保护，避免了访问
firstColumn[0]时可能出现的错误。
138-138: 增加类型安全性
prev参数需要显式声明类型。- setInnerValue((prev) => { + setInnerValue((prev: PickerValue[]) => {
175-185: 确认默认选项处理逻辑当前实现移除了默认选择第一个选项的逻辑（第182行的注释）。请确认：
- 是否需要保留默认选择第一个选项的逻辑
- 当前实现是否会导致未选择状态下的异常
195-217: 渲染逻辑清晰合理组件渲染逻辑结构清晰，包含了必要的遮罩层和指示器元素。
|
|
||
| ### 样式变量 | ||
|
|
||
| 组件提供了下列 CSS 变量，可用于自定义样式，使用方法请参考 [ConfigProvider 组件](#/zh-CN/component/configprovider)。 |
There was a problem hiding this comment.
修复 ConfigProvider 组件链接
当前文档中的 ConfigProvider 组件链接 #/zh-CN/component/configprovider 无效，请更新为正确的链接地址。
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
86-86: Link fragments should be valid
null
(MD051, link-fragments)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/packages/pickerview/utils.ts (1)
1-4: 建议添加参数验证和单元测试
- 建议添加参数验证以处理边缘情况
- 魔术数字 0.003 应该提取为命名常量
- 需要添加单元测试以验证功能正确性
建议修改如下:
+const MOMENTUM_FACTOR = 0.003 + export const momentum = (distance: number, duration: number) => { + if (duration === 0) return 0 const speed = Math.abs(distance / duration) - return (speed / 0.003) * (distance < 0 ? -1 : 1) + return (speed / MOMENTUM_FACTOR) * (distance < 0 ? -1 : 1) }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 2-4: src/packages/pickerview/utils.ts#L2-L4
Added lines #L2 - L4 were not covered by testssrc/packages/pickerview/hooks/useStyles.ts (2)
8-11: 建议优化过渡样式函数的类型安全性建议为
transformValue参数添加类型定义，并提取过渡时间曲线为常量。+const TRANSITION_TIMING = 'cubic-bezier(0.17, 0.89, 0.45, 1)' + -const getTransitionStyle = (transformValue: string) => ({ +const getTransitionStyle = (transformValue: `rotate3d(${number}, ${number}, ${number}, ${string})` | `translate3d(${string}, ${string}, ${string})`) => ({ - transition: `transform ${touchTime}ms cubic-bezier(0.17, 0.89, 0.45, 1)`, + transition: `transform ${touchTime}ms ${TRANSITION_TIMING}`, transform: transformValue, })
19-23: 建议提取魔术数字为命名常量在
rollerStyle函数中，3.2 是一个魔术数字，应该提取为有意义的命名常量。+const DEPTH_FACTOR = 3.2 + const rollerStyle = (index: number) => ({ transform: `rotate3d(1, 0, 0, ${-rotation * (index + 1)}deg) translate3d(0px, 0px, ${Math.round( - lineSpacing.current * 3.2 + lineSpacing.current * DEPTH_FACTOR )}px)`, })src/packages/pickerview/pickerroller.tsx (2)
28-31: 建议提取魔术数字为命名常量这些数值应该定义为有意义的常量，并考虑通过 props 允许自定义。
-const DEFAULT_DURATION = 200 -const INERTIA_TIME = 300 -const INERTIA_DISTANCE = 15 -const ROTATION = 20 +const ANIMATION_CONSTANTS = { + DEFAULT_DURATION: 200, + INERTIA_TIME: 300, + INERTIA_DISTANCE: 15, + ROTATION: 20 +} as const
67-94: 建议添加错误边界处理
handleMove函数缺少错误处理机制，建议添加 try-catch 块来捕获可能的计算错误。const handleMove = (move: number, type?: string, time?: number) => { + try { let updatedMove = move + transformY.current if (type === 'end') { // ... existing code ... } else { // ... existing code ... } + } catch (error) { + console.error('移动计算错误:', error) + // 重置到安全状态 + setScrollDistance(0) + setTouchDeg('0deg') + } }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 70-73: src/packages/pickerview/pickerroller.tsx#L70-L73
Added lines #L70 - L73 were not covered by tests
[warning] 76-80: src/packages/pickerview/pickerroller.tsx#L76-L80
Added lines #L76 - L80 were not covered by testssrc/packages/pickerview/pickerview.scss (2)
58-61: 建议使用 Autoprefixer 处理浏览器前缀不建议手动添加浏览器前缀，应该使用 Autoprefixer 自动处理。
backface-visibility: hidden; - -moz-backface-visibility: hidden; - -webkit-backface-visibility: hidden;
6-7: 建议优化 CSS 计算逻辑建议将复杂的计算提取为独立的 CSS 变量，以提高可维护性。
+ --nutui-picker-list-height: #{$picker-list-height}; + --nutui-picker-top: calc((var(--nutui-picker-list-height) - var(--nutui-picker-item-height)) / 2); height: $picker-list-height; - $pickerview-top: calc(($picker-list-height - $picker-item-height) / 2); + $pickerview-top: var(--nutui-picker-top);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/packages/pickerview/__test__/__snapshots__/pickerview.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
src/packages/pickerview/hooks/useStyles.ts(1 hunks)src/packages/pickerview/pickerroller.taro.tsx(1 hunks)src/packages/pickerview/pickerroller.tsx(1 hunks)src/packages/pickerview/pickerview.scss(1 hunks)src/packages/pickerview/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/pickerview/pickerroller.taro.tsx
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/pickerview/utils.ts
[warning] 2-4: src/packages/pickerview/utils.ts#L2-L4
Added lines #L2 - L4 were not covered by tests
src/packages/pickerview/pickerroller.tsx
[warning] 70-73: src/packages/pickerview/pickerroller.tsx#L70-L73
Added lines #L70 - L73 were not covered by tests
[warning] 76-80: src/packages/pickerview/pickerroller.tsx#L76-L80
Added lines #L76 - L80 were not covered by tests
[warning] 101-105: src/packages/pickerview/pickerroller.tsx#L101-L105
Added lines #L101 - L105 were not covered by tests
[warning] 108-115: src/packages/pickerview/pickerroller.tsx#L108-L115
Added lines #L108 - L115 were not covered by tests
[warning] 118-130: src/packages/pickerview/pickerroller.tsx#L118-L130
Added lines #L118 - L130 were not covered by tests
[warning] 156-156: src/packages/pickerview/pickerroller.tsx#L156
Added line #L156 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (2)
src/packages/pickerview/pickerroller.tsx (2)
148-158: 优化 CSS 变量获取的错误处理当前获取 CSS 变量的逻辑缺少适当的错误处理机制。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 156-156: src/packages/pickerview/pickerroller.tsx#L156
Added line #L156 was not covered by tests
172-202: 修正重复绑定事件监听的副作用当前
useEffect未设置依赖数组，会在组件每次渲染时反复绑定/解绑事件。
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/packages/pickerview/pickerroller.tsx (2)
147-157: 🛠️ Refactor suggestion优化 CSS 变量获取逻辑
当前 CSS 变量获取逻辑缺少适当的错误处理。建议：
- 添加 CSS 变量不存在的警告
- 处理解析失败的情况
- 提供默认值保障
useEffect(() => { const element = pickerRollerRef.current if (element) { const computedStyle = getComputedStyle(element) const currentLineSpacing = computedStyle.getPropertyValue( '--nutui-picker-item-height' ) - !!currentLineSpacing && - (lineSpacing.current = parseFloat(currentLineSpacing)) + if (!currentLineSpacing) { + console.warn('未找到 --nutui-picker-item-height CSS 变量') + return + } + const parsedValue = parseFloat(currentLineSpacing) + if (isNaN(parsedValue)) { + console.warn('CSS 变量值格式无效') + return + } + lineSpacing.current = parsedValue } }, [])🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 155-155: src/packages/pickerview/pickerroller.tsx#L155
Added line #L155 was not covered by tests
171-201: 🛠️ Refactor suggestion优化事件监听器的设置
当前实现在每次渲染时都会重新绑定事件监听器，这可能导致性能问题。建议：
- 优化依赖数组
- 使用 useCallback 包装事件处理函数
+const memoizedTouchStart = useCallback(handleTouchStart, [touch, setStartY, setStartTime]) +const memoizedTouchMove = useCallback(handleTouchMove, [touch, moving, setMove]) +const memoizedTouchEnd = useCallback(handleTouchEnd, [moving, touch, setMove]) useEffect(() => { const options = passiveSupported ? { passive: false } : false pickerRollerRef.current?.addEventListener('touchstart', handleTouchStart, options) pickerRollerRef.current?.addEventListener('touchmove', handleTouchMove, options) pickerRollerRef.current?.addEventListener('touchend', handleTouchEnd, options) return () => { pickerRollerRef.current?.removeEventListener('touchstart', handleTouchStart) pickerRollerRef.current?.removeEventListener('touchmove', handleTouchMove) pickerRollerRef.current?.removeEventListener('touchend', handleTouchEnd) } -}, [pickerRollerRef.current, handleTouchStart, handleTouchMove, handleTouchEnd]) +}, [memoizedTouchStart, memoizedTouchMove, memoizedTouchEnd])src/packages/pickerview/pickerroller.taro.tsx (2)
174-176:⚠️ Potential issue修复事件选项配置
当前实现在 Taro 版本中添加了
once: true选项，这会导致事件监听器在触发一次后被移除。建议移除该选项，因为这些事件需要持续监听。const eventOptions = passiveSupported - ? { passive: false, once: true } + ? { passive: false } : false
149-159: 🛠️ Refactor suggestion优化平台特定的 CSS 变量处理
当前实现在 Taro 环境下的 CSS 变量获取逻辑需要更健壮的错误处理。建议添加平台特定的错误处理和默认值。
useEffect(() => { const element = pickerRollerRef.current if (element && web()) { const computedStyle = getComputedStyle(element) const currentLineSpacing = computedStyle.getPropertyValue( '--nutui-picker-item-height' ) - !!currentLineSpacing && - (lineSpacing.current = parseFloat(currentLineSpacing)) + if (!currentLineSpacing) { + console.warn('在 Taro 环境中未找到 --nutui-picker-item-height CSS 变量') + return + } + const parsedValue = parseFloat(currentLineSpacing) + if (isNaN(parsedValue)) { + console.warn('CSS 变量值格式无效') + return + } + lineSpacing.current = parsedValue } -}, [pickerRollerRef.current]) +}, [])
🧹 Nitpick comments (3)
src/packages/pickerview/utils.ts (2)
1-4: 建议添加单元测试覆盖
momentum函数缺少单元测试覆盖。该函数计算滚动动量，对于滚动体验至关重要，应该确保其行为符合预期。需要我为这个函数生成单元测试代码吗？
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 2-4: src/packages/pickerview/utils.ts#L2-L4
Added lines #L2 - L4 were not covered by tests
6-31: 建议优化样式生成函数
useStyles函数可以通过以下方式优化：
- 使用
useMemo缓存样式对象，避免不必要的重新计算- 为 CSS 变量添加类型定义，提高类型安全性
建议按如下方式重构：
-export const useStyles = ( +export const useStyles = React.useMemo(( touchTime: number, touchDeg: string, scrollDistance: number, lineSpacing: React.MutableRefObject<number>, rotation: number -) => { +) => ({ const getTransitionStyle = (transformValue: string) => ({ transition: `transform ${touchTime}ms cubic-bezier(0.17, 0.89, 0.45, 1)`, transform: transformValue, }) const touchRollerStyle = () => getTransitionStyle(`rotate3d(1, 0, 0, ${touchDeg})`) const touchTiledStyle = () => getTransitionStyle(`translate3d(0, ${scrollDistance}px, 0)`) const rollerStyle = (index: number) => ({ transform: `rotate3d(1, 0, 0, ${-rotation * (index + 1)}deg) translate3d(0px, 0px, ${Math.round( lineSpacing.current * 3.2 )}px)`, }) return { touchRollerStyle, touchTiledStyle, rollerStyle } -} +}), [touchTime, touchDeg, scrollDistance, lineSpacing.current, rotation])src/packages/pickerview/pickerroller.tsx (1)
37-41: 建议优化 ref 类型定义当前
ref使用any类型，这降低了类型安全性。建议定义具体的类型：-const rollerRef = useRef<any>(null) -const pickerRollerRef = useRef<any>(null) +const rollerRef = useRef<HTMLDivElement>(null) +const pickerRollerRef = useRef<HTMLDivElement>(null)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/packages/pickerview/pickerroller.taro.tsx(1 hunks)src/packages/pickerview/pickerroller.tsx(1 hunks)src/packages/pickerview/utils.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/pickerview/utils.ts
[warning] 2-4: src/packages/pickerview/utils.ts#L2-L4
Added lines #L2 - L4 were not covered by tests
src/packages/pickerview/pickerroller.tsx
[warning] 69-72: src/packages/pickerview/pickerroller.tsx#L69-L72
Added lines #L69 - L72 were not covered by tests
[warning] 75-79: src/packages/pickerview/pickerroller.tsx#L75-L79
Added lines #L75 - L79 were not covered by tests
[warning] 100-104: src/packages/pickerview/pickerroller.tsx#L100-L104
Added lines #L100 - L104 were not covered by tests
[warning] 107-114: src/packages/pickerview/pickerroller.tsx#L107-L114
Added lines #L107 - L114 were not covered by tests
[warning] 117-129: src/packages/pickerview/pickerroller.tsx#L117-L129
Added lines #L117 - L129 were not covered by tests
[warning] 155-155: src/packages/pickerview/pickerroller.tsx#L155
Added line #L155 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/packages/pickerview/pickerroller.taro.tsx (2)
39-40: 优化 ref 类型定义
useRef使用any类型可能导致类型安全问题。建议添加具体的类型定义：
- const rollerRef = useRef<any>(null) - const pickerRollerRef = useRef<any>(null) + const rollerRef = useRef<HTMLDivElement>(null) + const pickerRollerRef = useRef<HTMLDivElement>(null)
1-260: 建议提升组件的可测试性组件的复杂度较高，建议：
- 将动画和触摸处理逻辑抽离为独立的 hooks
- 添加单元测试覆盖关键功能
- 考虑使用状态机管理复杂的交互状态
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/pickerview/pickerroller.taro.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (5)
src/packages/pickerview/pickerroller.taro.tsx (5)
112-112: 增强事件预防默认行为的处理此问题在之前的评审中已提出，建议仍然需要增加对事件可取消状态的检查。
149-159: 优化 useEffect 依赖项之前的评审中已指出，
useEffect的依赖数组包含pickerRollerRef.current可能导致不必要的重渲染。
174-176: 检查事件选项的影响之前的评审中已指出，使用
once: true选项可能导致事件监听器在触发一次后被移除。
232-232: 修复重复的 key 值之前的评审中已指出，使用
item.value ?? index作为 key 可能导致重复值的问题。Also applies to: 243-243
9-9:Details
✅ Verification successful
修复模块导入路径
导入路径已从
@/utils/use-touch更新为@/hooks/use-touch，但仍需验证此路径是否正确配置。请运行以下脚本验证模块路径：
🏁 Script executed:
#!/bin/bash # 验证 hooks/use-touch 模块是否存在 fd -t f "use-touch.(ts|tsx)" "src/hooks"Length of output: 63
修复模块导入路径
经过验证，
src/hooks/use-touch.ts文件存在，因此更新后的导入路径@/hooks/use-touch已正确配置，不再需要修改。请继续关注此模块在对应平台上的使用情况。
2 participants
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新功能
文档
样式