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. Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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()的作用会完全阻断事件冒泡，请您确认在业务场景下父组件或其他外部逻辑是否需要捕获同一事件进行额外处理。请根据具体业务逻辑，确认后续是否需要调整，以免破坏依赖事件冒泡的父组件的功能。
我在想有没有必要开个discussion讨论下这个问题 因此改造后的渲染流程如下 open = true时，content展现，然后高度过渡 所以我觉得需要讨论下这部分的实现，我先说下我的想法
个人上是更倾向方式1，这样的话，在高版本浏览器中，组件将变得足够简单，性能也将更上一层楼，仅做状态同步即可 |
|
1 可以的，我也倾向渐进式。 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/Panel.tsx (1)
164-177: 更新了组件的HTML结构将原来的
div结构改为使用语义化的details和summary元素：
- 使用
details作为根容器，通过open属性控制展开状态- 使用
summary作为标题容器- 根据条件渲染内容部分
这种修改充分利用了HTML的语义化结构，不仅提高了可访问性，还能利用浏览器原生行为。同时通过
open={isActive}保持了对组件状态的受控特性。可能会影响动画效果，需要确认：
- 动画效果是否已恢复
details元素的原生展开/折叠行为是否与CSSMotion兼容
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
assets/index.less(1 hunks)src/Collapse.tsx(1 hunks)src/Panel.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Collapse.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Panel.tsx (1)
src/interface.ts (1)
CollapsePanelProps(45-67)
🔇 Additional comments (6)
src/Panel.tsx (6)
8-8: 更新了组件的根元素类型将组件的根元素从
div改为details，同时更新了转发引用类型从HTMLDivElement变为HTMLDetailsElement。这种修改更符合HTML语义化标准，利用了浏览器原生的可折叠组件功能。
35-42: 添加了浏览器特性检测使用
CSS.supportsAPI检测浏览器是否支持::details-content伪元素选择器，这决定了是使用CSS原生动画还是回退到CSSMotion。这是实现渐进增强策略的关键部分，能够在支持现代CSS特性的浏览器中提供更好的性能，同时保持向后兼容性。
哪些浏览器支持details::details-content伪元素选择器？
44-48: 更新了事件处理逻辑修改了点击事件处理函数的参数类型，并添加了
e.stopPropagation()防止事件冒泡。添加
stopPropagation()是必要的，可以防止事件冒泡导致details元素的原生点击事件与自定义处理逻辑冲突。
80-81: 条件性应用动画类名仅在浏览器支持
::details-content选择器时才将motionName应用到details元素上。这与渐进增强的方法一致，确保动画效果只在支持的浏览器中应用。
94-94: 泛化了header属性的类型定义将
headerProps的类型从HTMLDivElement更改为更通用的HTMLElement。这是一个合理的修改，因为现在header是
summary元素而不是div。
102-162: 重构内容渲染逻辑引入了
createPanelContent函数和基于浏览器特性的条件渲染逻辑：
- 抽象出内容创建逻辑到单独的函数
- 根据浏览器支持情况决定使用
CSSMotion还是原生CSS控制内容显示- 模拟CSSMotion的生命周期管理逻辑
这种设计能够在现代浏览器中利用原生功能实现更好的性能，同时为不支持的浏览器提供良好的回退方案。代码结构清晰，条件逻辑易于理解。
| list-style-position: outside; | ||
| interpolate-size: allow-keywords; | ||
| overflow: hidden; | ||
|
|
||
| &::details-content { | ||
| block-size: 0; | ||
| transition: block-size 0.6s, content-visibility 0.6s; | ||
| transition-behavior: allow-discrete; | ||
| } | ||
|
|
||
| &[open]::details-content { | ||
| block-size: auto; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
使用现代CSS特性实现动画过渡
代码引入了一些新的CSS特性来实现基于<details>元素的动画效果：
interpolate-size: allow-keywords;允许在尺寸关键词（如auto）和具体值之间进行插值动画::details-content伪元素选择器用于直接控制details内容区域的样式和动画- 使用
transition-behavior: allow-discrete;支持离散过渡效果
这些CSS特性是比较新的，需要注意浏览器兼容性问题。与Panel.tsx中的特性检测(CSS.supports('selector(details::details-content)'))相配合，可以实现渐进增强的用户体验。
建议添加一些注释说明这些属性的浏览器兼容性要求，或考虑为不支持这些特性的浏览器提供兼容性样式。
🤖 Prompt for AI Agents (early access)
In assets/index.less around lines 38 to 50, the CSS uses modern features like
interpolate-size, ::details-content, and transition-behavior which have limited
browser support. Add comments explaining the browser compatibility requirements
for these properties and consider adding fallback styles or alternative
approaches for browsers that do not support these features to ensure graceful
degradation.
|
|
或许可以移除CSSMotion，根据是否支持 |
关联antd ant-design/ant-design#48974
Summary by CodeRabbit
Chores
重构
新特性
@rc-component/np以支持项目功能。样式