feat: add info.item to onSelect and onClick callback #864
Conversation
- Add info.item to MenuInfo interface to expose menu item config (label, icon, disabled, extra, etc.) - Support both items config and children mode - Add unit tests for info.item in onSelect and onClick - Update README documentation Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough将结构化的菜单项快照作为 Changes菜单事件 info.item 支持
Sequence Diagram(s)sequenceDiagram
participant User
participant Menu
participant InternalMenuItem
participant EventHandler
User->>Menu: 点击菜单项
Menu->>InternalMenuItem: 转发点击事件
InternalMenuItem->>InternalMenuItem: getEventInfo() 构建 MenuInfo（包含 info.item）
InternalMenuItem->>EventHandler: 调用 onClick/onSelect，传递包含 info.item 的事件对象
估算审阅工作量🎯 3 (Moderate) | ⏱️ ~20 分钟 可能相关的 PRs
推荐审阅者
诗文
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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/utils/nodeUtil.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs+fabric@4.0.1_postcss@8.5.14/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #864 +/- ##
=======================================
Coverage 99.72% 99.72%
=======================================
Files 26 26
Lines 729 732 +3
Branches 201 204 +3
=======================================
+ Hits 727 730 +3
Misses 2 2 ☔ 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.
🧹 Nitpick comments (3)
tests/MenuItem.spec.tsx (2)
154-186: ⚡ Quick win建议验证向后兼容性
测试用例验证了新增的
info.item属性，但未明确验证原有的item: ReactComponent参数是否仍然存在。建议添加断言以确保向后兼容性，避免破坏现有代码。♻️ 建议的增强方案
fireEvent.click(container.querySelector('.rc-menu-item')!); expect(onSelect).toHaveBeenCalledWith( expect.objectContaining({ key: '1', + item: expect.anything(), // 验证向后兼容性 info: expect.objectContaining({ item: expect.objectContaining({ key: '1', label: 'Menu Item', }), }), }), ); expect(onClick).toHaveBeenCalledWith( expect.objectContaining({ key: '1', + item: expect.anything(), // 验证向后兼容性 info: expect.objectContaining({ item: expect.objectContaining({ key: '1', label: 'Menu Item', }), }), }), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/MenuItem.spec.tsx` around lines 154 - 186, 当前测试只断言事件回调中的 info.item（metadata）结构，未确认原来的 item: ReactComponent 参数仍存在，请在该测试（Menu, MenuItem, onSelect/onClick 调用断言处）为 onSelect 和 onClick 的调用结果另外添加断言以验证原始 item 参数仍然被传入；具体做法是在现有 expect 调用旁对两个回调的第一个参数断言包含 top-level item 字段且该字段为一个 React 元素/组件实例（例如使用 React.isValidElement 或 expect.any(Object) 做存在性/类型检查），以确保向后兼容性。
188-223: ⚡ Quick win建议验证向后兼容性
与上一个测试类似，建议添加对原有
item: ReactComponent参数的验证以确保向后兼容性。♻️ 建议的增强方案
fireEvent.click(container.querySelector('.rc-menu-item')!); expect(onSelect).toHaveBeenCalledWith( expect.objectContaining({ key: '1', + item: expect.anything(), // 验证向后兼容性 info: expect.objectContaining({ item: expect.objectContaining({ key: '1', label: 'Menu Item', }), }), }), ); expect(onClick).toHaveBeenCalledWith( expect.objectContaining({ key: '1', + item: expect.anything(), // 验证向后兼容性 info: expect.objectContaining({ item: expect.objectContaining({ key: '1', label: 'Menu Item', }), }), }), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/MenuItem.spec.tsx` around lines 188 - 223, Extend the test for onSelect/onClick to also verify backward compatibility with the legacy item-as-ReactComponent shape: when rendering <Menu items={[{ key: 'comp', label: <MyComp /> }]} /> (or add a second item whose label is a React element) trigger the click and assert that onSelect and onClick were called with info.item equal to that original React element/component instance (use expect.objectContaining or toEqual on the info.item value) so the Menu component still passes through the ReactComponent item unchanged; update the existing test case that uses onSelect/onClick and items to include this additional assertion referencing onSelect, onClick and the Menu items prop.src/MenuItem.tsx (1)
36-37: ⚡ Quick win复用
MenuInfo['info']类型，避免契约漂移
MenuItemProps.info与MenuInfo.info是同一结构，建议直接复用，避免后续字段扩展时双处漏改。♻️ 建议修改
export interface MenuItemProps extends Omit<MenuItemType, 'label' | 'key' | 'ref'>, @@ /** `@private` Origin item config from items prop */ - info?: { item: MenuItemType }; + info?: MenuInfo['info']; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MenuItem.tsx` around lines 36 - 37, MenuItemProps 中的 info 与 MenuInfo.info 结构相同，应复用类型以避免契约漂移；在 src/MenuItem.tsx 中将 MenuItemProps 的 info 类型从 `{ item: MenuItemType }` 改为复用 `MenuInfo['info']`（或直接引用 MenuInfo 类型中对应导出），并调整相应的导入/导出声明以确保类型可用（更新任何使用到 `MenuItemProps.info` 的地方以适配新引用类型）。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/MenuItem.tsx`:
- Around line 36-37: MenuItemProps 中的 info 与 MenuInfo.info 结构相同，应复用类型以避免契约漂移；在
src/MenuItem.tsx 中将 MenuItemProps 的 info 类型从 `{ item: MenuItemType }` 改为复用
`MenuInfo['info']`（或直接引用 MenuInfo 类型中对应导出），并调整相应的导入/导出声明以确保类型可用（更新任何使用到
`MenuItemProps.info` 的地方以适配新引用类型）。
In `@tests/MenuItem.spec.tsx`:
- Around line 154-186: 当前测试只断言事件回调中的 info.item（metadata）结构，未确认原来的 item:
ReactComponent 参数仍存在，请在该测试（Menu, MenuItem, onSelect/onClick 调用断言处）为 onSelect 和
onClick 的调用结果另外添加断言以验证原始 item 参数仍然被传入；具体做法是在现有 expect 调用旁对两个回调的第一个参数断言包含
top-level item 字段且该字段为一个 React 元素/组件实例（例如使用 React.isValidElement 或
expect.any(Object) 做存在性/类型检查），以确保向后兼容性。
- Around line 188-223: Extend the test for onSelect/onClick to also verify
backward compatibility with the legacy item-as-ReactComponent shape: when
rendering <Menu items={[{ key: 'comp', label: <MyComp /> }]} /> (or add a second
item whose label is a React element) trigger the click and assert that onSelect
and onClick were called with info.item equal to that original React
element/component instance (use expect.objectContaining or toEqual on the
info.item value) so the Menu component still passes through the ReactComponent
item unchanged; update the existing test case that uses onSelect/onClick and
items to include this additional assertion referencing onSelect, onClick and the
Menu items prop.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c6c77460-e83c-45eb-bf01-b31b392b2590
📒 Files selected for processing (5)
README.mdsrc/MenuItem.tsxsrc/interface.tssrc/utils/nodeUtil.tsxtests/MenuItem.spec.tsx
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds an 'info' property to the 'MenuInfo' interface and 'MenuItem' component, allowing 'onSelect' and 'onClick' callbacks to access the original item configuration. This change covers both 'items' and 'children' modes and includes updated documentation and tests. Reviewers suggested renaming the 'info' field in 'MenuInfo' to something more descriptive like 'itemConfig' to avoid redundant 'info.info' access patterns. Additionally, a suggestion was made to simplify the event info construction in 'MenuItem.tsx' and include more properties for better consistency.
|
@gemini-code-assist CR again |
There was a problem hiding this comment.
Code Review
This pull request introduces a new info property to the onSelect and onClick event callbacks, providing metadata about the menu item for both items-based and children-based configurations. The changes include updates to the MenuItem component, interface definitions, and documentation, along with new test cases. Review feedback suggests refining the omit utility import for better consistency and expanding the list of omitted keys when constructing the info object to strictly match the MenuItemInfo type definition.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/nodeUtil.tsx`:
- Line 58: The info object passed to the callback should use mergedKey instead
of the original possibly-undefined key so reported keys match the React key;
locate the JSX where info={{ item: { label, key, itemIcon: restProps?.itemIcon,
extra } }} is constructed and replace the inner key with mergedKey (the same
mergedKey used when setting the component's key) so info.item.key is always the
resolved fallback key.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7f4c3e2a-bc42-4003-93ff-290d4a2a8d8d
📒 Files selected for processing (3)
src/MenuItem.tsxsrc/interface.tssrc/utils/nodeUtil.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/MenuItem.tsx
- src/interface.ts
1 participant
Issue
Menu 导航菜单 希望在 onSelect，onClick 里获取传入的 item 的 label
背景
用户在使用 Menu 组件时，希望在
onSelect和onClick回调中获取菜单项的配置信息（如 label、icon、disabled、extra等）。目前只能获取到
key，无法获取其他配置属性。改动
在
MenuInfo接口中添加info: { item: MenuItemType }属性，用户可以通过info.info.item获取菜单项的完整配置。改动文件
src/interface.ts- 添加info类型定义src/MenuItem.tsx- 实现 info.item 的传递（支持 items 配置和 children 两种模式）src/utils/nodeUtil.tsx- items 模式传递 info proptests/MenuItem.spec.tsx- 添加单元测试README.md- 更新文档使用示例