fix(elevator): v15 适配 & 选中值支持受控 #3210
Conversation
## Walkthrough
本次变更为 Elevator 组件及其相关演示、样式和文档带来了多项更新。Elevator 组件新增了 `mode` 属性（支持 `horizontal` 和 `vertical`），并调整了 sticky 头部的渲染逻辑和子项结构，支持不同布局模式。样式（SCSS 和变量）针对新布局做了大量调整，优化了字体、间距、颜色等。文档移除了 `titleHeight` 属性，调整了 `spaceHeight` 默认值。多个演示 demo 的数据结构和渲染方式也做了相应扩展和优化。
## Changes
| 文件/路径分组 | 变更摘要 |
|---|---|
| `src/packages/elevator/elevator.tsx`, `src/packages/elevator/elevator.taro.tsx` | Elevator 组件新增 `mode` 属性（支持 horizontal/vertical），默认值为 horizontal，调整 sticky 头部渲染与子项结构，优化滚动与高度计算逻辑。 |
| `src/packages/elevator/elevator.scss`, `src/styles/variables.scss`, `src/styles/variables-jmapp.scss`, `src/styles/variables-jrkf.scss` | Elevator 相关样式大幅调整，采用 flex 布局，优化字体、间距、颜色，新增/移除部分变量，支持横向/纵向模式。 |
| `src/packages/elevator/demo.tsx` | 移除每个 Demo 组件外层的白色背景 div，Demo 直接渲染。 |
| `src/packages/elevator/demos/h5/demo1.tsx`, `src/packages/elevator/demos/h5/demo3.tsx`, `src/packages/elevator/demos/h5/demo5.tsx`, `src/packages/elevator/demos/taro/demo1.tsx`, `src/packages/elevator/demos/taro/demo3.tsx`, `src/packages/elevator/demos/taro/demo5.tsx` | Demo 数据结构扩展，分组及省份条目数量增加，ID 与排序调整，部分 Demo 渲染顺序和样式优化。 |
| `src/packages/elevator/demos/h5/demo4.tsx`, `src/packages/elevator/demos/taro/demo4.tsx` | Elevator 组件新增 `mode="vertical"` 属性，调整为纵向模式。 |
| `src/packages/elevator/doc.md`, `src/packages/elevator/doc.en-US.md`, `src/packages/elevator/doc.taro.md`, `src/packages/elevator/doc.zh-TW.md` | 文档移除 `titleHeight` 属性，`spaceHeight` 默认值由 23 改为 18，新增 `mode` 属性说明，调整多个 CSS 变量默认值。 |
| `src/types/spec/elevator/base.ts` | 新增 `ElevatorMode` 类型及 `mode` 属性定义，支持 horizontal 和 vertical 两种模式。 |
| `src/sites/sites-react/doc/docs/react/migrate-from-v2.md`, `src/sites/sites-react/doc/docs/taro/migrate-from-v2.md` | 更新迁移文档，新增 `mode` 属性说明，移除 `titleHeight` 属性说明。 |
| `src/packages/elevator/index.ts`, `src/packages/elevator/index.taro.ts` | 类型导出由本地类型改为从外部 `@/types` 引入对应的 `WebElevatorProps` 和 `TaroElevatorProps`，并重命名为 `ElevatorProps`。 |
| `src/packages/elevator/__tests__/elevator.spec.tsx` | 大幅扩展测试覆盖，新增多种模式、交互、边界情况测试，移除未使用的回调函数。 |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant 用户
participant Elevator组件
participant 样式系统
用户->>Elevator组件: 传入 mode (horizontal/vertical)
Elevator组件->>Elevator组件: 根据 mode 切换布局与 sticky 头部渲染
Elevator组件->>样式系统: 应用对应的 CSS 类和变量
样式系统-->>Elevator组件: 返回对应布局样式
Elevator组件-->>用户: 渲染横向或纵向电梯列表Possibly related PRs
Suggested reviewers
Poem
|
github-actions
bot
added
3.x
Codecov ReportAttention: Patch coverage is
❌ Your patch check has failed because the patch coverage (75.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #3210 +/- ##
=============================================
+ Coverage 86.70% 86.87% +0.16%
=============================================
Files 291 291
Lines 19062 19071 +9
Branches 2891 2899 +8
=============================================
+ Hits 16528 16567 +39
+ Misses 2529 2499 -30
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (2)
src/packages/elevator/doc.md (1)
57-67: 🛠️ Refactor suggestion缺少新增的 mode 属性文档
文档中移除了
titleHeight属性，并更新了spaceHeight默认值，但未添加新增的mode属性文档。根据代码实现和演示示例，应添加mode属性说明。建议在属性表中添加以下内容：
| sticky | 索引是否吸顶 | `boolean` | `false` | | showKeys | 展示右侧导航 | `boolean` | `true` | +| mode | 布局模式 | `string` | `'horizontal'` | | spaceHeight | 右侧锚点的上下间距 | `number` | `18` |src/packages/elevator/doc.taro.md (1)
57-67: 🛠️ Refactor suggestion缺少新增的 mode 属性文档
文档中移除了
titleHeight属性，并更新了spaceHeight默认值，但未添加新增的mode属性文档。根据代码实现和演示示例，应添加mode属性说明。建议在属性表中添加以下内容：
| sticky | 索引是否吸顶 | `boolean` | `false` | | showKeys | 展示右侧导航 | `boolean` | `true` | +| mode | 布局模式 | `string` | `'horizontal'` | | spaceHeight | 右侧锚点的上下间距 | `number` | `18` |
🧹 Nitpick comments (9)
src/packages/elevator/demos/h5/demo3.tsx (2)
28-42: 数据结构已更新，但存在重复条目'G' 分组数据已更新，将原来的 "广西" 改为 "甘肃"，并添加了新条目。但请注意，在修改后的数据中出现了两个相同名称 "广东" 的条目（id 32 和 33）。虽然这在技术上是允许的，但可能会让查看示例的用户感到困惑。
建议考虑使用不同的名称，以更好地展示组件的功能。
{ name: '甘肃', id: 31, }, { name: '广东', id: 32, }, { - name: '广东', + name: '广西', id: 33, }, { name: '贵州', id: 34, },
49-67: 'H' 分组中存在重复条目在 'H' 分组中，同样存在重复的条目名称（"湖南"和"湖北"各出现两次）。虽然使用相同名称但不同 id 的方式可以测试组件的某些特性，但这可能不是一个理想的示例展示方式。
建议考虑使用更具差异性的数据来展示组件功能。
{ name: '湖南', id: 41, }, { name: '湖北', id: 42, }, { - name: '湖北', + name: '河北', id: 43, }, { - name: '湖南', + name: '河南', id: 44, }, { name: '海南', id: 45, },src/packages/elevator/demos/h5/demo5.tsx (3)
29-44: 重复数据可能导致混淆
list中出现了两条「广东」记录（id 分别为 32、33），名称相同但 id 不同。
如果组件逻辑依赖 id 唯一性，UI 层面显示则依赖名称可区分度，重复项可能给使用者带来困惑，甚至导致点击回调拿到“错误”的省份。如无业务需求，建议去重或在名称中增加区分信息。
如果确需展示重复名称，请在文档或示例注释中说明原因，避免后续维护者误以为是疏漏。
49-70: 同名省份大量重复，需确认意图在 “H” 分组里，「湖南」与「湖北」均出现两次；
新增的「海南」与前两者同属 H 组但排序置于最后，排序规则不一致。
请确认：
- 是否确实需要同名多条记录。
- 是否有意按 id 而非拼音/名称排序。
- 是否考虑保持地域顺序一致性（例如 “海南” 理应位于 “湖北” 之前）。
如仅为演示效果，可保留，但建议在 demo 顶部添加注释阐明，减少误解。
111-113: 无障碍与语义化建议图标现在位于文字之后并带有
margin-left: 4px，视觉和交互都 OK。
但<Fabulous>组件若纯装饰作用，应添加aria-hidden="true"或role="presentation"，避免屏幕阅读器重复朗读。
此外，行内样式可改为className统一维护，提高可维护性。src/packages/elevator/elevator.tsx (1)
202-208: className 拼接可能产生 'undefined'根节点 className 写法为
`${classPrefix} ${classPrefix}-${mode} ${className}`当外部未传入
className时会得到字符串"nut-elevator nut-elevator-horizontal undefined"，影响样式调试。建议使用
classNames工具或条件拼接：- className={`${classPrefix} ${classPrefix}-${mode} ${className}`} + className={classNames(classPrefix, `${classPrefix}-${mode}`, className)}src/packages/elevator/elevator.taro.tsx (1)
224-225: 根节点 className 多余空格字符串模板拼接两端多出空格，最终渲染结果形如
"nut-elevator custom nut-elevator-horizontal "。
虽不影响功能，但会增加样式排查成本，可改为：-className={`${classPrefix} ${className} ${classPrefix}-${mode} `} +className={classNames(classPrefix, `${classPrefix}-${mode}`, className)}src/packages/elevator/elevator.scss (2)
95-107: 优化点位导航项样式
&-bars-inner-item使用inline-flex和一系列硬编码尺寸（16px），建议考虑将尺寸提取为 SCSS 变量，以便未来主题定制和统一管理。
129-137: 纵向模式专属样式
.nut-elevator-vertical下设置flex-direction:column、分割线和统一内边距，建议后续将公共 padding 合并到基础类以减少重复。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/packages/elevator/demo.tsx(1 hunks)src/packages/elevator/demos/h5/demo1.tsx(2 hunks)src/packages/elevator/demos/h5/demo3.tsx(2 hunks)src/packages/elevator/demos/h5/demo4.tsx(1 hunks)src/packages/elevator/demos/h5/demo5.tsx(3 hunks)src/packages/elevator/demos/taro/demo1.tsx(2 hunks)src/packages/elevator/demos/taro/demo3.tsx(2 hunks)src/packages/elevator/demos/taro/demo4.tsx(1 hunks)src/packages/elevator/demos/taro/demo5.tsx(4 hunks)src/packages/elevator/doc.en-US.md(1 hunks)src/packages/elevator/doc.md(1 hunks)src/packages/elevator/doc.taro.md(1 hunks)src/packages/elevator/doc.zh-TW.md(1 hunks)src/packages/elevator/elevator.scss(3 hunks)src/packages/elevator/elevator.taro.tsx(9 hunks)src/packages/elevator/elevator.tsx(5 hunks)src/styles/variables.scss(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/packages/elevator/elevator.tsx (1)
src/utils/typings.ts (1)
ComponentDefaults(10-13)
src/packages/elevator/elevator.taro.tsx (2)
src/packages/elevator/elevator.tsx (3)
elevatorContext(13-13)ElevatorProps(15-25)Elevator(41-297)src/utils/typings.ts (2)
BasicComponent(3-8)ComponentDefaults(10-13)
🪛 Biome (1.9.4)
src/packages/elevator/elevator.taro.tsx
[error] 108-108: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (40)
src/packages/elevator/demos/taro/demo4.tsx (1)
128-128: 添加了垂直模式支持，符合新功能设计添加的
mode="vertical"属性使组件以垂直布局模式展示，这与 Elevator 组件的新功能设计一致。同时移除了之前的className="test-elevator4"属性，简化了代码结构。src/packages/elevator/doc.md (1)
64-64: 文档中 spaceHeight 属性默认值已更新
spaceHeight默认值从 23 更新为 18，与代码实现保持一致。src/packages/elevator/demo.tsx (1)
37-47: 简化了演示组件结构移除了每个演示组件外层的白色背景容器，直接渲染 Demo 组件，使整体结构更加简洁。这种变更有助于更好地展示新增的垂直模式和水平模式布局差异。
src/packages/elevator/doc.taro.md (1)
64-64: 文档中 spaceHeight 属性默认值已更新
spaceHeight默认值从 23 更新为 18，与代码实现保持一致。src/packages/elevator/demos/h5/demo4.tsx (1)
128-128: 正确使用了新增的mode属性这里正确地使用了新增的
mode="vertical"属性，使示例展示了电梯组件的垂直模式。这与组件的新特性一致，有助于用户理解不同布局模式的使用方法。src/packages/elevator/demos/h5/demo3.tsx (1)
70-78: 增加了新的分组，丰富了示例数据添加了 'L' 分组及其条目，这有助于丰富示例并更全面地测试组件功能，是一个良好的改进。
src/packages/elevator/demos/taro/demo3.tsx (4)
33-42: 数据结构中更新了 'G' 组的省份 ID 和内容'G' 组中的省份结构已更新，新增了两个广东条目（ID 32 和 33）和一个贵州条目（ID 34）。这些更改与新版本的电梯组件布局模式相匹配。
50-67: 'H' 组数据结构更新，扩展了省份列表'H' 组中的湖南和湖北的 ID 已更新（41 和 42），并增加了额外的湖北、湖南和新的海南条目。这种数据扩展支持展示更丰富的内容，更好地测试组件在不同数据量下的表现。
71-76: 新增 'L' 组数据添加了新的 'L' 组，包含辽宁省。这进一步丰富了演示数据，有助于测试组件处理更多分组的能力。
88-94: 移除了测试类名属性注意到组件调用中移除了
className="test-elevator3"属性。这与其他 Taro 演示文件中类似的更改一致，使代码更加简洁。src/packages/elevator/demos/h5/demo1.tsx (4)
28-42: 'G' 组数据结构完全重组'G' 组中的省份数据已完全重组，"广西"被"甘肃"（ID 31）替换，并添加了多个"广东"条目（ID 32、33）和"贵州"（ID 34）。这种更新与 Taro 演示中的变更保持一致，确保了跨平台的数据一致性。
50-67: 'H' 组数据扩展和 ID 更新'H' 组中的省份数据进行了扩展，湖南和湖北的 ID 已更新（41 和 42），并新增了多个条目。这种变更增强了演示的数据丰富度。
71-78: 新增 'L' 组数据添加了新的 'L' 组数据，包含辽宁省。这种新增的数据组更好地测试组件对多种分组的处理能力。
80-97: 新增并扩展了 'S' 组数据新增了 'S' 组，包含山东、山西、上海和陕西多个省市。这一变更与 Taro 演示保持一致，提供了更全面的测试数据。
src/packages/elevator/demos/taro/demo1.tsx (2)
28-42: 'G' 组数据结构全面更新'G' 组的数据结构已全面更新，使用"甘肃"（ID 31）替换了原来的"广西"，并添加了多个"广东"条目和"贵州"。这种数据更新与 H5 版本的 demo1 保持一致，确保了跨平台体验的一致性。
50-67: 'H' 组数据扩展和重组'H' 组的数据已经扩展和重组，包括更新的 ID 和新增的条目。这些变更提供了更丰富的演示数据。
src/packages/elevator/demos/taro/demo5.tsx (4)
4-4: 新增 Text 组件导入增加了
Text组件的导入，用于改进内容渲染结构。这与组件新的布局模式要求一致。
30-44: 'G' 组数据更新和扩展'G' 组数据已更新，用"甘肃"替换了"广西"，并添加了新条目。这与其他演示文件中的变更保持一致。
52-69: 'H' 组数据扩展'H' 组数据已扩展，包括更新的 ID 和新增的条目。这种变更与其他演示文件一致。
113-114: 优化了子元素渲染顺序和样式更改了
Elevator.Context.Consumer中子元素的渲染顺序，现在先渲染文本内容，然后是图标，并调整了图标的边距和高度。这种调整更符合常见的文本+图标布局习惯，提高了可读性。- <Fabulous style={{ marginLeft: '15px' }} size={12} /> - <span style={{ marginLeft: '15px' }}>{value?.name}</span> + <Text>{value?.name}</Text> + <Fabulous style={{ marginLeft: '4px' }} height={12} />src/packages/elevator/elevator.tsx (1)
226-252: 子列表增加包裹节点后样式需同步新增
.nut-elevator-list-item-sublist容器后，原有 SCSS 若未同步更新，可能导致：
- Flex / margin 失效，子项换行异常
- 旧选择器
.nut-elevator-list-item > *覆盖不到新结构请确认
elevator.scss已加入对应样式。src/packages/elevator/elevator.taro.tsx (1)
123-147:await calculateHeight()实际未生效受上条问题影响，这里的
await不会等待高度数组准备完成，仍存在取不到高度的竞态条件。
修复calculateHeight返回值后，此处逻辑即可正常工作；请务必同时将scrollTo的错误边界判断与listHeight写入逻辑自测一遍。src/packages/elevator/elevator.scss (10)
3-4: 新增主容器 Flex 布局
此处将.nut-elevator设置为display:flex及width:100%，符合水平/垂直模式的通用布局要求。
9-10: 优化列表容器 Flex 布局
&-list采用display:flex; width:100%以撑满父容器，适配横纵两种模式。
14-17: 配置列表内部子容器 Flex Column
&-list-inner使用flex-direction:column和min-height:100%以保证滚动区域完整性和 Sticky 头部体验。
23-27: 添加子列表 Flex 布局
&-list-item及其子&-sublist使用display:flex和flex-direction:column，结构清晰，可考虑在后续 demo 中验证多层嵌套效果。
30-31: 对码元居中对齐
&-code添加align-items:center保证左侧字母/数字垂直居中，体验更好。
41-42: 统一列表名称字体和颜色
&-name的font-size与color已对齐到变量设置，无需修改。
55-63: 固定标题增强可读性
&-list-fixed设置display:flex; align-items:center; width:100%; background-color，提升 Sticky 标题在两种模式下的表现。
76-84: 当前码元高亮样式
&-code-current用display:flex; align-items:center; justify-content:center保证高亮圈内文本水平垂直居中，并设置color。
111-116: 平台条件样式无误
&-bars-inner-item-active根据weapp jd和非 JD 平台分别使用渐变或纯色，确保跨平台一致性。
121-126: 横向模式专属样式
在.nut-elevator-horizontal下调整.nut-elevator-list-item-code宽度并居中，布局逻辑清晰。src/styles/variables.scss (8)
2229-2229: 设置列表字体大小
$elevator-list-font-size默认使用$font-size-base，与主题变量一致。
2233-2234: 更新列表项内边距
将$elevator-list-item-padding调整为0 36px 0 20px，满足横纵模式下的视觉间距。
2237-2238: 调整名称行高
$elevator-list-item-name-height更新为34px，与子项对齐后视觉更平衡。
2254-2255: 同步码元高度
$elevator-list-item-code-height修改为34px，保证与名称行高一致。
2257-2259: 新增码元宽度变量
$elevator-list-item-code-width设置为34px，与高度匹配正方形外观。
2304-2306: 调整导航字体尺寸
$elevator-bars-font-size使用$font-size-xxs，与视觉稿匹配。
2307-2309: 更新导航默认颜色
$elevator-bars-color设置为$color-text-help，确保默认状态颜色弱化。
2311-2313: 更新导航激活颜色
$elevator-bars-active-color改为$white，结合高亮背景更具对比度。
| }, | ||
| { | ||
| name: '湖北', | ||
| id: 42, | ||
| }, | ||
| { | ||
| name: '湖北', | ||
| id: 6, | ||
| id: 43, | ||
| }, | ||
| { | ||
| name: '湖南', | ||
| id: 44, | ||
| }, | ||
| { | ||
| name: '海南', | ||
| id: 45, | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| title: 'L', | ||
| list: [ | ||
| { | ||
| name: '辽宁', | ||
| id: 51, | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| title: 'S', | ||
| list: [ | ||
| { | ||
| name: '山东', | ||
| id: 51, | ||
| }, | ||
| { | ||
| name: '山西', | ||
| id: 52, | ||
| }, | ||
| { | ||
| name: '上海', | ||
| id: 53, | ||
| }, | ||
| { | ||
| name: '河南', | ||
| id: 7, | ||
| name: '陕西', | ||
| id: 54, | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
需注意 ID 重复问题
在数据结构中发现了重复的 ID：山东（ID 51）和辽宁（ID 51）使用了相同的 ID 值。这可能在某些操作中导致意外行为，例如在点击回调中通过 ID 识别具体项目时。
建议修复重复的 ID 问题：
- id: 51,
+ id: 61,🏁 Script executed:
#!/bin/bash
# 检查所有 demo 文件中是否存在重复的 ID
grep -r "id: 51" src/packages/elevator/demos/Length of output: 527
需修复：多处演示文件中存在 ID 重复
根据检查结果，id: 51 在以下文件中出现多次，会导致通过 ID 定位项目时产生歧义，请统一调整为唯一值（可参考 GB/T 2260 省级行政区划代码或按业务需求自定义）：
- src/packages/elevator/demos/h5/demo1.tsx （2 处）
- src/packages/elevator/demos/h5/demo3.tsx
- src/packages/elevator/demos/h5/demo5.tsx
- src/packages/elevator/demos/taro/demo1.tsx （2 处）
- src/packages/elevator/demos/taro/demo3.tsx
- src/packages/elevator/demos/taro/demo5.tsx
示例修复（以 h5/demo1.tsx 为例）：
--- a/src/packages/elevator/demos/h5/demo1.tsx
@@ -55,7 +55,7 @@
{
title: 'L',
list: [
- { name: '辽宁', id: 51 },
+ { name: '辽宁', id: 21 }, // 官方省份编码：辽宁 = 21
],
},
{
@@ -62,7 +62,7 @@
{
title: 'S',
list: [
- { name: '山东', id: 51 },
+ { name: '山东', id: 37 }, // 官方省份编码：山东 = 37
{ name: '山西', id: 52 },
{ name: '上海', id: 53 },
{ name: '陕西', id: 54 },📝 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.
| }, | |
| { | |
| name: '湖北', | |
| id: 42, | |
| }, | |
| { | |
| name: '湖北', | |
| id: 6, | |
| id: 43, | |
| }, | |
| { | |
| name: '湖南', | |
| id: 44, | |
| }, | |
| { | |
| name: '海南', | |
| id: 45, | |
| }, | |
| ], | |
| }, | |
| { | |
| title: 'L', | |
| list: [ | |
| { | |
| name: '辽宁', | |
| id: 51, | |
| }, | |
| ], | |
| }, | |
| { | |
| title: 'S', | |
| list: [ | |
| { | |
| name: '山东', | |
| id: 51, | |
| }, | |
| { | |
| name: '山西', | |
| id: 52, | |
| }, | |
| { | |
| name: '上海', | |
| id: 53, | |
| }, | |
| { | |
| name: '河南', | |
| id: 7, | |
| name: '陕西', | |
| id: 54, | |
| }, | |
| { | |
| title: 'L', | |
| list: [ | |
| { | |
| name: '辽宁', | |
| id: 21, // 官方省份编码：辽宁 = 21 | |
| }, | |
| ], | |
| }, | |
| { | |
| title: 'S', | |
| list: [ | |
| { | |
| name: '山东', | |
| id: 37, // 官方省份编码：山东 = 37 | |
| }, | |
| { | |
| name: '山西', | |
| id: 52, | |
| }, | |
| { | |
| name: '上海', | |
| id: 53, | |
| }, | |
| { | |
| name: '陕西', | |
| id: 54, | |
| }, | |
| ], | |
| }, |
| title: 'L', | ||
| list: [ | ||
| { | ||
| name: '辽宁', | ||
| id: 51, | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| title: 'S', | ||
| list: [ | ||
| { | ||
| name: '山东', | ||
| id: 51, | ||
| }, | ||
| { | ||
| name: '山西', | ||
| id: 52, | ||
| }, | ||
| { | ||
| name: '上海', | ||
| id: 53, | ||
| }, | ||
| { | ||
| name: '河南', | ||
| id: 7, | ||
| name: '陕西', | ||
| id: 54, | ||
| }, |
There was a problem hiding this comment.
新增 'L' 和 'S' 组数据
新增了 'L' 和 'S' 组数据，包含多个省市。这一变更与 H5 演示保持一致，但存在与 H5 demo1 相同的 ID 重复问题。
山东（ID 51）和辽宁（ID 51）使用了相同的 ID，这可能导致在回调函数中难以区分这两个不同的条目。建议修复：
- id: 51,
+ id: 61,📝 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.
| title: 'L', | |
| list: [ | |
| { | |
| name: '辽宁', | |
| id: 51, | |
| }, | |
| ], | |
| }, | |
| { | |
| title: 'S', | |
| list: [ | |
| { | |
| name: '山东', | |
| id: 51, | |
| }, | |
| { | |
| name: '山西', | |
| id: 52, | |
| }, | |
| { | |
| name: '上海', | |
| id: 53, | |
| }, | |
| { | |
| name: '河南', | |
| id: 7, | |
| name: '陕西', | |
| id: 54, | |
| }, | |
| title: 'L', | |
| list: [ | |
| { | |
| name: '辽宁', | |
| id: 51, | |
| }, | |
| ], | |
| }, | |
| { | |
| title: 'S', | |
| list: [ | |
| { | |
| name: '山东', | |
| id: 61, | |
| }, | |
| { | |
| name: '山西', | |
| id: 52, | |
| }, | |
| { | |
| name: '上海', | |
| id: 53, | |
| }, | |
| { | |
| name: '陕西', | |
| id: 54, | |
| }, | |
| ], | |
| }, |
| title: 'S', | ||
| list: [ | ||
| { | ||
| name: '山东', | ||
| id: 51, | ||
| }, | ||
| { | ||
| name: '山西', | ||
| id: 52, | ||
| }, | ||
| { | ||
| name: '上海', | ||
| id: 53, | ||
| }, | ||
| { | ||
| name: '河南', | ||
| id: 7, | ||
| name: '陕西', | ||
| id: 54, | ||
| }, |
There was a problem hiding this comment.
新增 'S' 组数据
新增了 'S' 组，包含多个省市。这与其他演示文件中的变更保持一致，但同样存在 ID 重复问题。
山东使用了与辽宁相同的 ID（51），建议修复：
- id: 51,
+ id: 61,📝 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.
| title: 'S', | |
| list: [ | |
| { | |
| name: '山东', | |
| id: 51, | |
| }, | |
| { | |
| name: '山西', | |
| id: 52, | |
| }, | |
| { | |
| name: '上海', | |
| id: 53, | |
| }, | |
| { | |
| name: '河南', | |
| id: 7, | |
| name: '陕西', | |
| id: 54, | |
| }, | |
| title: 'S', | |
| list: [ | |
| { | |
| name: '山东', | |
| - id: 51, | |
| + id: 61, | |
| }, | |
| { | |
| name: '山西', | |
| id: 52, | |
| }, | |
| { | |
| name: '上海', | |
| id: 53, | |
| }, | |
| { | |
| name: '陕西', | |
| id: 54, | |
| }, | |
| ], |
| return ( | ||
| <div className={`${classPrefix} ${className}`} style={style} {...rest}> | ||
| {sticky && scrollY > 0 ? ( | ||
| <div | ||
| className={`${classPrefix} ${classPrefix}-${mode} ${className}`} | ||
| style={style} | ||
| {...rest} | ||
| > | ||
| {mode === 'vertical' && sticky && scrollY > 0 ? ( |
There was a problem hiding this comment.
未移除 scroll 事件监听，存在内存泄漏风险
useEffect 中向 listview.current 绑定了 scroll 事件，但在组件卸载或 listview.current 变更时没有执行 removeEventListener。
这会导致：
- 组件多次挂载/卸载后事件叠加，性能下降。
- 某些场景下产生无法释放的闭包引用。
建议在 useEffect 返回清理函数，同时依赖数组使用 []（ref 对象本身不会变化）：
- useEffect(() => {
- if (listview.current) {
- setListGroup()
- listview.current.addEventListener('scroll', listViewScroll)
- }
- }, [listview])
+ useEffect(() => {
+ if (listview.current) {
+ setListGroup()
+ const current = listview.current
+ current.addEventListener('scroll', listViewScroll)
+ return () => {
+ current.removeEventListener('scroll', listViewScroll)
+ }
+ }
+ }, [])Committable suggestion skipped: line range outside the PR's diff.
| const listViewScroll = (e: any) => { | ||
| const { listHeight } = state.current | ||
| if (!listHeight.length) { | ||
| calculateHeight() | ||
| } | ||
| const target = e.target as Element | ||
| const { scrollTop } = target | ||
| state.current.scrollY = Math.floor(scrollTop) | ||
| Taro.getEnv() === 'WEB' && setScrollTop(scrollTop) | ||
| if (sticky && scrollTop !== scrollY) { | ||
| setScrollY(Math.floor(scrollTop) > 0 ? 1 : 0) | ||
| } | ||
| if (scrolling.current) return | ||
| for (let i = 0; i < listHeight.length - 1; i++) { | ||
| const height1 = listHeight[i] | ||
| const height2 = listHeight[i + 1] | ||
| if (state.current.scrollY >= height1 && state.current.scrollY < height2) { | ||
| return setCodeIndex(i) | ||
| raf(() => { | ||
| const { listHeight } = state.current | ||
| if (!listHeight.length) { | ||
| calculateHeight() | ||
| } | ||
| } | ||
| const target = e.target as Element | ||
| const { scrollTop } = target | ||
| state.current.scrollY = Math.floor(scrollTop) | ||
| Taro.getEnv() === 'WEB' && setScrollTop(scrollTop) | ||
| if (mode === 'vertical' && sticky) { | ||
| setScrollY(Math.floor(scrollTop) > 0 ? 1 : 0) | ||
| } | ||
| if (scrolling.current) return | ||
|
|
||
| const index = listHeight.findIndex( | ||
| (height, i) => | ||
| state.current.scrollY >= height && | ||
| state.current.scrollY < (listHeight[i + 1] || Infinity) | ||
| ) | ||
|
|
||
| if (index !== -1 && index !== codeIndex) { | ||
| setCodeIndex(index) | ||
| } | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
滚动节流逻辑仍可能读取过期数据
raf 回调里若 state.current.listHeight 为空时触发 calculateHeight()，同样由于异步执行，在后续 findIndex 判断前未必完成写入，可能返回 -1。
可在 calculateHeight 完成后再执行后续逻辑，或在获取高度失败时提前 return，以保证索引计算准确。
| state.current.listHeight = [0] | ||
|
|
||
| state.current.listHeight.push(height) | ||
| for (let i = 0; i < state.current.listGroup.length; i++) { | ||
| const query = createSelectorQuery() | ||
| query | ||
| .selectAll(`.${classPrefix}-${uuid} .nut-elevator-item-${i}`) | ||
| .boundingClientRect() | ||
| // eslint-disable-next-line no-loop-func | ||
| query.exec((res: any) => { | ||
| if (res[0][0]) height += res[0][0].height | ||
| state.current.listHeight.push(height) | ||
| const query = createSelectorQuery() | ||
| query | ||
| .selectAll(`#${classPrefix}-${uuid} .${classPrefix}-list-item`) | ||
| .boundingClientRect() | ||
| .exec((rect = []) => { | ||
| if (rect[0] && rect[0].length) { | ||
| // rect[0] = rect[0].reverse() | ||
| state.current.listHeight = rect[0].reduce( | ||
| (acc: any[], item: { height: any }, index: number) => { | ||
| // 当前项的高度等于前面所有项的高度之和 | ||
| const height = acc[index] + item.height | ||
| acc.push(height) | ||
| return acc | ||
| }, | ||
| [0] | ||
| ) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
calculateHeight 为异步回调，但未返回 Promise
函数内部 createSelectorQuery().exec() 异步回调完成后才写入 state.current.listHeight，
然而函数本身没有 return Promise，导致调用方无法感知完成时机。
立即在下方 await calculateHeight() 等待并不会真正阻塞执行，scrollTo 里仍然可能读取到空数组，出现定位错误。
建议改造成返回 Promise<void>，示例 diff 如下：
- const calculateHeight = () => {
- state.current.listHeight = [0]
- const query = createSelectorQuery()
- ...
- .exec((rect = []) => {
- ...
- })
- }
+ const calculateHeight = (): Promise<void> =>
+ new Promise((resolve) => {
+ state.current.listHeight = [0]
+ createSelectorQuery()
+ .selectAll(`#${classPrefix}-${uuid} .${classPrefix}-list-item`)
+ .boundingClientRect()
+ .exec((rect = []) => {
+ if (rect[0]?.length) {
+ state.current.listHeight = rect[0].reduce(
+ (acc: number[], item: { height: number }, idx: number) => {
+ acc.push(acc[idx] + item.height)
+ return acc
+ },
+ [0]
+ )
+ }
+ resolve()
+ })
+ })📝 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.
| state.current.listHeight = [0] | |
| state.current.listHeight.push(height) | |
| for (let i = 0; i < state.current.listGroup.length; i++) { | |
| const query = createSelectorQuery() | |
| query | |
| .selectAll(`.${classPrefix}-${uuid} .nut-elevator-item-${i}`) | |
| .boundingClientRect() | |
| // eslint-disable-next-line no-loop-func | |
| query.exec((res: any) => { | |
| if (res[0][0]) height += res[0][0].height | |
| state.current.listHeight.push(height) | |
| const query = createSelectorQuery() | |
| query | |
| .selectAll(`#${classPrefix}-${uuid} .${classPrefix}-list-item`) | |
| .boundingClientRect() | |
| .exec((rect = []) => { | |
| if (rect[0] && rect[0].length) { | |
| // rect[0] = rect[0].reverse() | |
| state.current.listHeight = rect[0].reduce( | |
| (acc: any[], item: { height: any }, index: number) => { | |
| // 当前项的高度等于前面所有项的高度之和 | |
| const height = acc[index] + item.height | |
| acc.push(height) | |
| return acc | |
| }, | |
| [0] | |
| ) | |
| } | |
| }) | |
| } | |
| } | |
| const calculateHeight = (): Promise<void> => | |
| new Promise((resolve) => { | |
| state.current.listHeight = [0] | |
| createSelectorQuery() | |
| .selectAll(`#${classPrefix}-${uuid} .${classPrefix}-list-item`) | |
| .boundingClientRect() | |
| .exec((rect = []) => { | |
| if (rect[0]?.length) { | |
| state.current.listHeight = rect[0].reduce( | |
| (acc: number[], item: { height: number }, idx: number) => { | |
| acc.push(acc[idx] + item.height) | |
| return acc | |
| }, | |
| [0] | |
| ) | |
| } | |
| resolve() | |
| }) | |
| }) |
🧰 Tools
🪛 Biome (1.9.4)
[error] 108-108: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/packages/elevator/doc.md (3)
79-79: 建议补充内边距调整原因说明
--nutui-elevator-list-item-padding从旧的值更新为0 36px 0 20px，建议在文档中简要说明此调整是为适配横向/竖向模式下的布局需求，以便使用者快速理解。
80-80: 补充列表项高度修改背景信息
--nutui-elevator-list-item-name-height默认值变更为34px，建议在文档中标注此修改适配的场景（如文字大小或行高更新），以提高可维护性。
92-92: 确认导航栏定位变量的更新逻辑
--nutui-elevator-bars-right默认值更新为16px，建议检查在横、竖两种模式下，此定位值是否符合预期，并在文档中区分不同模式的定位差异。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/packages/elevator/doc.en-US.md(2 hunks)src/packages/elevator/doc.md(2 hunks)src/packages/elevator/doc.taro.md(2 hunks)src/packages/elevator/doc.zh-TW.md(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/packages/elevator/doc.zh-TW.md
- src/packages/elevator/doc.en-US.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/elevator/doc.taro.md
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (5)
src/packages/elevator/doc.md (5)
64-64: 请确认 spaceHeight 默认值与组件实现保持一致并补充单位说明
文档中将spaceHeight默认值调整为18，建议检查elevator.tsx或相关类型声明里是否也同步更新为18，并在文档中明确该值单位（如px或 数值对应的默认单位）。
77-77: 校验列表项字体大小变量的实际默认值
文档将--nutui-elevator-list-font-size的默认值改为$font-size-base，请确认在variables.scss中该 CSS 变量的默认值是否已修改，并与文档保持一致。
86-86: 校验提示背景色变量的实际效果
文档将--nutui-elevator-list-item-code-current-bg-color默认值设置为$color-text-disabled，请在组件样式中预览该配色在深色/浅色主题下的可读性，确保无色彩对比度问题。
95-95: 验证激活态文字颜色与主题一致性
--nutui-elevator-bars-active-color的默认值改为$white，请确认在深色背景或主题色背景下，该激活文字的可见性与设计规范是否一致。
98-98: 检查导航文字大小更新的合理性
文档将--nutui-elevator-bars-font-size调整为$font-size-xxs，建议与设计稿或视觉规范对齐后再确认该字号是否适合各终端设备的可读性。
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/packages/elevator/elevator.taro.tsx (1)
109-132: 🛠️ Refactor suggestion适当处理异步高度计算
将scrollTo方法改为async并添加await calculateHeight()是一个好的改进，但由于calculateHeight没有返回Promise，这个await并不会真正阻塞执行。
另外，增加了基于mode的条件判断
if (mode === 'vertical' && sticky)来控制滚动行为，这很好地支持了不同模式的差异化处理。除了修复calculateHeight返回Promise外，建议在此方法中增加错误处理：
const scrollTo = async (index: number) => { if (!index && index !== 0) { return } if (!state.current.listHeight.length) { await calculateHeight() + // 检查计算后的高度列表是否有效 + if (!state.current.listHeight.length) { + console.warn('Failed to calculate list heights') + return + } } // 后续代码... }
♻️ Duplicate comments (2)
src/packages/elevator/elevator.taro.tsx (2)
86-107:⚠️ Potential issue优化高度计算方法
calculateHeight方法改进了列表高度的计算，通过一次性查询所有列表项并使用reduce计算累积高度，替代了之前逐项查询的方式，提高了性能。
但仍存在一个问题：该方法没有返回Promise，导致后续的
await calculateHeight()不会真正起到等待作用。建议将函数改造为返回Promise：
- const calculateHeight = () => { + const calculateHeight = (): Promise<void> => new Promise((resolve) => { state.current.listHeight = [0] const query = createSelectorQuery() query .selectAll(`#${classPrefix}-${uuid} .${classPrefix}-list-item`) .boundingClientRect() .exec((rect = []) => { if (rect[0] && rect[0].length) { state.current.listHeight = rect[0].reduce( (acc: any[], item: { height: any }, index: number) => { const height = acc[index] + item.height acc.push(height) return acc }, [0] ) } + resolve() }) + })🧰 Tools
🪛 Biome (1.9.4)
[error] 94-94: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
169-193:⚠️ Potential issue优化滚动处理逻辑
使用
raf工具进行滚动节流是很好的性能优化，但当listHeight为空时触发calculateHeight()后，由于异步执行，在后续的findIndex判断前未必完成写入，可能返回-1。建议在获取高度失败时提前返回，以保证索引计算准确：
const listViewScroll = (e: any) => { raf(() => { const { listHeight } = state.current if (!listHeight.length) { calculateHeight() + return // 等待下一帧再处理 } // 后续代码... const index = listHeight.findIndex( (height, i) => state.current.scrollY >= height && state.current.scrollY < (listHeight[i + 1] || Infinity) ) if (index !== -1 && index !== codeIndex) { setCodeIndex(index) } }) }
🧹 Nitpick comments (1)
src/packages/elevator/elevator.taro.tsx (1)
94-94: 优化对象安全访问使用可选链操作符可以简化代码并提高安全性。
- if (rect[0] && rect[0].length) { + if (rect[0]?.length) {🧰 Tools
🪛 Biome (1.9.4)
[error] 94-94: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/packages/elevator/doc.en-US.md(2 hunks)src/packages/elevator/doc.md(2 hunks)src/packages/elevator/doc.taro.md(2 hunks)src/packages/elevator/doc.zh-TW.md(2 hunks)src/packages/elevator/elevator.taro.tsx(10 hunks)src/packages/elevator/elevator.tsx(6 hunks)src/sites/sites-react/doc/docs/react/migrate-from-v2.md(1 hunks)src/sites/sites-react/doc/docs/taro/migrate-from-v2.md(1 hunks)src/types/spec/elevator/base.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/sites/sites-react/doc/docs/taro/migrate-from-v2.md
- src/sites/sites-react/doc/docs/react/migrate-from-v2.md
🚧 Files skipped from review as they are similar to previous changes (5)
- src/packages/elevator/doc.md
- src/packages/elevator/doc.en-US.md
- src/packages/elevator/elevator.tsx
- src/packages/elevator/doc.taro.md
- src/packages/elevator/doc.zh-TW.md
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/packages/elevator/elevator.taro.tsx (3)
src/packages/elevator/elevator.tsx (2)
elevatorContext(14-14)Elevator(27-283)src/types/spec/elevator/base.ts (1)
ElevatorItem(4-9)src/utils/typings.ts (1)
ComponentDefaults(10-13)
🪛 Biome (1.9.4)
src/packages/elevator/elevator.taro.tsx
[error] 94-94: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (10)
src/types/spec/elevator/base.ts (2)
18-18: 添加ElevatorMode类型支持水平和垂直模式这个新增的类型定义很好地支持了Elevator组件的布局模式扩展，明确限制了可选值为'horizontal'和'vertical'。
21-21: BaseElevator接口正确添加了mode属性将mode属性添加到BaseElevator接口中，使其成为必填项，确保了类型安全和一致性。
src/packages/elevator/elevator.taro.tsx (8)
12-16: 导入必要的工具和类型导入的工具和类型与组件需求一致，尤其是添加的
raf工具可以优化滚动性能，ElevatorItem和TaroElevatorProps类型确保了类型安全。
18-18: 上下文类型明确为ElevatorItem将createContext的泛型类型从空对象更改为ElevatorItem，提高了类型安全性和代码可读性。
20-29: 更新默认属性配置默认属性添加了
mode: 'horizontal'，并适当调整了spaceHeight的默认值，与整体设计保持一致。
65-67: 状态类型更新为ElevatorItem将currentData的类型从通用对象更新为具体的ElevatorItem类型，提高了类型安全性。
208-214: 根元素类名中添加模式区分将模式（horizontal/vertical）添加到根元素的类名中，有助于应用特定于模式的样式，使组件更灵活。
216-227: 优化ScrollView配置添加
enhanced属性并显式禁用滚动动画（scrollWithAnimation={false}）可以提高滚动性能，特别是在复杂列表中。
237-262: 列表项子列表结构优化子列表的结构优化得当，使用ElevatorItem类型提高了类型安全性，保持了代码一致性。
306-312: 基于模式条件渲染固定标题仅在垂直模式下且启用sticky时显示固定标题，符合垂直模式下的用户体验预期，避免在水平模式下不必要的元素渲染。
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/packages/elevator/index.taro.ts(1 hunks)src/packages/elevator/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: test
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/packages/elevator/elevator.tsx (1)
189-194:⚠️ Potential issue未移除 scroll 事件监听，存在内存泄漏风险
useEffect中向listview.current绑定了scroll事件，但在组件卸载或listview.current变更时没有执行removeEventListener。
这会导致：
- 组件多次挂载/卸载后事件叠加，性能下降。
- 某些场景下产生无法释放的闭包引用。
建议在
useEffect返回清理函数，同时依赖数组使用[]（ref 对象本身不会变化）：- useEffect(() => { - if (listview.current) { - setListGroup() - listview.current.addEventListener('scroll', listViewScroll) - } - }, [listview]) + useEffect(() => { + if (listview.current) { + setListGroup() + const current = listview.current + current.addEventListener('scroll', listViewScroll) + return () => { + current.removeEventListener('scroll', listViewScroll) + } + } + }, [])
🧹 Nitpick comments (2)
src/packages/elevator/elevator.tsx (2)
11-14: 类型定义更新改进了代码的可维护性代码将类型定义从本地移动到外部的
@/types文件，并使用了ElevatorItem和WebElevatorProps类型。这种做法改进了代码组织结构，使类型在整个项目中可以被重用。建议：
- 考虑在文件中添加对这些类型定义的简短注释，以提高代码的可读性。
- 确保这些导入的类型已经在
@/types中完整定义，并且与组件需求保持一致。Also applies to: 26-26, 28-28, 31-31, 61-63
124-127: 拖拽事件处理优化但缺少测试覆盖在 onDragStart 中添加了对无效索引的早期返回处理，这是一个很好的防御性编程实践。但根据静态分析，这段代码也缺少测试覆盖。
建议：
- 添加相关测试案例，确保这种边缘情况能被正确处理
- 考虑记录一条调试日志，以便在开发模式下观察这种情况的发生
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 124-127: src/packages/elevator/elevator.tsx#L124-L127
Added lines #L124 - L127 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/elevator/elevator.tsx(8 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/elevator/elevator.tsx
[warning] 80-83: src/packages/elevator/elevator.tsx#L80-L83
Added lines #L80 - L83 were not covered by tests
[warning] 124-127: src/packages/elevator/elevator.tsx#L124-L127
Added lines #L124 - L127 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (2)
src/packages/elevator/elevator.tsx (2)
18-18: 新增的 mode 属性增强了组件的灵活性新增的
mode属性（默认值为 'horizontal'）允许组件在水平和垂直两种布局模式下工作，这大大增强了组件的适用性。根据 mode 的值，组件的 class 名和固定标题的渲染逻辑也相应地进行了调整。建议：
- 确保为这两种模式添加适当的样式规则
- 在组件文档中明确说明这两种模式的用途和区别
Also applies to: 198-198, 202-208
220-221: 改进了子列表的结构和样式将子列表项从 React Fragment 更改为带有类名
${classPrefix}-list-item-sublist的 div，这有助于应用特定样式并提高结构的清晰度。建议：
- 确保相应的 CSS 样式已经定义，以便正确显示子列表
- 考虑为这个子列表容器添加适当的语义角色（aria role）以提高可访问性
Also applies to: 245-245
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/packages/elevator/elevator.tsx (1)
189-194:⚠️ Potential issue事件监听存在内存泄漏风险
useEffect中向listview.current绑定了scroll事件，但在组件卸载时没有移除事件监听，这可能导致内存泄漏。建议修改为：
-useEffect(() => { - if (listview.current) { - setListGroup() - listview.current.addEventListener('scroll', listViewScroll) - } -}, [listview]) +useEffect(() => { + if (listview.current) { + setListGroup() + const current = listview.current + current.addEventListener('scroll', listViewScroll) + return () => { + current.removeEventListener('scroll', listViewScroll) + } + } +}, []) // 依赖为空数组，因为ref对象本身不会变化
♻️ Duplicate comments (1)
src/packages/elevator/elevator.taro.tsx (1)
169-193:⚠️ Potential issue滚动处理逻辑优化但存在潜在问题
使用
raf进行滚动事件节流是一个好的优化，但在回调中可能存在竞态条件：当listHeight为空时调用calculateHeight()，但由于该函数是异步执行的，可能在后续的findIndex判断之前尚未完成，可能导致返回-1的索引。建议修改为：
raf(() => { const { listHeight } = state.current if (!listHeight.length) { calculateHeight() + return // 如果需要计算高度，则先返回，等待下一帧 } const target = e.target as Element const { scrollTop } = target state.current.scrollY = Math.floor(scrollTop) // 其他代码... })或者将
calculateHeight函数改造为返回Promise，以便能够等待计算完成：-const calculateHeight = () => { +const calculateHeight = (): Promise<void> => + new Promise((resolve) => { state.current.listHeight = [0] const query = createSelectorQuery() query .selectAll(`#${classPrefix}-${uuid} .${classPrefix}-list-item`) .boundingClientRect() .exec((rect = []) => { if (rect[0]?.length) { state.current.listHeight = rect[0].reduce( (acc: any[], item: { height: any }, index: number) => { const height = acc[index] + item.height acc.push(height) return acc }, [0] ) } + resolve() }) + })
🧹 Nitpick comments (1)
src/packages/elevator/elevator.taro.tsx (1)
86-106: 优化高度计算逻辑但存在潜在问题使用单个选择器查询获取所有列表项，提高了性能。但在第94行的条件判断中，建议使用可选链操作符以防止潜在的空引用异常。
-if (rect[0] && rect[0].length) { +if (rect[0]?.length) {🧰 Tools
🪛 Biome (1.9.4)
[error] 94-94: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/packages/address/__test__/__snapshots__/address.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
src/packages/elevator/__tests__/elevator.spec.tsx(1 hunks)src/packages/elevator/elevator.taro.tsx(11 hunks)src/packages/elevator/elevator.tsx(10 hunks)src/types/spec/elevator/base.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types/spec/elevator/base.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/elevator/elevator.taro.tsx
[error] 94-94: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Check: codecov/patch
src/packages/elevator/elevator.tsx
[warning] 80-83: src/packages/elevator/elevator.tsx#L80-L83
Added lines #L80 - L83 were not covered by tests
[warning] 124-127: src/packages/elevator/elevator.tsx#L124-L127
Added lines #L124 - L127 were not covered by tests
[warning] 260-260: src/packages/elevator/elevator.tsx#L260
Added line #L260 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (35)
src/packages/elevator/__tests__/elevator.spec.tsx (10)
133-137: 新增的模式属性测试覆盖了重要功能测试验证了新增的
mode="vertical"属性能正确渲染垂直模式类名，这对确保布局渲染正确很重要。
139-143: showKeys属性测试覆盖了UI条件渲染测试验证了当
showKeys为false时不渲染索引栏，符合预期行为。
145-162: 自定义内容渲染测试实现完善测试验证了通过
children属性实现自定义内容渲染的功能，测试用例结构清晰，断言合理。
164-171: 非数字高度值处理测试有效测试验证了组件对非数字高度值(如"50vh")的正确处理，确保了组件的灵活性。
173-177: 空列表渲染测试覆盖了边缘情况测试验证了当列表为空数组时的渲染行为，确保组件在无数据时不会崩溃。
179-209: 自定义楼层键测试实现完善测试验证了通过
floorKey属性支持自定义标识字段的功能，测试数据结构合理，断言准确。
211-241: 非字符串值渲染测试确保了类型兼容性测试验证了组件对数字类型索引值的正确处理，确保了更广泛的数据兼容性。
243-257: 索引点击传值测试保证了事件回调准确性测试验证了点击索引栏项目时能正确传递索引值到回调函数，确保事件处理机制正常。
259-281: 滚动高亮测试确保了交互一致性测试验证了滚动时索引高亮状态的正确性，使用
act和waitFor正确处理了异步操作。
283-309: 垂直模式下固定头部测试覆盖了关键功能测试验证了垂直模式下启用sticky属性时固定头部的显示逻辑，模拟了滚动事件并正确断言了结果。
src/packages/elevator/elevator.taro.tsx (14)
12-16: 优化引用的类型和工具函数引入了
raf工具函数用于请求动画帧处理，并从@/types导入了更规范的类型定义，提高了代码质量和类型安全性。
18-18: 更新上下文类型定义将上下文类型从
ElevatorData更新为ElevatorItem，与整体类型系统保持一致。
20-29: 新增mode属性并优化默认配置新增了
mode属性支持水平和垂直布局，默认值为'horizontal'，同时调整了spaceHeight默认值为18，使组件更加灵活。
31-33: 更新组件Props类型定义将组件Props类型从
Partial<ElevatorProps>更新为Partial<TaroElevatorProps>，增强了类型系统的一致性。
65-67: 统一状态类型定义将
currentData状态类型更新为ElevatorItem，与其他部分保持一致。
109-132: 滚动函数异步化并增加模式条件将
scrollTo函数改为异步函数并在需要时等待高度计算完成，同时添加了模式条件判断，只在垂直模式下更新滚动Y值，提高了代码的健壮性。
159-163: 更新事件处理函数参数类型将
handleClickItem函数的参数类型更新为ElevatorItem，与新的类型系统保持一致。🧰 Tools
🪛 Biome (1.9.4)
[error] 160-162: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
202-206: 使用nextTick优化初始化时机使用
nextTick确保在组件渲染完成后再计算高度，提高了初始化的可靠性。
209-212: 更新根容器类名以支持不同模式在根容器类名中添加了模式标识
${classPrefix}-${mode}，便于针对不同模式应用样式。
219-221: 优化滚动视图配置添加了
enhanced属性并关闭了滚动动画，提高了滚动性能和用户体验。
237-262: 改进子列表结构将原来的React片段替换为具有类名的div容器，提供了更好的DOM结构和样式控制能力。
277-278: 确保索引键值类型一致性使用
String()确保索引键值渲染为字符串，防止可能的类型错误。
298-299: 确保楼层键值类型一致性使用
String()确保楼层键值渲染为字符串，防止可能的类型错误。
306-312: 优化固定标题的条件渲染只在垂直模式且启用sticky属性时才渲染固定标题，符合新的设计需求。
src/packages/elevator/elevator.tsx (11)
11-12: 优化引用的类型从
@/types导入了更规范的类型定义，提高了代码质量和类型安全性。
14-14: 更新上下文类型定义将上下文类型从
ElevatorData更新为ElevatorItem，与整体类型系统保持一致。
16-25: 新增mode属性并优化默认配置新增了
mode属性支持水平和垂直布局，默认值为'horizontal'，同时调整了spaceHeight默认值为18，使组件更加灵活。
27-29: 更新组件Props类型定义将组件Props类型从
Partial<ElevatorProps>更新为Partial<WebElevatorProps>，增强了类型系统的一致性。
61-63: 统一状态类型定义将
currentData状态类型更新为ElevatorItem，与其他部分保持一致。
145-149: 更新事件处理函数参数类型将
handleClickItem函数的参数类型更新为ElevatorItem，与新的类型系统保持一致。
197-201: 更新根容器类名以支持不同模式在根容器类名中添加了模式标识
${classPrefix}-${mode}，便于针对不同模式应用样式。
202-208: 优化固定标题的条件渲染只在垂直模式且启用sticky属性时才渲染固定标题，符合新的设计需求。
220-245: 改进子列表结构将原来的React片段替换为具有类名的div容器，提供了更好的DOM结构和样式控制能力。
281-282: 确保楼层键值类型一致性使用
String()确保楼层键值渲染为字符串，防止可能的类型错误。
260-261:✅ Verification successful
确保索引键值类型一致性
使用
String()确保索引键值渲染为字符串，防止可能的类型错误，但此行代码缺少测试覆盖。建议添加针对非字符串值的测试用例：
🏁 Script executed:
#!/bin/bash # 检查是否有测试非字符串值渲染的测试用例 grep -r "非字符串" --include="*.spec.tsx" --include="*.test.tsx" src/packages/elevator/Length of output: 196
无需新增非字符串值渲染测试
在
src/packages/elevator/__tests__/elevator.spec.tsx已包含 “测试非字符串值的正确渲染” 用例，覆盖了使用String()强制转换的场景，无需额外添加测试。🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 260-260: src/packages/elevator/elevator.tsx#L260
Added line #L260 was not covered by tests
| if (index < 0) { | ||
| setScrollStart(false) | ||
| return | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
增加拖拽起始点的边界检查但缺少测试覆盖
增加了对索引小于0的情况的处理，防止无效操作，但这部分代码也缺少测试覆盖。
建议添加单元测试覆盖此边界情况，并考虑添加更明确的日志或注释说明此边界条件的处理逻辑。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 124-127: src/packages/elevator/elevator.tsx#L124-L127
Added lines #L124 - L127 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/packages/elevator/__tests__/elevator.spec.tsx (1)
378-394: 列表高度计算测试可以更精确当前测试只验证了列表项的数量，但没有验证高度计算的正确性。建议添加更精确的断言来确保高度计算正确。
// 验证列表项高度计算 expect(listItems.length).toBe(list.length) + // 确保每个列表项都有高度 + listItems.forEach((item) => { + expect(item.clientHeight).toBeGreaterThan(0) + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/packages/elevator/__tests__/elevator.spec.tsx(1 hunks)src/packages/elevator/elevator.tsx(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/elevator/elevator.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/packages/elevator/__tests__/elevator.spec.tsx (1)
src/packages/elevator/elevator.tsx (1)
Elevator(27-287)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (13)
src/packages/elevator/__tests__/elevator.spec.tsx (13)
133-137: 完善了对垂直模式的测试这个测试很好地验证了 Elevator 组件在垂直模式下的渲染行为，确保添加了正确的 CSS 类名。这与组件添加的新
mode属性一致。
139-143: 验证了 showKeys 属性功能这个测试正确验证了当
showKeys设置为 false 时，索引条不会被渲染，这是一个重要的行为验证。
145-162: 自定义内容渲染测试设计合理这个测试很好地验证了使用 children 属性自定义内容的渲染功能。测试中创建了一个自定义组件并验证其内容被正确显示。
164-171: 非数字高度值处理测试有效这个测试正确验证了组件能够处理非数字高度值（如 CSS 单位 "vh"），确保组件在各种使用场景下都能正常工作。
173-177: 空列表处理测试合理这个测试验证了组件在列表为空时的渲染行为，确保组件能够优雅地处理边缘情况。
179-209: 自定义楼层键测试设计全面这个测试很好地验证了组件支持自定义
floorKey的功能，通过使用非标准的键名进行测试，确保组件的灵活性。
211-241: 非字符串值渲染测试有效这个测试验证了组件能够正确处理非字符串值（如数字）作为楼层标识，确保了组件在各种数据类型下的兼容性。
243-257: 索引点击处理测试完善这个测试很好地验证了点击索引条时回调函数接收到正确的键值，确保了事件处理的正确性。
311-319: getData 边界情况测试逻辑合理这个测试很好地验证了当元素不存在 data-index 属性时的行为，确保组件能够处理边缘情况。
321-352: scrollTo 函数边界情况测试全面这个测试很好地覆盖了 scrollTo 函数的各种边界情况，包括滚动到索引 0、负索引和超出列表长度的索引，确保组件的健壮性。
354-376: 拖拽操作状态更新测试全面这个测试很好地验证了组件在拖拽操作期间的状态更新，包括拖拽开始和结束时的状态变化。
414-426: 列表组设置测试设计合理这个测试很好地验证了组件正确设置列表组，确保所有列表项都有正确的类名。
428-465: 组合点击场景测试设计全面这个测试很好地验证了组件在点击索引和列表项时的行为，确保回调函数接收到正确的参数并且高亮状态正确更新。
| // 测试列表滚动时高亮显示的正确性 | ||
| test('should highlight the correct index when scrolling', async () => { | ||
| const { container } = render(<Elevator list={list} height={200} />) | ||
|
|
||
| // 模拟滚动 | ||
| const listView = container.querySelector('.nut-elevator-list-inner') | ||
|
|
||
| await act(() => { | ||
| // 手动触发点击索引，应该会导致滚动和高亮 | ||
| const indexItem = container.querySelectorAll( | ||
| '.nut-elevator-bars-inner-item' | ||
| )[2] | ||
| fireEvent.click(indexItem) | ||
| }) | ||
|
|
||
| // 检查是否正确高亮了第三个索引 | ||
| waitFor(() => { | ||
| const activeIndex = container.querySelector( | ||
| '.nut-elevator-bars-inner-item-active' | ||
| ) | ||
| expect(activeIndex?.textContent).toBe('G') | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
滚动高亮测试有待完善
测试验证了点击索引后的高亮逻辑，但 waitFor 没有返回 Promise 并等待断言完成，这可能导致测试不稳定。
- waitFor(() => {
+ await waitFor(() => {
const activeIndex = container.querySelector(
'.nut-elevator-bars-inner-item-active'
)
expect(activeIndex?.textContent).toBe('G')
- })
+ })📝 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.
| // 测试列表滚动时高亮显示的正确性 | |
| test('should highlight the correct index when scrolling', async () => { | |
| const { container } = render(<Elevator list={list} height={200} />) | |
| // 模拟滚动 | |
| const listView = container.querySelector('.nut-elevator-list-inner') | |
| await act(() => { | |
| // 手动触发点击索引，应该会导致滚动和高亮 | |
| const indexItem = container.querySelectorAll( | |
| '.nut-elevator-bars-inner-item' | |
| )[2] | |
| fireEvent.click(indexItem) | |
| }) | |
| // 检查是否正确高亮了第三个索引 | |
| waitFor(() => { | |
| const activeIndex = container.querySelector( | |
| '.nut-elevator-bars-inner-item-active' | |
| ) | |
| expect(activeIndex?.textContent).toBe('G') | |
| }) | |
| }) | |
| // 测试列表滚动时高亮显示的正确性 | |
| test('should highlight the correct index when scrolling', async () => { | |
| const { container } = render(<Elevator list={list} height={200} />) | |
| // 模拟滚动 | |
| const listView = container.querySelector('.nut-elevator-list-inner') | |
| await act(() => { | |
| // 手动触发点击索引，应该会导致滚动和高亮 | |
| const indexItem = container.querySelectorAll( | |
| '.nut-elevator-bars-inner-item' | |
| )[2] | |
| fireEvent.click(indexItem) | |
| }) | |
| // 检查是否正确高亮了第三个索引 | |
| await waitFor(() => { | |
| const activeIndex = container.querySelector( | |
| '.nut-elevator-bars-inner-item-active' | |
| ) | |
| expect(activeIndex?.textContent).toBe('G') | |
| }) | |
| }) |
| // 测试 listViewScroll 函数 | ||
| test('should handle list view scroll correctly', async () => { | ||
| const { container } = render(<Elevator list={list} height={200} />) | ||
| const listView = container.querySelector('.nut-elevator-list-inner') | ||
|
|
||
| // 模拟滚动事件 | ||
| await act(() => { | ||
| if (listView) { | ||
| Object.defineProperty(listView, 'scrollTop', { value: 100 }) | ||
| fireEvent.scroll(listView) | ||
| } | ||
| }) | ||
|
|
||
| // 验证滚动位置更新 | ||
| const fixedTitle = container.querySelector('.nut-elevator-list-fixed-title') | ||
| expect(fixedTitle).toBeTruthy() | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
列表视图滚动处理测试需要修正断言
在垂直模式下测试固定标题的存在，但没有明确指定 mode="vertical"，这可能导致测试不可靠。
const { container } = render(
- <Elevator list={list} height={200} />
+ <Elevator list={list} height={200} mode="vertical" sticky />
)📝 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.
| // 测试 listViewScroll 函数 | |
| test('should handle list view scroll correctly', async () => { | |
| const { container } = render(<Elevator list={list} height={200} />) | |
| const listView = container.querySelector('.nut-elevator-list-inner') | |
| // 模拟滚动事件 | |
| await act(() => { | |
| if (listView) { | |
| Object.defineProperty(listView, 'scrollTop', { value: 100 }) | |
| fireEvent.scroll(listView) | |
| } | |
| }) | |
| // 验证滚动位置更新 | |
| const fixedTitle = container.querySelector('.nut-elevator-list-fixed-title') | |
| expect(fixedTitle).toBeTruthy() | |
| }) | |
| // 测试 listViewScroll 函数 | |
| test('should handle list view scroll correctly', async () => { | |
| const { container } = render( | |
| <Elevator list={list} height={200} mode="vertical" sticky /> | |
| ) | |
| const listView = container.querySelector('.nut-elevator-list-inner') | |
| // 模拟滚动事件 | |
| await act(() => { | |
| if (listView) { | |
| Object.defineProperty(listView, 'scrollTop', { value: 100 }) | |
| fireEvent.scroll(listView) | |
| } | |
| }) | |
| // 验证滚动位置更新 | |
| const fixedTitle = container.querySelector('.nut-elevator-list-fixed-title') | |
| expect(fixedTitle).toBeTruthy() | |
| }) |
| // 测试当存在垂直模式和sticky时，固定头部是否正确显示 | ||
| test('should show fixed title in vertical mode with sticky', async () => { | ||
| const { container } = render( | ||
| <Elevator list={list} mode="vertical" sticky height={200} /> | ||
| ) | ||
|
|
||
| // 首先触发点击以模拟滚动 | ||
| await act(() => { | ||
| const indexItem = container.querySelectorAll( | ||
| '.nut-elevator-bars-inner-item' | ||
| )[1] | ||
| fireEvent.click(indexItem) | ||
|
|
||
| // 模拟滚动事件 | ||
| const listView = container.querySelector('.nut-elevator-list-inner') | ||
| if (listView) { | ||
| Object.defineProperty(listView, 'scrollTop', { value: 50 }) | ||
| fireEvent.scroll(listView) | ||
| } | ||
| }) | ||
|
|
||
| // 等待滚动效果完成后检查固定标题 | ||
| waitFor(() => { | ||
| const fixedTitle = container.querySelector('.nut-elevator-list-fixed-title') | ||
| expect(fixedTitle).not.toBeNull() | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
垂直模式下固定标题测试需要修复
与前一个类似的问题，waitFor 需要等待断言完成，否则测试可能不稳定。
- waitFor(() => {
+ await waitFor(() => {
const fixedTitle = container.querySelector('.nut-elevator-list-fixed-title')
expect(fixedTitle).not.toBeNull()
- })
+ })📝 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.
| // 测试当存在垂直模式和sticky时，固定头部是否正确显示 | |
| test('should show fixed title in vertical mode with sticky', async () => { | |
| const { container } = render( | |
| <Elevator list={list} mode="vertical" sticky height={200} /> | |
| ) | |
| // 首先触发点击以模拟滚动 | |
| await act(() => { | |
| const indexItem = container.querySelectorAll( | |
| '.nut-elevator-bars-inner-item' | |
| )[1] | |
| fireEvent.click(indexItem) | |
| // 模拟滚动事件 | |
| const listView = container.querySelector('.nut-elevator-list-inner') | |
| if (listView) { | |
| Object.defineProperty(listView, 'scrollTop', { value: 50 }) | |
| fireEvent.scroll(listView) | |
| } | |
| }) | |
| // 等待滚动效果完成后检查固定标题 | |
| waitFor(() => { | |
| const fixedTitle = container.querySelector('.nut-elevator-list-fixed-title') | |
| expect(fixedTitle).not.toBeNull() | |
| }) | |
| }) | |
| // 测试当存在垂直模式和sticky时，固定头部是否正确显示 | |
| test('should show fixed title in vertical mode with sticky', async () => { | |
| const { container } = render( | |
| <Elevator list={list} mode="vertical" sticky height={200} /> | |
| ) | |
| // 首先触发点击以模拟滚动 | |
| await act(() => { | |
| const indexItem = container.querySelectorAll( | |
| '.nut-elevator-bars-inner-item' | |
| )[1] | |
| fireEvent.click(indexItem) | |
| // 模拟滚动事件 | |
| const listView = container.querySelector('.nut-elevator-list-inner') | |
| if (listView) { | |
| Object.defineProperty(listView, 'scrollTop', { value: 50 }) | |
| fireEvent.scroll(listView) | |
| } | |
| }) | |
| // 等待滚动效果完成后检查固定标题 | |
| await waitFor(() => { | |
| const fixedTitle = container.querySelector('.nut-elevator-list-fixed-title') | |
| expect(fixedTitle).not.toBeNull() | |
| }) | |
| }) |
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/packages/elevator/elevator.taro.tsx (1)
84-90: 🛠️ Refactor suggestion缺少非法 index 处理，可能误将未定义数据当作 0
getData在未找到dataset.index时返回字符串'0'，导致用户点击空白处也会跳转到第一个分组。建议与 web 端保持一致，返回'-1'并在touchStart时检测并中断。-if (!el.dataset.index) { - return '0' -} -return el.dataset.index as string +return el.dataset.index ?? '-1'🧰 Tools
🪛 Biome (1.9.4)
[error] 84-84: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
♻️ Duplicate comments (2)
src/packages/elevator/elevator.tsx (1)
189-195:⚠️ Potential issue必须在卸载时移除 scroll 事件监听，避免内存泄漏
useEffect中仅添加了scroll监听，却没有在返回值里执行removeEventListener。组件被多次挂载/卸载时，事件会叠加，造成性能下降及潜在内存泄漏。该问题在上一次评审中已指出，仍未修复。- useEffect(() => { - if (listview.current) { - setListGroup() - listview.current.addEventListener('scroll', listViewScroll) - } - }, [listview]) + useEffect(() => { + if (listview.current) { + setListGroup() + const current = listview.current + current.addEventListener('scroll', listViewScroll) + return () => current.removeEventListener('scroll', listViewScroll) + } + // listview ref 对象本身不会变化，依赖可置空 + }, [])src/packages/elevator/elevator.taro.tsx (1)
91-112:⚠️ Potential issue
calculateHeight为异步回调，却未返回Promise——await无效
scrollTo中使用await calculateHeight()，但calculateHeight没有返回值，await会立即继续执行，state.current.listHeight仍为空，最终定位可能出错。上次评审已提示，仍未修复。-const calculateHeight = () => { - state.current.listHeight = [0] - const query = createSelectorQuery() - ... -} +const calculateHeight = (): Promise<void> => + new Promise((resolve) => { + state.current.listHeight = [0] + createSelectorQuery() + .selectAll(`#${classPrefix}-${uuid} .${classPrefix}-list-item`) + .boundingClientRect() + .exec((rect = []) => { + if (rect[0]?.length) { + state.current.listHeight = rect[0].reduce( + (acc: number[], item: { height: number }, idx: number) => { + acc.push(acc[idx] + item.height) + return acc + }, + [0] + ) + } + resolve() + }) + })🧰 Tools
🪛 Biome (1.9.4)
[error] 99-99: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🧹 Nitpick comments (2)
src/packages/elevator/elevator.tsx (2)
167-184: 滚动回调频繁 setState，建议节流处理
listViewScroll每次scroll都setScrollY/setCurrentIndex，在长列表上可能触发大量重渲染。可使用requestAnimationFrame或lodash/throttle对回调做节流，或在状态重复时提前return。
196-201: 类名拼接存在空字符串，使用classNames更安全当
className为空值时，${classPrefix} ${classPrefix}-${mode} ${className}会产生尾部空格。改用classNames可避免此问题并提升可读性。-<div className={`${classPrefix} ${classPrefix}-${mode} ${className}`} ...> +<div + className={classNames(classPrefix, `${classPrefix}-${mode}`, className)} + ...>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/packages/elevator/elevator.taro.tsx(10 hunks)src/packages/elevator/elevator.tsx(10 hunks)src/types/spec/elevator/base.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types/spec/elevator/base.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/packages/elevator/elevator.tsx (4)
src/packages/elevator/elevator.taro.tsx (2)
elevatorContext(19-19)Elevator(34-318)src/types/spec/elevator/base.ts (2)
ElevatorItem(4-8)ElevatorList(11-14)src/packages/elevator/index.ts (1)
WebElevatorProps(3-3)src/hooks/use-props-value.ts (1)
usePropsValue(11-35)
🪛 Biome (1.9.4)
src/packages/elevator/elevator.taro.tsx
[error] 99-99: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 165-169: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 171-173: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (1)
src/packages/elevator/elevator.taro.tsx (1)
114-135:scrollTo中的await calculateHeight()不起作用受上一问题影响，即使
await，随后立即读取listHeight[cacheIndex]也可能还是空数组，应在确保高度计算完成后再执行滚动逻辑，可将读取逻辑移入calculateHeight的then中或在其resolve后调用。
| const getData = (el: HTMLElement, name: string): string => { | ||
| const prefix = 'data-' | ||
| return el.getAttribute(prefix + name) as string | ||
| // 检查点击的元素是否直接包含 data-index 属性 | ||
| if (el.hasAttribute('data-index')) { | ||
| return el.getAttribute(prefix + name) as string | ||
| } | ||
| return '-1' | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
getData 仅检查当前元素，子节点点击将失效
很多场景中点击事件会冒泡到子节点（例如 <span> 或图标），这些子节点不含 data-index，函数会直接返回 -1，导致无法拖动。建议向上递归查找最近带有 data-index 的父元素，或改用事件代理读取 currentTarget.dataset.index。
| const index = listHeight.findIndex( | ||
| (height, i) => | ||
| state.current.scrollY >= height && | ||
| state.current.scrollY < (listHeight[i + 1] || Infinity) | ||
| ) | ||
|
|
||
| if (index !== -1 && index !== codeIndex) { | ||
| setCodeIndex(index) | ||
| } | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
findIndex 逻辑仍依赖异步数据，需考虑空数组分支
listHeight 为空时立即 calculateHeight() 但未等待结果即可执行 findIndex，依旧可能返回 -1。应在高度未就绪时直接 return，或同样改造成 Promise 链确保数据可用。
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/packages/elevator/__tests__/elevator.spec.tsx (2)
260-282:⚠️ Potential issue滚动高亮测试需要修复
测试验证了点击索引后的高亮逻辑，但
waitFor没有返回Promise并等待断言完成，这可能导致测试不稳定。- waitFor(() => { + await waitFor(() => { const activeIndex = container.querySelector( '.nut-elevator-bars-inner-item-active' ) expect(activeIndex?.textContent).toBe('G') - }) + })
284-310:⚠️ Potential issue垂直模式下固定标题测试需要修复
与前一个类似的问题，
waitFor需要等待断言完成，否则测试可能不稳定。- waitFor(() => { + await waitFor(() => { const fixedTitle = container.querySelector('.nut-elevator-list-fixed-title') expect(fixedTitle).not.toBeNull() - }) + })
🧹 Nitpick comments (2)
src/packages/elevator/__tests__/elevator.spec.tsx (2)
312-317: getData函数边界情况测试可优化当前测试只验证了元素没有data-index属性，但没有实际验证getData函数的返回值。建议添加对函数返回值的断言。
test('should return -1 when data-index attribute is not present', () => { - const { container } = render(<Elevator list={list} />) - const element = container.querySelector('.nut-elevator') - expect(element).not.toHaveAttribute('data-index') + const { container } = render(<Elevator list={list} />) + const element = container.querySelector('.nut-elevator')! + + // 模拟访问内部方法来测试getData函数 + // 注意：这需要将getData暴露出来或使用其他方法访问组件实例 + // 这里只是示例，实际实现可能需要调整 + expect(element).not.toHaveAttribute('data-index') + // TODO: 验证getData(element)返回-1 })
319-324: 简化测试用例可以扩展这些测试过于简化，只是检查组件能否正常渲染，没有验证实际功能。建议扩展这些测试以提供更有意义的功能验证。
例如，对于scrollTo函数的边界情况测试，可以：
- 模拟列表项的DOM结构
- 调用scrollTo函数并传入边界值（如-1或超出范围的索引）
- 验证是否正确处理了这些边界情况
对于其他简化的测试也可采用类似方法进行扩展。
Also applies to: 326-331, 340-347, 359-379, 381-386
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/elevator/__tests__/elevator.spec.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/packages/elevator/__tests__/elevator.spec.tsx (1)
src/packages/elevator/elevator.tsx (1)
Elevator(30-290)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (11)
src/packages/elevator/__tests__/elevator.spec.tsx (11)
5-5: 导入了事件模拟工具新增了触摸事件模拟工具的导入，用于测试拖拽相关功能，这是一个很好的改进。
134-138: 垂直模式测试实现正确正确地测试了垂直模式渲染，验证了对应的CSS类存在。
140-144: showKeys属性测试有效正确验证了当showKeys为false时不渲染索引栏。
146-163: 自定义内容渲染测试实现良好通过CustomItem组件测试了children属性的渲染逻辑，代码结构清晰。
165-172: 非数字高度值处理测试良好正确验证了组件能够处理"50vh"等非数字高度值。
174-178: 空列表渲染测试有效正确验证了当列表为空时不渲染任何列表项。
180-210: 自定义楼层键测试实现正确通过customTitle字段正确测试了floorKey属性的功能。
212-242: 非字符串值渲染测试有效正确验证了组件能够处理数字等非字符串值的渲染。
244-258: 索引点击事件参数传递测试正确验证了点击索引栏时能正确传递索引值到回调函数。
388-402: 拖拽开始测试实现良好正确使用了
await waitFor和事件模拟工具测试拖拽开始行为。
404-417: 拖拽结束测试实现良好正确测试了拖拽结束后的状态变化，确保了代码指示器被正确移除。
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/packages/elevator/__tests__/elevator.spec.tsx (2)
84-89: 应将 waitFor 方法与 await 一起使用以确保测试稳定性测试中使用了 waitFor 方法但没有等待其完成，这可能导致测试不稳定。这与先前的审查意见相同，仍然需要修复。
- waitFor(() => { + await waitFor(() => { expect( container.querySelector('.nut-elevator-list-item-name-highcolor') ?.innerHTML ).toBe('安徽') })
239-265: 应将 waitFor 方法与 await 一起使用以确保测试稳定性与前一个类似的问题，
waitFor需要等待断言完成，否则测试可能不稳定。- waitFor(() => { + await waitFor(() => { const fixedTitle = container.querySelector('.nut-elevator-list-fixed-title') expect(fixedTitle).not.toBeNull() - }) + })
🧹 Nitpick comments (2)
src/packages/elevator/__tests__/elevator.spec.tsx (2)
125-142: 建议增强自定义内容渲染测试测试验证了自定义内容能够正确渲染，但没有实际测试 Context 的使用。考虑添加使用 Elevator.Context 的测试用例以更全面地验证功能。
test('should render custom content with children', () => { // 使用 Context 消费者组件 const CustomItem = () => { // 在测试环境中，我们不能直接使用 useContext，因此直接渲染一个固定元素 - return <div className="custom-item">自定义内容</div> + return ( + <Elevator.Context.Consumer> + {(value) => ( + <div className="custom-item" data-id={value?.id}> + {value?.name || '自定义内容'} + </div> + )} + </Elevator.Context.Consumer> + ) } const { container } = render( <Elevator list={list}> <CustomItem /> </Elevator> ) const customItems = container.querySelectorAll('.custom-item') expect(customItems.length).toBeGreaterThan(0) - expect(customItems[0].textContent).toBe('自定义内容') + expect(customItems[0]).toHaveAttribute('data-id') + expect(customItems[0].textContent).toBeTruthy() })
267-272: 测试覆盖不足，建议增强对 getData 函数的测试当前测试只验证了元素没有 data-index 属性，但没有验证 getData 函数返回的实际值。建议增强测试以验证函数的完整行为。
test('should return -1 when data-index attribute is not present', () => { const { container } = render(<Elevator list={list} />) const element = container.querySelector('.nut-elevator') expect(element).not.toHaveAttribute('data-index') + + // 模拟一个有 data-index 的元素 + const mockElement = document.createElement('div') + mockElement.setAttribute('data-index', '2') + + // 访问内部 getData 函数进行测试 + // 注意：这种做法在实际测试中可能需要额外的方法来访问组件内部函数 + // 这里仅作为示例 + // const result = component.instance().getData(mockElement, 'index') + // expect(result).toBe('2') })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/elevator/__tests__/elevator.spec.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/packages/elevator/__tests__/elevator.spec.tsx (1)
src/packages/elevator/elevator.tsx (1)
Elevator(30-290)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (6)
src/packages/elevator/__tests__/elevator.spec.tsx (6)
113-117: LGTM! 垂直模式的渲染测试正确有效测试正确验证了当设置
mode="vertical"时，组件会添加对应的 CSS 类。
119-123: LGTM! showKeys 属性验证逻辑合理测试正确地验证了当
showKeys设置为 false 时，索引栏不会被渲染。
144-151: LGTM! 非数字高度值的处理逻辑正确测试正确地验证了组件可以处理非数字高度值（如 "50vh"）。
153-157: LGTM! 空列表渲染测试有效测试正确验证了当提供空列表时组件的渲染行为。
159-189: LGTM! 自定义楼层键测试全面测试全面验证了组件可以使用自定义属性名作为楼层键。
191-221: LGTM! 非字符串值的渲染测试完善测试正确验证了组件可以处理数字类型的索引值，并将其正确地转换为字符串显示。
| test('should handle edge cases in scrollTo function', () => { | ||
| render(<Elevator list={list} height={200} />) | ||
| // 这个测试只是确认组件能正常渲染，不会因为边界条件而崩溃 | ||
| expect(true).toBeTruthy() | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
多个简化测试需要增强断言逻辑
多个测试（包括 scrollTo、拖拽操作、listViewScroll 和 resetScrollState 函数测试）过于简化，只验证了组件能够渲染而没有测试实际功能。建议增强这些测试以验证相应功能的正确实现。
考虑为这些简化测试添加更具体的断言，比如：
- 对于 scrollTo 测试，可以在调用函数后验证滚动位置变化
- 对于拖拽操作，可以验证状态变化和 UI 更新
- 对于 listViewScroll，可以模拟实际滚动并验证索引更新
- 对于 resetScrollState，可以验证状态重置后的 UI 变化
例如，对于 resetScrollState 测试：
test('should reset scroll state correctly', () => {
- render(<Elevator list={list} height={200} />)
- // 简化测试，只确认组件正常渲染
- expect(true).toBeTruthy()
+ const { container } = render(<Elevator list={list} height={200} />)
+
+ // 首先模拟拖拽开始，触发 scrollStart 状态
+ const barsInner = container.querySelector('.nut-elevator-bars-inner')
+ act(() => {
+ trigger(barsInner, 'touchstart', 0, 0)
+ trigger(barsInner, 'touchmove', 0, 20)
+ })
+
+ // 验证拖拽开始后，滚动指示器显示
+ expect(container.querySelector('.nut-elevator-code-current')).toBeTruthy()
+
+ // 模拟拖拽结束，触发状态重置
+ act(() => {
+ trigger(barsInner, 'touchend', 0, 0)
+ })
+
+ // 验证状态重置后，滚动指示器消失
+ expect(container.querySelector('.nut-elevator-code-current')).toBeFalsy()
})Also applies to: 281-286, 295-302, 336-341
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
src/packages/elevator/__tests__/elevator.spec.tsx (6)
84-89: 需要添加 await 以确保测试稳定性此处等待断言完成应使用
await waitFor而不是waitFor，以确保测试稳定性。- waitFor(() => { + await waitFor(() => { expect( container.querySelector('.nut-elevator-list-item-name-highcolor') ?.innerHTML ).toBe('安徽') })
261-264: 测试中 waitFor 需要 await 支持垂直模式下固定标题测试需要添加 await 以确保测试稳定性。
- waitFor(() => { + await waitFor(() => { const fixedTitle = container.querySelector('.nut-elevator-list-fixed-title') expect(fixedTitle).not.toBeNull() })
275-279: 简化测试需要增强断言逻辑此测试仅确认组件能正常渲染，没有测试 scrollTo 函数的边界情况。建议添加实际的边界条件测试和断言。
建议增加对 scrollTo 函数的实际测试，比如：
test('should handle edge cases in scrollTo function', () => { - render(<Elevator list={list} height={200} />) - // 这个测试只是确认组件能正常渲染，不会因为边界条件而崩溃 - expect(true).toBeTruthy() + const { container } = render(<Elevator list={list} height={200} />) + + // 获取组件实例 + const component = container.querySelector('.nut-elevator') + + // 检查无效索引的情况 + const indexItems = container.querySelectorAll('.nut-elevator-bars-inner-item') + const invalidIndex = indexItems.length + 1 + + // 触发点击事件到有效和无效索引 + fireEvent.click(indexItems[0]) + + // 验证组件状态更新但不会崩溃 + expect(container.querySelector('.nut-elevator-bars-inner-item-active')).toBeTruthy() })
281-286: 拖拽操作状态变化测试过于简化测试没有实际验证拖拽操作过程中的状态变化，仅验证了组件能够渲染。建议增加实际的拖拽操作测试和相应的状态断言。
test('should update states correctly during drag operations', () => { - render(<Elevator list={list} height={200} />) - // 简化测试，只确认组件正常渲染 - expect(true).toBeTruthy() + const { container } = render(<Elevator list={list} height={200} />) + + const barsInner = container.querySelector('.nut-elevator-bars-inner') + + // 模拟拖拽操作 + act(() => { + trigger(barsInner, 'touchstart', 0, 0) + trigger(barsInner, 'touchmove', 0, 20) + }) + + // 验证拖拽开始后的状态变化 + expect(container.querySelector('.nut-elevator-code-current')).toBeTruthy() + + // 模拟拖拽结束 + act(() => { + trigger(barsInner, 'touchend', 0, 20) + }) + + // 验证拖拽结束后的状态重置 + expect(container.querySelector('.nut-elevator-code-current')).toBeFalsy() })
295-302: 列表视图滚动处理测试需要加强listViewScroll 函数测试过于简化，没有实际验证滚动事件的处理逻辑。建议增加实际的滚动事件测试和相应的断言。
test('should handle list view scroll correctly', () => { const { container } = render( <Elevator list={list} height={200} mode="vertical" sticky /> ) - // 简化测试，只确认组件正常渲染 - expect(container.querySelector('.nut-elevator')).toBeTruthy() + + // 获取列表视图元素 + const listView = container.querySelector('.nut-elevator-list-inner') + + // 模拟滚动事件 + act(() => { + if (listView) { + Object.defineProperty(listView, 'scrollTop', { value: 50 }) + fireEvent.scroll(listView) + } + }) + + // 验证滚动后固定标题显示 + const fixedTitle = container.querySelector('.nut-elevator-list-fixed-title') + expect(fixedTitle).not.toBeNull() })
336-341: 滚动状态重置测试需要加强resetScrollState 函数测试过于简化，没有实际验证状态重置的逻辑。建议增加实际的状态重置测试和相应的断言。
test('should reset scroll state correctly', () => { - render(<Elevator list={list} height={200} />) - // 简化测试，只确认组件正常渲染 - expect(true).toBeTruthy() + const { container } = render(<Elevator list={list} height={200} />) + + // 获取导航栏元素 + const barsInner = container.querySelector('.nut-elevator-bars-inner') + + // 模拟拖拽开始，触发滚动状态 + act(() => { + trigger(barsInner, 'touchstart', 0, 0) + trigger(barsInner, 'touchmove', 0, 20) + }) + + // 验证拖拽开始后滚动指示器显示 + expect(container.querySelector('.nut-elevator-code-current')).toBeTruthy() + + // 模拟拖拽结束，触发状态重置 + act(() => { + trigger(barsInner, 'touchend', 0, 20) + }) + + // 验证状态重置后滚动指示器消失 + expect(container.querySelector('.nut-elevator-code-current')).toBeFalsy() })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/elevator/__tests__/elevator.spec.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (8)
src/packages/elevator/__tests__/elevator.spec.tsx (8)
113-117: 新增的垂直模式测试良好为新添加的
mode="vertical"属性编写了测试，验证了垂直模式下正确应用了 CSS 类。这有助于确保该功能在未来的更改中保持稳定。
119-123: showKeys 属性测试实现合理针对
showKeys={false}情况的测试确保了当此属性设置为 false 时不会渲染索引栏，这是一个有用的功能测试。
125-142: 自定义内容渲染测试实现完善通过自定义组件
CustomItem测试了 children 属性，验证了组件能够正确渲染自定义内容，这对于组件的灵活性至关重要。
153-157: 空列表处理测试有助于稳定性测试了空列表情况下的组件渲染，这对于防止边界情况下的崩溃非常重要。
159-189: 自定义属性键测试增强了组件灵活性通过
floorKey属性测试了自定义标题属性，这增强了组件的灵活性，允许用户使用自定义数据结构。
191-221: 非字符串值渲染测试完善了组件健壮性通过测试数字索引值的正确渲染，增强了组件处理不同数据类型的能力，提高了组件的健壮性。
223-237: 索引点击值测试确保了事件处理正确性点击索引项后验证了正确的索引值被传递给回调函数，这对于确保组件的交互行为符合预期非常重要。
354-367: 拖拽结束测试实现合理使用
triggerDrag模拟完整的拖拽过程，并验证了拖拽结束后的状态变化，测试逻辑完整且有效。
| // 测试 handleClickItem 和 handleClickIndex 的组合场景 - 使用简化的测试 | ||
| test('should handle combined click scenarios', () => { | ||
| const onItemClick = vi.fn() | ||
| const onIndexClick = vi.fn() | ||
|
|
||
| const { container, getByText } = render( | ||
| <Elevator | ||
| list={list} | ||
| height={200} | ||
| sticky | ||
| onIndexClick={(key: string) => testClick(key)} | ||
| onItemClick={onItemClick} | ||
| onIndexClick={onIndexClick} | ||
| /> | ||
| ) | ||
| const listItem = container.querySelectorAll( | ||
| '.nut-elevator-bars-inner-item' | ||
| )[2] | ||
| fireEvent.click(listItem) // 模拟点击 | ||
| setTimeout(() => { | ||
| expect(container.querySelectorAll('.nut-elevator-list-fixed').length).toBe( | ||
| 1 | ||
| ) | ||
| }, 300) | ||
|
|
||
| // 简化测试，只确认组件正常渲染和事件处理函数可以被调用 | ||
| const indexItem = container.querySelector('.nut-elevator-bars-inner-item') | ||
| expect(indexItem).toBeTruthy() | ||
|
|
||
| const listItem = container.querySelector('.nut-elevator-list-item-name') | ||
| expect(listItem).toBeTruthy() | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
组合点击场景测试需要添加实际点击操作和断言
测试声明了要测试组合点击场景，但实际上只检查了元素的存在，没有触发任何点击事件或验证处理函数的调用情况。
test('should handle combined click scenarios', () => {
const onItemClick = vi.fn()
const onIndexClick = vi.fn()
const { container, getByText } = render(
<Elevator
list={list}
height={200}
onItemClick={onItemClick}
onIndexClick={onIndexClick}
/>
)
- // 简化测试，只确认组件正常渲染和事件处理函数可以被调用
const indexItem = container.querySelector('.nut-elevator-bars-inner-item')
expect(indexItem).toBeTruthy()
const listItem = container.querySelector('.nut-elevator-list-item-name')
expect(listItem).toBeTruthy()
+
+ // 测试点击索引
+ fireEvent.click(indexItem)
+ expect(onIndexClick).toHaveBeenCalled()
+
+ // 测试点击列表项
+ fireEvent.click(listItem)
+ expect(onItemClick).toHaveBeenCalled()
+
+ // 验证点击索引后列表项的高亮状态
+ fireEvent.click(indexItem)
+ expect(container.querySelector('.nut-elevator-bars-inner-item-active')).toBeTruthy()
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 测试 handleClickItem 和 handleClickIndex 的组合场景 - 使用简化的测试 | |
| test('should handle combined click scenarios', () => { | |
| const onItemClick = vi.fn() | |
| const onIndexClick = vi.fn() | |
| const { container, getByText } = render( | |
| <Elevator | |
| list={list} | |
| height={200} | |
| sticky | |
| onIndexClick={(key: string) => testClick(key)} | |
| onItemClick={onItemClick} | |
| onIndexClick={onIndexClick} | |
| /> | |
| ) | |
| const listItem = container.querySelectorAll( | |
| '.nut-elevator-bars-inner-item' | |
| )[2] | |
| fireEvent.click(listItem) // 模拟点击 | |
| setTimeout(() => { | |
| expect(container.querySelectorAll('.nut-elevator-list-fixed').length).toBe( | |
| 1 | |
| ) | |
| }, 300) | |
| // 简化测试，只确认组件正常渲染和事件处理函数可以被调用 | |
| const indexItem = container.querySelector('.nut-elevator-bars-inner-item') | |
| expect(indexItem).toBeTruthy() | |
| const listItem = container.querySelector('.nut-elevator-list-item-name') | |
| expect(listItem).toBeTruthy() | |
| }) | |
| // 测试 handleClickItem 和 handleClickIndex 的组合场景 - 使用简化的测试 | |
| test('should handle combined click scenarios', () => { | |
| const onItemClick = vi.fn() | |
| const onIndexClick = vi.fn() | |
| const { container, getByText } = render( | |
| <Elevator | |
| list={list} | |
| height={200} | |
| onItemClick={onItemClick} | |
| onIndexClick={onIndexClick} | |
| /> | |
| ) | |
| const indexItem = container.querySelector('.nut-elevator-bars-inner-item') | |
| expect(indexItem).toBeTruthy() | |
| const listItem = container.querySelector('.nut-elevator-list-item-name') | |
| expect(listItem).toBeTruthy() | |
| // 测试点击索引 | |
| fireEvent.click(indexItem) | |
| expect(onIndexClick).toHaveBeenCalled() | |
| // 测试点击列表项 | |
| fireEvent.click(listItem) | |
| expect(onItemClick).toHaveBeenCalled() | |
| // 验证点击索引后列表项的高亮状态 | |
| fireEvent.click(indexItem) | |
| expect(container.querySelector('.nut-elevator-bars-inner-item-active')).toBeTruthy() | |
| }) |
| test('should handle drag start correctly', async () => { | ||
| const { container } = render(<Elevator list={list} height={200} />) | ||
| const barsInner = container.querySelector('.nut-elevator-bars-inner') | ||
|
|
||
| // 模拟拖拽开始 | ||
| await act(async () => { | ||
| trigger(barsInner, 'dragstart', 0, 0) | ||
| trigger(barsInner, 'dragmove', 0, 20) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
拖拽开始测试需要统一事件名称和添加断言
测试中使用了 dragstart 和 dragmove 事件，而不是标准的 touchstart 和 touchmove 事件，与第 359-361 行的 triggerDrag 方法不一致。同时没有验证拖拽开始后的状态变化。
test('should handle drag start correctly', async () => {
const { container } = render(<Elevator list={list} height={200} />)
const barsInner = container.querySelector('.nut-elevator-bars-inner')
// 模拟拖拽开始
await act(async () => {
- trigger(barsInner, 'dragstart', 0, 0)
- trigger(barsInner, 'dragmove', 0, 20)
+ trigger(barsInner, 'touchstart', 0, 0)
+ trigger(barsInner, 'touchmove', 0, 20)
})
+
+ // 验证拖拽开始后滚动指示器显示
+ await waitFor(() => {
+ expect(container.querySelector('.nut-elevator-code-current')).toBeTruthy()
+ })
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('should handle drag start correctly', async () => { | |
| const { container } = render(<Elevator list={list} height={200} />) | |
| const barsInner = container.querySelector('.nut-elevator-bars-inner') | |
| // 模拟拖拽开始 | |
| await act(async () => { | |
| trigger(barsInner, 'dragstart', 0, 0) | |
| trigger(barsInner, 'dragmove', 0, 20) | |
| }) | |
| }) | |
| test('should handle drag start correctly', async () => { | |
| const { container } = render(<Elevator list={list} height={200} />) | |
| const barsInner = container.querySelector('.nut-elevator-bars-inner') | |
| // 模拟拖拽开始 | |
| await act(async () => { | |
| trigger(barsInner, 'touchstart', 0, 0) | |
| trigger(barsInner, 'touchmove', 0, 20) | |
| }) | |
| // 验证拖拽开始后滚动指示器显示 | |
| await waitFor(() => { | |
| expect(container.querySelector('.nut-elevator-code-current')).toBeTruthy() | |
| }) | |
| }) |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
src/packages/elevator/__tests__/elevator.spec.tsx (6)
238-264: 修复垂直模式下固定标题测试中的异步问题测试需要等待异步操作完成，当前实现可能导致测试不稳定。
在
waitFor前添加await确保异步断言完成：- waitFor(() => { + await waitFor(() => { const fixedTitle = container.querySelector('.nut-elevator-list-fixed-title') expect(fixedTitle).not.toBeNull() - }) + })
273-278: scrollTo 函数边界情况测试需增强当前测试过于简化，只验证了组件能够渲染而没有测试实际功能。
建议增强测试以验证 scrollTo 函数的实际功能，例如：
test('should handle edge cases in scrollTo function', () => { - render(<Elevator list={list} height={200} />) - // 这个测试只是确认组件能正常渲染，不会因为边界条件而崩溃 - expect(true).toBeTruthy() + const { container } = render(<Elevator list={list} height={200} />) + + // 获取组件实例 + const elevatorInstance = container.querySelector('.nut-elevator') + + // 模拟点击某个索引来触发 scrollTo 函数 + const indexItem = container.querySelectorAll('.nut-elevator-bars-inner-item')[2] + fireEvent.click(indexItem) + + // 验证滚动发生了变化 + const listView = container.querySelector('.nut-elevator-list-inner') + expect(listView.scrollTop).not.toBe(0) + + // 测试边界情况：点击不存在的索引 + // ... })
280-286: 拖拽状态变化测试需增强当前测试过于简化，只验证了组件能够渲染而没有测试实际的拖拽状态变化。
建议增强测试以验证拖拽操作过程中的状态变化，例如验证拖拽开始后滚动指示器显示、拖拽结束后滚动状态重置等。
294-302: listViewScroll 函数测试需增强当前测试过于简化，只验证了组件能够渲染而没有测试实际的滚动功能。
建议增强测试以验证列表滚动时的索引更新逻辑，例如模拟滚动事件并验证当前索引值的变化。
313-333: 点击处理函数组合测试需增强当前测试仅验证元素存在，没有执行实际的点击操作或验证函数调用。
建议增强测试以验证实际的点击场景，例如：
test('should handle combined click scenarios', () => { const onItemClick = vi.fn() const onIndexClick = vi.fn() const { container, getByText } = render( <Elevator list={list} height={200} onItemClick={onItemClick} onIndexClick={onIndexClick} /> ) - // 简化测试，只确认组件正常渲染和事件处理函数可以被调用 const indexItem = container.querySelector('.nut-elevator-bars-inner-item') expect(indexItem).toBeTruthy() const listItem = container.querySelector('.nut-elevator-list-item-name') expect(listItem).toBeTruthy() + + // 测试点击索引 + fireEvent.click(indexItem) + expect(onIndexClick).toHaveBeenCalled() + + // 测试点击列表项 + fireEvent.click(listItem) + expect(onItemClick).toHaveBeenCalled() + + // 验证点击索引后列表项的高亮状态 + fireEvent.click(indexItem) + expect(container.querySelector('.nut-elevator-bars-inner-item-active')).toBeTruthy() })
335-341: resetScrollState 函数测试需增强当前测试过于简化，只验证了组件能够渲染而没有测试实际的状态重置功能。
建议增强测试以验证状态重置后的 UI 变化，例如模拟拖拽然后触发状态重置，验证滚动指示器的消失。
🧹 Nitpick comments (3)
src/packages/elevator/__tests__/elevator.spec.tsx (3)
287-292: calculateHeight 函数测试有待完善当前测试只验证了列表项目数量，没有验证高度计算的正确性。
建议修改测试以验证高度计算的正确性，例如模拟不同高度的列表项，然后验证计算结果。
303-311: setListGroup 函数测试需增强当前测试仅验证了列表项目的类名和数量，没有验证列表组的设置逻辑。
建议增强测试以验证 setListGroup 函数的实际功能，例如检查 DOM 元素的引用是否被正确获取。
342-366: 拖拽开始事件测试实现测试通过 CustomEvent 模拟了拖拽开始事件，但缺少对拖拽开始后状态的验证。
建议添加状态验证，例如检查拖拽开始后滚动指示器是否显示：
await act(async () => { // 触发 onDragStart const event = new CustomEvent('dragstart', { detail: dragStartEvent }) barItem?.dispatchEvent(event) }) + + // 验证拖拽开始后的状态 + await waitFor(() => { + const currentCode = container.querySelector('.nut-elevator-code-current') + expect(currentCode).toBeTruthy() + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/elevator/__tests__/elevator.spec.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/packages/elevator/__tests__/elevator.spec.tsx (1)
src/packages/elevator/elevator.tsx (1)
Elevator(30-290)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (11)
src/packages/elevator/__tests__/elevator.spec.tsx (11)
112-116: 测试组件垂直模式渲染正确这个测试正确地验证了当设置
mode="vertical"属性时，组件会添加.nut-elevator-vertical类。
118-122: 验证 showKeys 属性控制索引栏显示测试正确地验证了当
showKeys={false}时，不会渲染电梯索引栏。
124-141: 自定义内容渲染测试完善测试通过传入自定义组件作为 children 属性并验证其渲染结果，充分测试了组件的自定义内容渲染能力。
143-150: 非数字高度值处理测试测试验证了组件能够正确处理 "50vh" 这样的非数字高度值，属于良好的边界情况测试。
152-156: 空列表渲染测试测试验证了组件在列表为空时的渲染行为，是必要的边界情况测试。
158-188: 自定义 floorKey 属性测试测试验证了当使用自定义的
floorKey属性时，组件能够正确读取并显示对应的标题值。
190-220: 非字符串值渲染测试测试验证了组件能够正确处理和显示数字等非字符串类型的索引值，这是重要的边界情况测试。
222-236: 索引点击事件参数传递测试测试验证了点击索引栏项目时传递的参数正确性，逻辑清晰且完整。
266-271: 数据属性缺失的边界情况测试测试验证了当元素没有
data-index属性时的行为，这是必要的边界情况测试。
368-397: 拖拽结束事件测试完善测试正确模拟了拖拽结束事件并验证了状态更新，使用
await waitFor保证了异步测试的可靠性。
399-455: 完整拖拽流程测试实现测试模拟了完整的拖拽过程（开始、移动、结束）并验证了最终状态，测试设计全面且包含了必要的状态验证。
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新功能
mode属性切换，默认水平模式。样式
文档
spaceHeight默认值由 23 改为 18，移除titleHeight属性，完善中英文及 Taro 版文档说明。mode属性说明，明确默认值及可选项。演示与示例
测试