fix(MenuItem): Fix style props not working error#56041
fix(MenuItem): Fix style props not working error#56041meet-student merged 2 commits intoant-design:masterfrom
Conversation
|
|
👁 Visual Regression Report for PR #56041 Passed ✅
🎊 Congrats! No visual-regression diff found. 
|
|
|
Summary of ChangesHello @Wxh16144, 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 resolves a bug in the 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
|
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. 摘要MenuItem 组件的样式优先级调整：根元素现在合并自身计算的样式与传入的 props.style，Icon 元素的样式组合顺序已交换以改变样式覆盖优先级。外部样式现在可以更容易地覆盖内部默认值。添加了测试用例验证 MenuItem 的 style 属性支持。 更改内容
代码审查工作量评估🎯 2 (Simple) | ⏱️ ~10 分钟
诗
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Code Review
This pull request effectively resolves an issue where the style prop on MenuItem components was not being applied. The solution correctly merges the user-provided styles with the default component styles, ensuring custom styles take precedence. The changes also correctly handle styles and class names for the item's icon. A comprehensive test case has been added to validate the fix. I have one minor suggestion in the new test file to improve type safety.
| const expected: any = { | ||
| One: 'rgb(255, 0, 0)', | ||
| 'Two-One': 'rgb(0, 128, 0)', | ||
| 'Two-Two': 'rgb(0, 0, 255)', | ||
| }; |
There was a problem hiding this comment.
To improve type safety and code clarity, it's better to avoid using any. You can define a more specific type for the expected object, such as Record<string, string>.
| const expected: any = { | |
| One: 'rgb(255, 0, 0)', | |
| 'Two-One': 'rgb(0, 128, 0)', | |
| 'Two-Two': 'rgb(0, 0, 255)', | |
| }; | |
| const expected: Record<string, string> = { | |
| One: 'rgb(255, 0, 0)', | |
| 'Two-One': 'rgb(0, 128, 0)', | |
| 'Two-Two': 'rgb(0, 0, 255)', | |
| }; |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/menu/MenuItem.tsx(1 hunks)components/menu/__tests__/semantic.test.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always use TypeScript with strict type checking
Never useanytype - define precise types instead
Use interfaces (not type aliases) for object structures
Export all public interface types
Prefer union types over enums, useas constfor constants
Use early returns to improve readability
Support tree shaking
Follow import order: React → dependencies → antd components → custom components → types → styles
Prefer antd built-in components over external dependencies
Pass all ESLint and TypeScript checks
No console errors or warnings
**/*.{ts,tsx}: Use camelCase for property names
All components and functions must provide accurate type definitions
Avoid usinganytype, define types as precisely as possible
Use interface rather than type alias for defining object structures
All TypeScript should compile without errors or warnings
Use generics appropriately to enhance type flexibility
Use intersection types (&) to merge multiple types
Use literal union types to define limited option sets
Avoid using enum, prefer union types andas const
Rely on TypeScript's type inference as much as possible
Use type assertions (as) only when necessary
Complex data structures should be split into multiple interface definitions
Avoid outdated APIs and keep updated to new recommended usage
Files:
components/menu/__tests__/semantic.test.tsxcomponents/menu/MenuItem.tsx
components/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
components/**/*.tsx: Component props interfaces should be namedComponentNameProps
Component ref types should useReact.ForwardRefRenderFunction
Use functional components with hooks exclusively (no class components)
Apply performance optimizations with React.memo, useMemo, useCallback appropriately
Support server-side rendering
Components must support ref forwarding with structure including nativeElement, focus, and other methods
Use PascalCase for component names
Use camelCase for props with specific patterns:default+PropNamefor defaults,forceRenderfor force rendering,openinstead ofvisiblefor panel state,show+PropNamefor display toggles,PropName+ablefor capabilities,dataSourcefor data source,disabledfor disabled state,extrafor additional content,iconfor icons,triggerfor triggers,classNamefor CSS classes
Useon+EventNamepattern for event handlers (e.g.,onClick,onChange)
Useon+SubComponentName+EventNamepattern for sub-component events
Use complete names, never abbreviations in prop naming
Optimize for minimal re-renders
UseuseLocalehook fromcomponents/locale/index.tsx
Support accessibility (WCAG 2.1 AA)
Files:
components/menu/__tests__/semantic.test.tsxcomponents/menu/MenuItem.tsx
components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never hardcode colors, sizes, or spacing values
components/**/*.{ts,tsx}: Use TypeScript and React for component development
Use functional components and hooks, avoid class components
Use PascalCase for component names
Use complete names rather than abbreviations in component naming
Initialize properties should usedefault+PropNamenaming pattern
Panel opening should useopenprop instead ofvisible
Event handlers should followon+EventNamenaming pattern
Child component events should followon+SubComponentName+EventNamenaming pattern
Components should provide arefattribute withnativeElementandfocusmethods
Export all public interface types for user convenience
Component props should use interface definition namedComponentNameProps
Use CSS transitions for simple animations, rc-motion for complex animations
Respect user's prefers-reduced-motion setting for animations
Follow WCAG 2.1 AA level accessibility standards for styling
Achieve 100% test coverage
UseuseLocalehook fromcomponents/locale/index.tsxto get localization configuration
Use React.memo, useMemo, and useCallback appropriately for performance optimization
UseReact.ForwardRefRenderFunctionfor component ref types
Callback function types should have explicit parameter and return value definitions
Component state should have dedicated interfaces (e.g., ComponentNameState)
Components should display well on different screen sizes (responsive design)
Components should support right-to-left (RTL) reading direction
Support page zoom to 200% with normal layout
Avoid animations that cause flickering
Ensure code runs normally with no console errors
Avoid unnecessary re-renders
Support Tree Shaking for bundle optimization
Files:
components/menu/__tests__/semantic.test.tsxcomponents/menu/MenuItem.tsx
**/__tests__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/__tests__/**/*.test.{ts,tsx}: Write comprehensive tests using Jest and React Testing Library
Include snapshot tests for UI components
**/__tests__/**/*.test.{ts,tsx}: Test files should be named index.test.tsx or xxx.test.tsx and placed in tests directory
Use snapshot testing for UI components
Files:
components/menu/__tests__/semantic.test.tsx
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation
Files:
components/menu/__tests__/semantic.test.tsxcomponents/menu/MenuItem.tsx
**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain cross-browser compatibility
Files:
components/menu/__tests__/semantic.test.tsxcomponents/menu/MenuItem.tsx
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js}: Use early returns to improve code readability
Pass all ESLint and TypeScript checks
Files:
components/menu/__tests__/semantic.test.tsxcomponents/menu/MenuItem.tsx
**/__tests__/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Jest and React Testing Library for unit tests
Files:
components/menu/__tests__/semantic.test.tsx
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/style/**/*.{ts,tsx} : Use `ant-design/cssinjs` as the styling solution
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/style/**/*.{ts,tsx} : Ensure sufficient color contrast and do not rely on color alone to convey information
Applied to files:
components/menu/__tests__/semantic.test.tsx
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/style/**/*.{ts,tsx} : Token modifications should cascade downward to ensure design system consistency
Applied to files:
components/menu/__tests__/semantic.test.tsxcomponents/menu/MenuItem.tsx
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/style/*.{ts,tsx} : Component Token naming format should follow: `variant (optional)` + `semantic part` + `semantic part variant (optional)` + `css property` + `size/disabled (optional)`
Applied to files:
components/menu/__tests__/semantic.test.tsx
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to components/**/style/*.ts : Support both light and dark themes
Applied to files:
components/menu/__tests__/semantic.test.tsx
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/style/**/*.{ts,tsx} : All components must support dark mode
Applied to files:
components/menu/__tests__/semantic.test.tsx
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/style/**/*.{ts,tsx} : Provide clear visual indication for focus state
Applied to files:
components/menu/__tests__/semantic.test.tsx
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/style/**/*.{ts,tsx} : Avoid hardcoding colors, sizes, spacing values; use design Tokens instead
Applied to files:
components/menu/__tests__/semantic.test.tsx
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Write comprehensive tests using Jest and React Testing Library
Applied to files:
components/menu/__tests__/semantic.test.tsx
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/style/**/*.{ts,tsx} : Component-level Token naming should follow: Component + property name (e.g., buttonPrimaryColor)
Applied to files:
components/menu/__tests__/semantic.test.tsx
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/*.{ts,tsx} : Follow WCAG 2.1 AA level accessibility standards for styling
Applied to files:
components/menu/__tests__/semantic.test.tsx
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/demo/*.tsx : Demo code import order: React → dependencies → antd components → custom components → types → styles
Applied to files:
components/menu/__tests__/semantic.test.tsx
📚 Learning: 2025-07-30T12:46:13.180Z
Learnt from: gregor-mueller
Repo: ant-design/ant-design PR: 54539
File: components/menu/interface.ts:8-10
Timestamp: 2025-07-30T12:46:13.180Z
Learning: In Ant Design Menu component's DataAttributes type, using `any` for data attribute values is correct because HTML data attributes can accept any value type (boolean, null, undefined, objects, etc.) and the restrictive `string | number` type breaks type inference when MenuItemType is used on objects with various data attribute value types.
Applied to files:
components/menu/__tests__/semantic.test.tsx
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to components/**/style/*.ts : Use design tokens from the Ant Design token system
Applied to files:
components/menu/__tests__/semantic.test.tsx
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/style/**/*.{ts,tsx} : Use class selectors instead of tag selectors for style overrides to improve specificity
Applied to files:
components/menu/MenuItem.tsx
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/style/**/*.{ts,tsx} : Components should use global Tokens and component-level Tokens for styling
Applied to files:
components/menu/MenuItem.tsx
🧬 Code graph analysis (1)
components/menu/MenuItem.tsx (2)
.dumi/theme/builtins/Badge/index.tsx (1)
props(17-24)components/_util/reactNode.ts (1)
cloneElement(25-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: visual-diff snapshot (1/2)
- GitHub Check: visual-diff snapshot (2/2)
- GitHub Check: test-react-legacy (18, 1/2)
- GitHub Check: test lib/es module (es, 2/2)
- GitHub Check: test lib/es module (es, 1/2)
- GitHub Check: size
- GitHub Check: lint
- GitHub Check: test lib/es module (lib, 1/2)
- GitHub Check: test-node
- GitHub Check: build
- GitHub Check: test-react-legacy (18, 2/2)
- GitHub Check: test-react-latest (dom, 1/2)
- GitHub Check: test-react-latest (dom, 2/2)
- GitHub Check: build preview
- GitHub Check: build
🔇 Additional comments (3)
components/menu/MenuItem.tsx (3)
106-109: 样式合并顺序正确，成功修复了问题。将
props.style放在展开运算符的最后位置，允许用户传入的样式覆盖内部默认样式。这正确恢复了 v5 的行为，修复了 issue #56017 中报告的问题。
116-116: 验证此更改是否在问题范围内。此更改将
oriProps.className添加到图标的类名列表中。虽然这与主要样式修复保持一致，但 PR 描述和关联的 issue #56017 主要关注 MenuItem 的style属性问题，而不是图标的 className。请确认：
- 此更改是否是修复 #56017 所必需的？
- 是否有相关的图标 className 不生效的问题需要一并修复？
如果此更改超出了当前问题的范围，建议在单独的 PR 中处理，以保持变更的原子性。
120-120: 验证此更改是否在问题范围内。此更改将
oriProps.style添加到图标的样式合并中。与 line 116 的 className 更改类似，这个变更可能超出了 issue #56017 的原始范围。请确认：
- 此更改是否是修复 MenuItem style 不生效问题所必需的？
- 是否有用户报告图标样式不生效的相关问题？
建议在 PR 描述中明确说明这些额外的改进，或考虑将它们分离到独立的 PR 中。
| // https://github.com/ant-design/ant-design/issues/56017 | ||
| it('support MenuItem style', () => { | ||
| const items = [ | ||
| { label: 'One', key: 'one', style: { color: 'red' } }, | ||
| { | ||
| label: 'Two', | ||
| key: 'two', | ||
| children: [ | ||
| { label: 'Two-One', key: 'two-one', style: { color: 'green' } }, | ||
| { label: 'Two-Two', key: 'two-two', style: { color: 'blue' } }, | ||
| ], | ||
| }, | ||
| ]; | ||
|
|
||
| const { getAllByRole } = render(<Menu mode="inline" items={items} openKeys={['two']} />); | ||
|
|
||
| const menuItems = getAllByRole('menuitem'); | ||
| expect(menuItems).toBeTruthy(); | ||
|
|
||
| // { [label: color] } | ||
| const expected: any = { | ||
| One: 'rgb(255, 0, 0)', | ||
| 'Two-One': 'rgb(0, 128, 0)', | ||
| 'Two-Two': 'rgb(0, 0, 255)', | ||
| }; | ||
|
|
||
| menuItems.forEach((item) => { | ||
| const labelNode = item.querySelector('.ant-menu-title-content'); | ||
| const label = labelNode?.textContent?.trim(); | ||
|
|
||
| if (label && expected[label]) { | ||
| expect(item).toHaveStyle({ color: expected[label] }); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
测试用例有效验证了修复，但存在类型定义问题。
此测试正确验证了 MenuItem 的 style 属性现在能够正常工作，覆盖了顶层菜单项和嵌套子菜单项的场景。测试逻辑清晰且全面。
但是，Line 157 使用了 any 类型，这违反了项目编码规范："Never use any type - define precise types instead"。
应用此修改以改进类型安全性：
- // { [label: color] }
- const expected: any = {
+ // { [label: color] }
+ const expected: Record<string, string> = {
One: 'rgb(255, 0, 0)',
'Two-One': 'rgb(0, 128, 0)',
'Two-Two': 'rgb(0, 0, 255)',
};基于编码规范，应使用精确的类型定义而不是 any。
📝 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.
| // https://github.com/ant-design/ant-design/issues/56017 | |
| it('support MenuItem style', () => { | |
| const items = [ | |
| { label: 'One', key: 'one', style: { color: 'red' } }, | |
| { | |
| label: 'Two', | |
| key: 'two', | |
| children: [ | |
| { label: 'Two-One', key: 'two-one', style: { color: 'green' } }, | |
| { label: 'Two-Two', key: 'two-two', style: { color: 'blue' } }, | |
| ], | |
| }, | |
| ]; | |
| const { getAllByRole } = render(<Menu mode="inline" items={items} openKeys={['two']} />); | |
| const menuItems = getAllByRole('menuitem'); | |
| expect(menuItems).toBeTruthy(); | |
| // { [label: color] } | |
| const expected: any = { | |
| One: 'rgb(255, 0, 0)', | |
| 'Two-One': 'rgb(0, 128, 0)', | |
| 'Two-Two': 'rgb(0, 0, 255)', | |
| }; | |
| menuItems.forEach((item) => { | |
| const labelNode = item.querySelector('.ant-menu-title-content'); | |
| const label = labelNode?.textContent?.trim(); | |
| if (label && expected[label]) { | |
| expect(item).toHaveStyle({ color: expected[label] }); | |
| } | |
| }); | |
| }); | |
| // https://github.com/ant-design/ant-design/issues/56017 | |
| it('support MenuItem style', () => { | |
| const items = [ | |
| { label: 'One', key: 'one', style: { color: 'red' } }, | |
| { | |
| label: 'Two', | |
| key: 'two', | |
| children: [ | |
| { label: 'Two-One', key: 'two-one', style: { color: 'green' } }, | |
| { label: 'Two-Two', key: 'two-two', style: { color: 'blue' } }, | |
| ], | |
| }, | |
| ]; | |
| const { getAllByRole } = render(<Menu mode="inline" items={items} openKeys={['two']} />); | |
| const menuItems = getAllByRole('menuitem'); | |
| expect(menuItems).toBeTruthy(); | |
| // { [label: color] } | |
| const expected: Record<string, string> = { | |
| One: 'rgb(255, 0, 0)', | |
| 'Two-One': 'rgb(0, 128, 0)', | |
| 'Two-Two': 'rgb(0, 0, 255)', | |
| }; | |
| menuItems.forEach((item) => { | |
| const labelNode = item.querySelector('.ant-menu-title-content'); | |
| const label = labelNode?.textContent?.trim(); | |
| if (label && expected[label]) { | |
| expect(item).toHaveStyle({ color: expected[label] }); | |
| } | |
| }); | |
| }); |
🤖 Prompt for AI Agents
components/menu/__tests__/semantic.test.tsx around lines 137 to 171: the test
declares "expected" with the any type on line 157 which violates the rule
against any — change it to a precise type such as Record<string, string> (or a
more specific mapped type) to represent the mapping from label text to CSS color
string, update any related type annotations to match, and ensure the test still
accesses expected[label] in a type-safe way (e.g., check for undefined before
use or narrow the type).
Bundle ReportChanges will increase total bundle size by 32 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: antd.min-array-pushAssets Changed:
|
More templates
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #56041 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 803 803
Lines 14831 14831
Branches 3916 3916
=========================================
Hits 14831 14831 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
中文版模板 / Chinese template
🤔 This is a ...
🔗 Related Issues
fix: #56017
💡 Background and Solution
📝 Change Log