feat: v15 popup #2844
feat: v15 popup #2844
Conversation
Walkthrough此次更改主要集中在 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Demo
participant Popup
User->>Demo: Click to open popup
Demo->>Popup: Set visible to true
Popup-->>User: Display popup
User->>Popup: Interact with popup
Popup-->>Demo: Trigger onClose if necessary
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (1)src/config.json (1)
Popup 组件添加 v15 标记符合预期。经过检查，其他具有 v15 标记的组件包括：
✅ Verification successfulLet me get more context about the components to verify their categorization and ensure the v15 flag is consistently applied across related components. v15 标记添加验证完成，确认符合预期 经过验证，Popup 组件添加 v15 标记是合理的。目前带有 v15 标记的组件包括：
所有组件都具有完整的中文名称、描述和分类，v15 标记使用统一且合理。 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# 描述：验证所有带有 v15 标记的组件
# 搜索所有带有 v15 标记的组件配置
rg '"v15":\s*true' -A 5
Length of output: 2558 Script: #!/bin/bash
# 获取更多上下文来确认组件类型和分类
rg -B 5 '"v15":\s*true' src/config.json
Length of output: 1769 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (8)
src/packages/popup/demos/h5/demo1.tsx (1)
16-20: 建议优化弹出层标题和描述的可配置性当前的标题和描述是硬编码的，建议考虑通过属性传递这些值，以提高组件的复用性。
const Demo = () => { const [showIcon, setShowIcon] = useState(false) + const popupConfig = { + title: "标题", + description: "这里是副标题这是副标题" + } return ( <> <Cell title="基础弹框" onClick={() => setShowIcon(true)} /> <Popup closeable visible={showIcon} - title="标题" - description="这里是副标题这是副标题" + title={popupConfig.title} + description={popupConfig.description} position="bottom" onClose={() => setShowIcon(false)} /> </> ) }src/packages/popup/demos/taro/demo1.tsx (1)
1-28: 建议重构以减少H5和Taro demo之间的代码重复H5版本和Taro版本的demo代码几乎完全相同，建议考虑以下重构方案：
- 创建一个共享的基础组件
- 使用高阶组件或自定义hook处理平台特定的逻辑
建议的目录结构：
src/packages/popup/demos/ ├── shared/ │ └── BasicPopup.tsx # 共享的基础组件 ├── h5/ │ └── demo1.tsx # H5特定的包装器 └── taro/ └── demo1.tsx # Taro特定的包装器src/packages/popup/demos/taro/demo8.tsx (1)
Line range hint
22-27: 建议优化滚动列表的实现方式当前实现存在以下几个问题：
- 使用 Array.from 创建200个元素的数组性能欠佳
- 固定的滚动区域高度可能在不同设备上表现不一致
建议的优化方案：
- <ScrollView scrollY style={{ height: '300px' }}> - {Array.from({ length: 200 }) - .fill('') - .map((_, i) => ( - <Cell key={i}>禁止滚动穿透-{i}</Cell> - ))} - </ScrollView> + const ITEMS_COUNT = 200 + const SCROLL_HEIGHT = '50vh' + + <ScrollView scrollY style={{ height: SCROLL_HEIGHT }}> + {[...new Array(ITEMS_COUNT)].map((_, i) => ( + <Cell key={i}>禁止滚动穿透-{i}</Cell> + ))} + </ScrollView>src/packages/popup/demos/h5/demo2.tsx (1)
72-86: 优化居中弹出层的滚动实现当前实现可能在大量数据时存在性能问题。建议考虑以下优化：
- 使用虚拟列表来优化长列表渲染
- 添加加载状态指示器
- <div style={{ height: '100px', overflowY: 'auto' }}> - {Array.from({ length: 10 }) - .fill('') - .map((_, i) => ( - <Cell key={i}>正文</Cell> - ))} - </div> + <VirtualList + height={100} + itemCount={10} + itemSize={44} + renderItem={({ index }) => ( + <Cell key={index}>正文</Cell> + )} + />src/packages/popup/demos/taro/demo2.tsx (2)
3-3: ScrollView 组件使用建议ScrollView 的实现符合 Taro 规范，但建议添加以下属性以提升用户体验：
- 添加下拉刷新功能
- 添加触底加载更多功能
- <ScrollView style={{ height: '100px' }} scrollY> + <ScrollView + style={{ height: '100px' }} + scrollY + refresherEnabled + onRefresherRefresh={onRefresh} + onScrollToLower={onLoadMore} + >Also applies to: 84-92
55-61: 底部弹出层样式优化建议建议将固定高度改为相对高度，以适应不同屏幕尺寸。
- style={{ height: '300px' }} + style={{ height: '50vh' }}src/packages/popup/popup.scss (2)
5-5: 弹出层最小高度调整建议固定的最小高度可能在某些场景下不够灵活，建议使用更灵活的方式。
- min-height: 46%; + min-height: min(46%, 460px);
31-31: 颜色变量使用建议建议将颜色值统一到设计规范中，避免使用散落的颜色变量。
- color: $color-text; + color: var(--nutui-color-text); - color: $color-title; - cursor: pointer; + color: var(--nutui-color-title); + cursor: pointer;Also applies to: 49-50
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (15)
src/packages/cascader/cascader.taro.tsx(0 hunks)src/packages/popup/demos/h5/demo1.tsx(1 hunks)src/packages/popup/demos/h5/demo2.tsx(3 hunks)src/packages/popup/demos/taro/demo1.tsx(1 hunks)src/packages/popup/demos/taro/demo2.tsx(3 hunks)src/packages/popup/demos/taro/demo8.tsx(1 hunks)src/packages/popup/popup.scss(3 hunks)src/packages/popup/popup.taro.tsx(1 hunks)src/packages/popup/popup.tsx(1 hunks)src/packages/timeselect/demos/taro/demo1.tsx(0 hunks)src/packages/timeselect/demos/taro/demo2.tsx(0 hunks)src/packages/timeselect/demos/taro/demo3.tsx(0 hunks)src/packages/timeselect/timeselect.taro.tsx(0 hunks)src/packages/timeselect/timeselect.tsx(0 hunks)src/styles/variables.scss(1 hunks)
💤 Files with no reviewable changes (6)
- src/packages/timeselect/demos/taro/demo3.tsx
- src/packages/timeselect/demos/taro/demo1.tsx
- src/packages/timeselect/demos/taro/demo2.tsx
- src/packages/timeselect/timeselect.tsx
- src/packages/cascader/cascader.taro.tsx
- src/packages/timeselect/timeselect.taro.tsx
🔇 Additional comments (9)
src/packages/popup/demos/h5/demo1.tsx (1)
4-6: 组件重命名和状态变量更新符合最佳实践
组件名称从 Demo1 更改为 Demo 使代码更简洁。状态变量的命名 showIcon 清晰地表达了其用途。
Also applies to: 28-28
src/packages/popup/demos/taro/demo8.tsx (1)
16-21: 验证lockScroll属性在不同场景下的效果
需要确保lockScroll属性在各种复杂的滚动场景下都能正常工作。
✅ Verification successful
lockScroll属性实现已经过验证，可以安全使用
根据代码搜索结果，我们可以确认：
lockScroll属性在Popup组件中已经完整实现，默认值为true- 该属性通过
useLockScroll/useLockScrollTaro钩子函数处理滚动锁定逻辑 - 在Taro环境下，还会自动转换为
catchMove属性来阻止滚动穿透 - 该属性已在多个组件（Dialog、Menu、Toast等）中得到广泛验证
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索其他使用lockScroll的组件，分析实现方式的一致性
rg -l "lockScroll" --type tsx
Length of output: 57
Script:
#!/bin/bash
# 搜索使用lockScroll的组件实现
rg "lockScroll" -A 5
Length of output: 35550
Script:
#!/bin/bash
# 查找所有可能包含lockScroll的文件
fd . -e ts -e tsx -e js -e jsx --exec rg "lockScroll" {}
Length of output: 1988
Script:
#!/bin/bash
# 使用ast-grep搜索包含lockScroll属性的JSX元素
ast-grep --pattern 'lockScroll={$_}'
Length of output: 38
src/packages/popup/demos/h5/demo2.tsx (2)
9-9: 新增状态变量管理居中弹出层
状态管理的实现符合 React Hooks 的最佳实践。
37-42: 新增居中弹出层的触发按钮
Cell 组件的使用恰当，标题和点击事件处理符合规范。
src/packages/popup/popup.taro.tsx (2)
286-286: 代码重构优化！
将 Fragment 语法简化为直接渲染 popup 组件，提高了代码的可读性。
292-292: 条件语句优化！
使用三元运算符替代 if-else 语句，使代码更简洁。
src/packages/popup/popup.tsx (2)
288-288: 代码重构优化！
将 Fragment 语法简化为直接渲染 popup 组件，保持与 Taro 版本一致的代码风格。
294-294: 条件语句优化！
使用三元运算符替代 if-else 语句，与 Taro 版本保持一致的代码风格。
src/styles/variables.scss (1)
1065-1065: 样式变量标准化优化！
- 弹出层圆角使用设计系统的
$radius-xl变量 - 标题样式更新：
- 字体大小改用
$font-size-l - 新增字体粗细设置为
$font-weight-bold
- 字体大小改用
- 描述文本字体大小更新为
$font-size-base
这些更改提高了组件与设计系统的一致性。
Also applies to: 1070-1074, 1078-1078
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #2844 +/- ##
=============================================
+ Coverage 84.56% 85.84% +1.27%
=============================================
Files 273 271 -2
Lines 18593 18427 -166
Branches 2691 2726 +35
=============================================
+ Hits 15723 15818 +95
+ Misses 2865 2604 -261
Partials 5 5 ☔ View full report in Codecov by Sentry. |
注意主标题和副标题的间距，关闭按钮的大小
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/packages/popup/demo.taro.tsx (1)
83-83: 类名逻辑优化得当className 的逻辑变更使得类名分配更加明确和环境特定。建议为 'web' 类添加注释说明其用途。
建议添加如下注释:
+ // web 环境下添加特定样式类 <ScrollView className={`demo ${Taro.getEnv() === 'WEB' ? 'web' : ''}`}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
src/packages/timeselect/__test__/__snapshots__/timeselect.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
src/packages/popup/demo.taro.tsx(1 hunks)src/packages/popup/demos/taro/demo2.tsx(3 hunks)src/packages/popup/popup.scss(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/popup/demos/taro/demo2.tsx
🔇 Additional comments (5)
src/packages/popup/popup.scss (5)
25-25: 标题和描述文本的颜色层次优化合理
使用 $color-title 和 $color-text 变量改善了文本的视觉层次，符合设计规范。
Also applies to: 31-33
36-41: 左侧标题定位方式合理
使用绝对定位和设计系统变量控制左侧内容位置，实现了更精确的布局控制。
44-51: 右侧图标样式优化合理
- 使用与左侧一致的定位方式
- 颜色更新为标题色值，提升视觉一致性
- 添加鼠标手型提升交互体验
88-88: 建议优化底部弹出层最大高度设置
当前设置可能在某些机型上出现底部安全区域遮挡问题。
建议参考之前的优化建议：
- max-height: 87%;
+ max-height: calc(87% - env(safe-area-inset-bottom));5-5: 建议验证最小高度增加的影响
最小高度从 26% 增加到 46% 的变更可能会影响在小屏幕设备上的显示效果。
请验证此更改在各种屏幕尺寸下的表现：
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/packages/overlay/overlay.scss (1)
64-69: 建议添加RTL相关注释为了提高代码的可维护性，建议在RTL样式块之前添加注释说明。
+// RTL support [dir='rtl'] .nut-overlay, .nut-rtl .nut-overlay { left: auto; right: 0; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/packages/overlay/overlay.scss(1 hunks)src/packages/popup/popup.scss(4 hunks)src/packages/popup/popup.taro.tsx(4 hunks)src/packages/popup/popup.tsx(4 hunks)src/styles/variables.scss(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/packages/popup/popup.scss
- src/packages/popup/popup.taro.tsx
- src/packages/popup/popup.tsx
- src/styles/variables.scss
🔇 Additional comments (1)
src/packages/overlay/overlay.scss (1)
65-69: RTL样式实现正确！
RTL样式选择器和属性设置符合右到左布局的要求，同时支持了属性选择器和类选择器两种方式。
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/packages/address/__test__/__snapshots__/address.spec.tsx.snapis excluded by!**/*.snapsrc/packages/timeselect/__test__/__snapshots__/timeselect.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
src/packages/cascader/cascader.taro.tsx(0 hunks)src/packages/popup/__tests__/popup.spec.tsx(2 hunks)src/packages/popup/demo.taro.tsx(1 hunks)src/packages/popup/popup.scss(4 hunks)src/packages/popup/popup.taro.tsx(3 hunks)src/packages/popup/popup.tsx(3 hunks)src/styles/variables.scss(1 hunks)
💤 Files with no reviewable changes (1)
- src/packages/cascader/cascader.taro.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- src/packages/popup/tests/popup.spec.tsx
- src/packages/popup/demo.taro.tsx
- src/packages/popup/popup.taro.tsx
- src/packages/popup/popup.scss
- src/styles/variables.scss
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/popup/popup.tsx
[warning] 292-295: src/packages/popup/popup.tsx#L292-L295
Added lines #L292 - L295 were not covered by tests
[warning] 299-301: src/packages/popup/popup.tsx#L299-L301
Added lines #L299 - L301 were not covered by tests
🔇 Additional comments (2)
src/packages/popup/popup.tsx (2)
194-203: 关于关闭图标渲染的重构
将关闭图标的渲染逻辑抽离成独立函数是个很好的改进，提高了代码的可维护性。
284-284: 可见性控制逻辑优化
使用三元运算符简化了可见性控制逻辑，提高了代码可读性。
| <div className={`${classPrefix}-title-wrapper`}> | ||
| {title && ( | ||
| <div className={`${classPrefix}-title-title`}>{title}</div> | ||
| )} | ||
| {description && ( | ||
| <div className={`${classPrefix}-title-description`}> | ||
| <div | ||
| className={`${classPrefix}-title-description ${title ? `${classPrefix}-title-description-gap` : ''}`} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
标题结构优化建议
新增的 title-wrapper 结构改进了布局，但根据PR评论中提到的间距问题，建议调整以下样式：
<div className={`${classPrefix}-title-description ${
- title ? `${classPrefix}-title-description-gap` : ''
+ title ? `${classPrefix}-title-description-gap-v15` : ''
}`}>请确保在对应的 SCSS 文件中定义新的间距类。
Committable suggestion skipped: line range outside the PR's diff.
| <div className={closeClasses} onClick={onHandleClickCloseIcon}> | ||
| {React.isValidElement(closeIcon) ? closeIcon : <Close />} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
关闭按钮样式调整建议
根据PR评论反馈，关闭按钮的尺寸需要调整。
建议在样式中添加以下调整：
+ .nut-popup-title-right .nutui-icon {
+ font-size: 16px;
+ }Committable suggestion skipped: line range outside the PR's diff.
| const resolveContainer = (getContainer: Teleport | undefined) => { | ||
| const container = | ||
| typeof getContainer === 'function' ? getContainer() : getContainer | ||
| return container || document.body | ||
| } | ||
|
|
||
| const renderToContainer = (getContainer: Teleport, node: ReactElement) => { | ||
| if (getContainer) { | ||
| const container = resolveContainer(getContainer) | ||
| return createPortal(node, container) as ReactPortal | ||
| } | ||
| return node | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
需要补充容器处理相关的测试用例
经过验证，当前 popup.spec.tsx 中虽然有测试 portal 相关的基本功能，但缺少对新增的 resolveContainer 和 renderToContainer 函数的完整测试覆盖。
建议添加以下测试场景：
- 使用函数返回自定义容器元素
- 直接传入 DOM 元素作为容器
- 未传入容器时使用 document.body
- 容器为 null 时的处理
🔗 Analysis chain
容器处理逻辑需要补充测试
新增的容器处理逻辑 resolveContainer 和 renderToContainer 函数缺少测试覆盖。
建议添加以下测试场景：
- 自定义容器函数
- 自定义容器元素
- 默认容器（document.body）
- portal 为 null 的情况
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否已存在相关测试
rg -l "resolveContainer|renderToContainer" "src/packages/popup/__tests__"
Length of output: 73
Script:
#!/bin/bash
# 检查测试文件是否存在
fd -t f . "src/packages/popup/__tests__"
# 检查是否有其他相关测试文件
rg -l "popup.*test" .
# 检查当前的测试覆盖情况
rg -A 5 "test|describe|it" "src/packages/popup/__tests__"
Length of output: 12884
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 292-295: src/packages/popup/popup.tsx#L292-L295
Added lines #L292 - L295 were not covered by tests
[warning] 299-301: src/packages/popup/popup.tsx#L299-L301
Added lines #L299 - L301 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/packages/address/address.tsx (1)
124-135: 小建议：考虑避免使用硬编码颜色。这里使用了 "#cccccc" 作为箭头图标的颜色，可能会与其他主题或样式冲突。建议使用主题变量或通过 CSS classes 统一管理颜色值，从而提高可维护性和灵活性。
📜 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 (1)
src/packages/address/address.tsx(1 hunks)
| if (custom) { | ||
| return ( | ||
| <div className={`${classPrefix}-left-icon`} onClick={onSwitchModule}> | ||
| {React.isValidElement(backIcon) ? ( | ||
| backIcon | ||
| ) : ( | ||
| <ArrowLeft color="#cccccc" /> | ||
| )} | ||
| </div> | ||
| ) | ||
| } | ||
| return null |
There was a problem hiding this comment.
建议：在回调中使用最新状态以避免逻辑错误
当前的 onSwitchModule 函数在更新 currentType 之后，才调用了 onSwitch 回调，但回调参数仍然是之前旧的 currentType 值，这可能导致不符合预期的行为。建议先用一个局部变量存储新的类型，再统一更新并调用回调，以确保回调函数拿到最新的类型值。
可以参考以下修正示例：
-const onSwitchModule = () => {
- if (currentType === 'exist') {
- setCurrentType('custom')
- } else {
- setCurrentType('exist')
- }
- onSwitch && onSwitch({ type: currentType })
+const onSwitchModule = () => {
+ const newType = currentType === 'exist' ? 'custom' : 'exist'
+ setCurrentType(newType)
+ onSwitch && onSwitch({ type: newType })
}📝 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.
| if (custom) { | |
| return ( | |
| <div className={`${classPrefix}-left-icon`} onClick={onSwitchModule}> | |
| {React.isValidElement(backIcon) ? ( | |
| backIcon | |
| ) : ( | |
| <ArrowLeft color="#cccccc" /> | |
| )} | |
| </div> | |
| ) | |
| } | |
| return null | |
| if (custom) { | |
| return ( | |
| <div className={`${classPrefix}-left-icon`} onClick={onSwitchModule}> | |
| {React.isValidElement(backIcon) ? ( | |
| backIcon | |
| ) : ( | |
| <ArrowLeft color="#cccccc" /> | |
| )} | |
| </div> | |
| ) | |
| } | |
| return null |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/packages/address/__test__/address.spec.tsx (1)
120-125: 测试用例命名重复，建议在语义层面进行区分
当前新增的测试用例与上面已存在的“Address: show custom”同名，仅在内部添加了“custom”属性。建议在命名上进一步细化，比如“Address: show custom prop enabled”，以避免混淆、提高可读性和可维护性。
📜 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 (1)
src/packages/address/__test__/address.spec.tsx(1 hunks)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/packages/timeselect/__test__/timeselect.spec.tsx (3)
32-32: 快照测试可加大场景覆盖现仅对容器进行一次快照测试，可以考虑补充更多场景（例如组件关闭后）对 UI 状态进行快照，以更全面地保障组件变更的可预期性。
38-38: handleSelect 的测试逻辑建议补充新增的 handleSelect 回调目前仅在组件中声明，尚未在测试中断言其调用场景或参数。若需要确保此回调功能有效，可在测试中模拟触发并断言其调用次数与传参。
+ expect(handleSelect).toHaveBeenCalled() + // 或校验参数 + expect(handleSelect).toHaveBeenCalledWith(/* ... */)
64-70: 对关闭按钮的选择器和可见性断言可考虑更多边界情况当前通过固定类名 .nut-popup-title-right-top-right 找到关闭按钮并触发点击，测试用例能正确验证弹层 display 的更新。但若类名后续改动，可能导致测试失效。可以增加更语义化的选择器或容错处理。此外，若要严格断言“关闭弹层”，可再验证组件整体的 DOM 结构是否有相应掉或触发了 onSelect 回调。
src/packages/address/__test__/address.spec.tsx (2)
121-126: 建议增强自定义属性的测试覆盖当前测试仅验证了快照匹配，建议添加更具体的断言来验证自定义属性的实际效果。
test('Address: show custom', async () => { const { container } = render( <Address visible options={optionsDemo1} custom title="选择地址" /> ) + const customElement = container.querySelector('.nut-address-custom') + expect(customElement).toBeTruthy() + expect(customElement?.textContent).toBe('选择地址') expect(container.innerHTML).toMatchSnapshot() })
128-139: 建议完善自定义图标的测试用例当前测试仅验证了快照匹配，建议添加具体的断言来验证
Star图标是否正确渲染。test('Address: show custom icon', async () => { const { container } = render( <Address visible options={optionsDemo1} custom backIcon={<Star />} title="选择地址" /> ) + const iconElement = container.querySelector('.nut-icon-star') + expect(iconElement).toBeTruthy() expect(container.innerHTML).toMatchSnapshot() })
📜 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 (2)
src/packages/address/__test__/address.spec.tsx(2 hunks)src/packages/timeselect/__test__/timeselect.spec.tsx(3 hunks)
🔇 Additional comments (3)
src/packages/timeselect/__test__/timeselect.spec.tsx (2)
2-2: 使用 waitFor 位置恰当但请留意测试覆盖场景
目前在同一测试用例中存在两种方式（直接 expect、异步等待）的混合使用，整体测试流程合理，但建议确保在断言前后，都有足够的时间让渲染与事件完成。保持测试中的一致性有助于避免间歇性失败。
46-46: onSelect 属性增加后请核对业务逻辑
代码正确地将 handleSelect 传递给 onSelect，但需确认 TimeSelect 组件内部对 onSelect 的调用时机及入参是否与预期一致，并在业务层充分使用该回调。
src/packages/address/__test__/address.spec.tsx (1)
4-4: 确认图标组件的导入使用情况
导入了 Star 图标组件，但仅在一个测试用例中使用。建议检查是否需要在其他测试用例中也使用该图标，以提高测试覆盖率。
✅ Verification successful
Star 图标组件的使用是合理的
根据代码库分析，Star 图标组件在多个组件和测试文件中都有使用，包括 Button、Loading、Table 等组件。在 address.spec.tsx 中，Star 图标仅用于测试自定义返回图标的功能，这是一个合理且常见的测试用例，不需要在其他测试用例中重复使用。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查其他测试文件中是否也需要使用 Star 图标
rg "import.*Star.*from.*@nutui/icons-react" --type tsx
Length of output: 84
Script:
#!/bin/bash
# 使用更通用的搜索方式查找 Star 图标的使用情况
rg "import.*Star.*from.*@nutui/icons-react"
# 查找 Star 组件在测试文件中的使用
rg "Star" "src/packages/address/__test__/address.spec.tsx" -A 5
Length of output: 3810
| expect(container.innerHTML).toMatchSnapshot() | ||
| }) | ||
|
|
||
| test('Address: show custom icon', async () => { |
There was a problem hiding this comment.
注意：测试用例命名重复
存在两个同名的测试用例 "Address: show custom icon"，这可能会导致测试结果的混淆。建议为第二个测试用例使用更具描述性的名称。
-test('Address: show custom icon', async () => {
+test('Address: show custom icon with exist address', async () => {📝 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('Address: show custom icon', async () => { | |
| test('Address: show custom icon with exist address', async () => { |
| test('Address: show custom icon', async () => { | ||
| const onExistSelect = vi.fn() | ||
| const onSwitch = vi.fn() | ||
| const { container } = render( | ||
| <Address | ||
| visible | ||
| type="exist" | ||
| existList={existList} | ||
| options={optionsDemo1} | ||
| custom="选择其他地址" | ||
| onExistSelect={onExistSelect} | ||
| onSwitch={onSwitch} | ||
| /> | ||
| ) | ||
| expect(container.innerHTML).toMatchSnapshot() | ||
| const btn = container.querySelector('.nut-address-footer-btn') as Element | ||
| fireEvent.click(btn) | ||
| const leftIcon = container.querySelector('.nut-address-left-icon') as Element | ||
| expect(leftIcon).toBeTruthy() | ||
| fireEvent.click(leftIcon) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议优化事件处理函数的测试
当前测试中的事件处理函数 onExistSelect 和 onSwitch 虽然被模拟，但没有验证它们是否被正确调用。建议添加相应的断言。
test('Address: show custom icon', async () => {
const onExistSelect = vi.fn()
const onSwitch = vi.fn()
const { container } = render(
<Address
visible
type="exist"
existList={existList}
options={optionsDemo1}
custom="选择其他地址"
onExistSelect={onExistSelect}
onSwitch={onSwitch}
/>
)
expect(container.innerHTML).toMatchSnapshot()
const btn = container.querySelector('.nut-address-footer-btn') as Element
fireEvent.click(btn)
+ expect(onSwitch).toHaveBeenCalled()
const leftIcon = container.querySelector('.nut-address-left-icon') as Element
expect(leftIcon).toBeTruthy()
fireEvent.click(leftIcon)
+ expect(onExistSelect).toHaveBeenCalled()
})📝 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('Address: show custom icon', async () => { | |
| const onExistSelect = vi.fn() | |
| const onSwitch = vi.fn() | |
| const { container } = render( | |
| <Address | |
| visible | |
| type="exist" | |
| existList={existList} | |
| options={optionsDemo1} | |
| custom="选择其他地址" | |
| onExistSelect={onExistSelect} | |
| onSwitch={onSwitch} | |
| /> | |
| ) | |
| expect(container.innerHTML).toMatchSnapshot() | |
| const btn = container.querySelector('.nut-address-footer-btn') as Element | |
| fireEvent.click(btn) | |
| const leftIcon = container.querySelector('.nut-address-left-icon') as Element | |
| expect(leftIcon).toBeTruthy() | |
| fireEvent.click(leftIcon) | |
| }) | |
| test('Address: show custom icon', async () => { | |
| const onExistSelect = vi.fn() | |
| const onSwitch = vi.fn() | |
| const { container } = render( | |
| <Address | |
| visible | |
| type="exist" | |
| existList={existList} | |
| options={optionsDemo1} | |
| custom="选择其他地址" | |
| onExistSelect={onExistSelect} | |
| onSwitch={onSwitch} | |
| /> | |
| ) | |
| expect(container.innerHTML).toMatchSnapshot() | |
| const btn = container.querySelector('.nut-address-footer-btn') as Element | |
| fireEvent.click(btn) | |
| expect(onSwitch).toHaveBeenCalled() | |
| const leftIcon = container.querySelector('.nut-address-left-icon') as Element | |
| expect(leftIcon).toBeTruthy() | |
| fireEvent.click(leftIcon) | |
| expect(onExistSelect).toHaveBeenCalled() | |
| }) |
pnpm-lock.pr!oasis-cloud!2842.yaml
Outdated
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新特性
样式
文档
测试