-
-
Notifications
You must be signed in to change notification settings - Fork 262
refactor: migrate rc-overflow to @rc-component/overflow #832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough将依赖从 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 分钟
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-10-23T06:01:34.975ZApplied to files:
🔇 Additional comments (2)
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. Comment |
Summary of ChangesHello @yoyo837, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on a dependency update, migrating the project from the legacy Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request migrates the project from rc-overflow to the scoped package @rc-component/overflow. The changes are straightforward, involving updates to package.json and import paths in several components. There are also some minor stylistic formatting changes to interface definitions, which improve code readability. The changes appear correct and are consistent with migrating to the new scoped package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Menu.tsx (1)
392-407: focus 回退逻辑仍依赖 childList,可能选中未渲染项
focus逻辑在找不到mergedActiveKey时,会优先用focusableElements推导 key,但最终还回退到childList.find(node => !node.props.disabled)?.key。在有 overflow 的场景里,childList里可能包含未真实挂载到 DOM 的项,导致拿到的 key 在key2element中没有对应元素,从而聚焦失败。建议后续重构时将回退完全限制在
focusableElements/key2element的结果集合内,例如仅根据已存在的element2key映射选择第一个可聚焦元素,而不是再从childList做兜底。Based on learnings, ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
package.json(1 hunks)src/Menu.tsx(2 hunks)src/MenuItem.tsx(2 hunks)src/SubMenu/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-23T06:01:34.975Z
Learnt from: codewizardTM
Repo: react-component/menu PR: 818
File: src/Menu.tsx:415-431
Timestamp: 2025-10-23T06:01:34.975Z
Learning: In src/Menu.tsx focus logic, defaultFocusKey should only use keys from focusableElements/key2element, not from childList.find(), because childList items might not be rendered in DOM (e.g., in overflow menu) which would cause focus to fail.
Applied to files:
src/Menu.tsx
🧬 Code graph analysis (2)
src/MenuItem.tsx (1)
src/interface.ts (1)
MenuItemType(44-63)
src/Menu.tsx (1)
src/index.ts (1)
MenuProps(25-25)
🔇 Additional comments (6)
src/SubMenu/index.tsx (1)
3-3: Overflow 导入路径更新看起来正确这里将
Overflow的来源切到@rc-component/overflow,与package.json中的新依赖保持一致,后文仍按<Overflow.Item>使用,语义不变。建议在本分支上跑一遍构建和现有测试,确认新包的默认导出接口与原rc-overflow兼容。src/Menu.tsx (2)
3-3: Overflow 导入路径更新与依赖一致
Overflow改为从@rc-component/overflow引入,与依赖清单中新增的包对齐;下文继续使用Overflow组件及静态成员(如Overflow.INVALIDATE/RESPONSIVE),只要新包保持兼容导出即可无行为差异。建议在本分支上跑一遍构建和测试,确认类型检查和运行时均正常。
53-56: MenuProps 继承部分仅为排版优化这里把
MenuProps的extends Omit<...>拆成多行,只是改善可读性和避免超长行,对外暴露的MenuProps类型含义和行为都没有变化,可以保持现状。src/MenuItem.tsx (2)
2-2: MenuItem 中 Overflow 导入与依赖保持一致这里把
Overflow改为从@rc-component/overflow引入,下方LegacyMenuItem仍按<Overflow.Item>使用,接口期望与原先rc-overflow一致。整体修改与其他文件、package.json 中的迁移方向统一,逻辑上没有问题。建议在本分支跑一下构建和测试,确认新包的默认导出上仍包含.Item等静态成员。
19-21: MenuItemProps 继承列表仅作格式化调整
MenuItemProps的extends现在拆成多行,阅读性更好,但仍然只是从MenuItemType中省略'label' | 'key' | 'ref',对外暴露的 props 形态与之前保持一致,无需额外改动。package.json (1)
47-52: Dependency migration fromrc-overflowto@rc-component/overflowis complete and consistentThe dependencies block correctly adds
@rc-component/overflow@^1.0.0and removes the oldrc-overflow. All Overflow imports in the codebase (MenuItem.tsx, Menu.tsx, SubMenu/index.tsx) have been updated to reference the new package, with no lingering references torc-overflowremaining.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #832 +/- ##
=======================================
Coverage 99.72% 99.72%
=======================================
Files 26 26
Lines 727 727
Branches 199 199
=======================================
Hits 725 725
Misses 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.