refactor: 重构collapse为details实现 #362
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次修改包含以下变更：
Changes
Sequence Diagram(s)sequenceDiagram
participant U as 用户
participant C as Collapse组件
participant R as React.startTransition
participant S as setActiveKey
U->>C: 点击折叠项
C->>R: 调用状态更新包装
R->>S: 执行状态更新
S-->>R: 更新完成
R-->>C: 渲染更新完成
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
src/Panel.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 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 (
|
|
ci 挂了 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #362 +/- ##
==========================================
+ Coverage 99.12% 99.13% +0.01%
==========================================
Files 5 5
Lines 114 116 +2
Branches 43 43
==========================================
+ Hits 113 115 +2
Misses 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fixed 少个包 |
|
收起动画丢了。 |
![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
🧹 Nitpick comments (1)
src/Panel.tsx (1)
45-48: 考虑重新评估 ARIA 角色的必要性使用
details和summary元素后，这些元素已经具有原生的语义和辅助功能。可以考虑是否仍然需要设置role="tab"、role="button"和其他 ARIA 属性，因为这些可能与元素的原生语义冲突。HTML5 的
details和summary元素原生支持可访问性，无需额外的 ARIA 角色。Is it necessary to add ARIA roles to details and summary elements?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Panel.tsx(5 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/Panel.tsx (2)
src/interface.ts (1) (1)
CollapsePanelProps(45-67)src/index.tsx (1) (1)
CollapsePanelProps(3-3)
🔇 Additional comments (3)
src/Panel.tsx (3)
8-8: 使用更符合语义的 HTML 元素将
forwardRef类型从HTMLDivElement更改为HTMLDetailsElement是一个很好的改进，这符合 HTML 语义化的最佳实践，details元素本身就是为折叠内容设计的。
83-83: 类型定义更新合理将
headerProps的类型从HTMLDivElement更改为更通用的HTMLElement是合理的，因为现在头部使用的是summary元素。
36-39:❓ Verification inconclusive
请确认事件传播行为是否符合预期
添加
e.stopPropagation()阻止了事件冒泡，这在某些情况下是必要的，但请确认这不会破坏依赖事件冒泡的父组件功能。
🏁 Script executed:
#!/bin/bash # 查找可能依赖事件冒泡的代码 rg -A 5 "onItemClick" --type ts --type tsxLength of output: 70
事件冒泡行为验证建议
文件：src/Panel.tsx (行 36-39)
代码示例：onClick: (e: React.MouseEvent) => { onItemClick?.(panelKey); e.stopPropagation(); },说明：
- 此处调用
e.stopPropagation()用于阻止事件冒泡，请确认这一行为不会对依赖事件冒泡机制的父组件造成影响。- 建议检查项目中其他对
onItemClick的引用，以确保没有依赖事件继续冒泡的逻辑。请运行上面的脚本，并手动确认是否有其他地方依赖事件冒泡逻辑，以便确保代码行为符合预期。
🏁 Script executed:
#!/bin/bash # 重新搜索所有文件中 onItemClick 的使用情况，不限制文件类型 rg -A 5 "onItemClick" || echo "未发现 onItemClick 使用记录"Length of output: 8169
请确认是否有依赖事件冒泡的父组件逻辑
通过搜索代码可知，代码在
src/Panel.tsx第 36-39 行设置了如下点击处理逻辑：onClick: (e: React.MouseEvent) => { onItemClick?.(panelKey); e.stopPropagation(); },
- 脚本搜索显示，
onItemClick在多个文件（如src/interface.ts,src/hooks/useItems.tsx和src/Panel.tsx）中作为回调使用。测试用例也验证了在点击时onItemClick正确触发，并且在禁用或特定场景下，行为符合预期。- 从现有代码和测试来看，没有明显依赖点击事件冒泡的父组件逻辑。但由于
e.stopPropagation()的作用会完全阻断事件冒泡，请您确认在业务场景下父组件或其他外部逻辑是否需要捕获同一事件进行额外处理。请根据具体业务逻辑，确认后续是否需要调整，以免破坏依赖事件冒泡的父组件的功能。
| <details {...resetProps} ref={ref} className={collapsePanelClassNames} open={isActive}> | ||
| <summary {...headerProps}> |
There was a problem hiding this comment.
HTML 结构重构可能影响动画效果
将 div 替换为 details 和 summary 是符合语义化的改进，但根据 PR 描述中的评论，这一变更导致折叠动画效果丢失。虽然代码仍然使用 CSSMotion 组件，但 details 元素的原生行为可能与自定义动画产生冲突。
需要确认：
- 动画效果是否已恢复
details元素的原生展开/折叠行为是否与CSSMotion兼容
建议添加必要的样式或逻辑以确保动画效果正常工作。可以考虑：
- 使用 CSS 禁用
details的默认动画 - 调整
CSSMotion配置以适应新的 HTML 结构
/* 可能需要添加的样式 */
details {
transition: none; /* 禁用默认动画 */
}Also applies to: 102-102, 128-128
我在想有没有必要开个discussion讨论下这个问题 因此改造后的渲染流程如下 open = true时，content展现，然后高度过渡 所以我觉得需要讨论下这部分的实现，我先说下我的想法
个人上是更倾向方式1，这样的话，在高版本浏览器中，组件将变得足够简单，性能也将更上一层楼，仅做状态同步即可 |
|
1 可以的，我也倾向渐进式。 |
关联antd ant-design/ant-design#48974
Summary by CodeRabbit
Chores
重构
新特性
@rc-component/np以支持项目功能。