feat(nav-menu): [nav-menu] 新增 icon 属性及icon slot，支持配置菜单图标 #3181
Conversation
WalkthroughThis pull request introduces an Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant N as NavMenu Component
participant D as DataItem
U->>N: Request menu rendering
N->>D: Retrieve menu item data (with optional icon)
alt Icon slot provided
N->>N: Render custom icon from slot
else if item.icon exists
N->>N: Render default icon from data attribute
else
N->>N: Render item without icon
end
N->>U: Display menu with appropriate icon
Possibly related PRs
Suggested labels
Suggested reviewers
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
examples/sites/demos/apis/nav-menu.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" 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-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. examples/sites/demos/pc/app/nav-menu/menu-icon-composition-api.vueOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" 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-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. examples/sites/demos/pc/app/nav-menu/menu-icon.spec.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" 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-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
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:
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 (
|
github-actions
bot
added
bug
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
Walkthrough此PR为 Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| @@ -153,6 +163,7 @@ interface IMenuItem { | |||
| interface IDataItem { | |||
| title: string | |||
| url: string | |||
| icon?: Commonent | |||
There was a problem hiding this comment.
The type Commonent seems to be a typo. It should likely be Component. Please correct this to avoid potential runtime errors.
WalkthroughThis PR has added an icon attribute and an icon slot for the Changes
|
![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)
examples/sites/demos/pc/app/nav-menu/menu-icon.spec.ts (1)
1-11: Test case looks good, but could be more comprehensiveThe test verifies icon visibility, which is a good start. However, consider adding more test cases to verify different icon behaviors such as:
- Testing with different icon types
- Checking icon rendering in child menu items
- Verifying icon slot overrides the icon property
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
examples/sites/demos/apis/nav-menu.js(2 hunks)examples/sites/demos/pc/app/nav-menu/menu-icon-composition-api.vue(1 hunks)examples/sites/demos/pc/app/nav-menu/menu-icon.spec.ts(1 hunks)examples/sites/demos/pc/app/nav-menu/menu-icon.vue(1 hunks)examples/sites/demos/pc/app/nav-menu/webdoc/nav-menu.js(1 hunks)packages/renderless/src/nav-menu/index.ts(2 hunks)packages/renderless/types/nav-menu.type.ts(1 hunks)packages/theme/src/nav-menu/index.less(8 hunks)packages/theme/src/nav-menu/vars.less(1 hunks)packages/vue/src/nav-menu/__tests__/nav-menu.test.tsx(4 hunks)packages/vue/src/nav-menu/src/pc.vue(4 hunks)
🔇 Additional comments (36)
packages/renderless/types/nav-menu.type.ts (1)
16-16: Good addition of icon property to menuItemType interfaceThe addition of the
icon?: anyproperty to the menuItemType interface allows menu items to include icons, enhancing the visual presentation of the navigation menu.examples/sites/demos/pc/app/nav-menu/webdoc/nav-menu.js (1)
57-68: Demo entry properly added with translationsThe new demo entry for menu icons has been correctly added with proper translations for both name and description. This ensures users can understand how to use the new icon feature.
examples/sites/demos/apis/nav-menu.js (1)
131-140: Icon slot properly defined with descriptionsThe icon slot is properly defined with clear descriptions in both Chinese and English, and correctly linked to the corresponding demo.
examples/sites/demos/pc/app/nav-menu/menu-icon-composition-api.vue (4)
1-5: Good use of the Vue 3 Composition API for this demo component.The template structure is clean and straightforward, clearly demonstrating the new icon functionality.
7-11: Appropriate imports for the demo implementation.The component correctly imports the necessary dependencies including Vue's ref, the TinyNavMenu component, and the icons needed for the demo.
12-139: Well-structured menuData with proper icon implementation.The menuData structure demonstrates the new icon functionality effectively by:
- Using different icon components (iconTotal, iconGenerating)
- Showing icons at various nesting levels of the menu
- Omitting icons from some items to show both states
This provides a comprehensive example for users to understand how to implement menu icons.
142-150: Clean and appropriate styling.The styling maintains consistency with the component's design while ensuring proper display in the demo environment.
examples/sites/demos/pc/app/nav-menu/menu-icon.vue (4)
1-14: Excellent demonstration of both icon configuration approaches.The template effectively showcases two implementation methods:
- Configuration mode (配置模式): Using the icon property in data
- Slot mode (slot 模式): Using the #icon slot with conditional rendering
This dual approach provides users with flexibility in how they implement icons.
16-25: Well-structured component registration.The component properly imports and registers the required dependencies. Notably, the icon components are registered correctly to enable their use both in the template slot and in the data structure.
26-157: Comprehensive menuData structure with icons.The data structure demonstrates the new icon functionality at multiple levels of the menu hierarchy. This provides a thorough example of how to implement icons in different scenarios.
161-173: Appropriate styling for the demo.The styling ensures proper display of the menu components with adequate spacing and hover behavior.
packages/theme/src/nav-menu/index.less (12)
62-71: Well-defined base styling for menu icons.The
.menu-iconstyling properly handles the display and alignment of icons within menu items. The CSS rules follow best practices for SVG rendering:
- Using inline-block for display
- Setting appropriate vertical alignment
- Using CSS variables for consistent spacing and sizes
79-83: Proper hover state styling for icons.The hover state correctly changes the SVG fill color using the theme variable, ensuring consistent theme integration.
89-91: Consistent selected state styling for icons.The selected state styling matches the hover state for the SVG fill color, providing a cohesive user experience.
201-204: Important override for selected state in more menu.Using
!importanthere is actually appropriate since this rule needs to take precedence over potentially conflicting rules from other selectors.
232-241: Consistent replication of icon styling for popmenu.The styling for icons in the popmenu matches the main menu, ensuring a consistent look and feel across different menu states.
291-300: Proper icon styling for group titles.The styling for icons in group titles is consistent with other menu items, maintaining visual harmony.
307-309: Selected state for group titles properly handles icon styling.The icon color changes appropriately when a group title is selected.
337-348: Nice hover effect for submenu items.The underline effect on hover adds a nice visual cue for users, improving the interactive experience.
367-376: Consistent icon styling for submenu items.The styling for icons in submenu items matches the rest of the component, ensuring visual consistency.
393-395: Selected state for submenu items properly handles icon styling.The icon color changes appropriately when a submenu item is selected.
398-406: Consistent hover and active states for submenu items.The styling for hover, active, and selected states is consistent across all menu item types.
482-484: Proper vertical alignment for menu button icons.The SVG vertical alignment ensures the icons are properly positioned in the menu button.
packages/vue/src/nav-menu/src/pc.vue (4)
40-46: Well-implemented icon rendering with slot fallback pattern.The implementation correctly:
- Conditionally renders the icon container only when needed
- Uses a named slot with relevant props (item, index, selected)
- Provides a fallback for the icon component if the slot isn't used
This approach gives users flexibility while keeping the code clean.
94-99: Consistent icon implementation in more menu items.The icon implementation in the more menu follows the same pattern as the main menu, ensuring consistency across different menu sections.
129-139: Proper icon implementation in submenu group titles.The group title icons use the same slot/fallback pattern, providing consistent behavior across the component.
160-165: Consistent icon implementation in submenu items.The icon implementation in submenu items maintains the same pattern as other menu items, ensuring a unified approach throughout the component.
packages/theme/src/nav-menu/vars.less (1)
84-91: LGTM! Icon styling variables appropriately defined.The addition of these CSS custom properties for the menu icons follows the existing pattern and correctly references base design system variables. This ensures consistent styling and makes the component more customizable.
packages/renderless/src/nav-menu/index.ts (2)
172-173: LGTM! Icon property successfully added to the menu item data structure.The addition of the
iconproperty to the object returned by thebuildDatafunction properly extends the data structure to support menu icons.
581-581: Increased text measurement buffer improves tooltip accuracy.The change from
rect.width + 2torect.width + 4provides more buffer space when determining if text needs to be truncated with a tooltip, which can prevent edge cases where text is cut off without showing a tooltip.packages/vue/src/nav-menu/__tests__/nav-menu.test.tsx (6)
1-1: LGTM! Added IconShare import for new icon functionality.Properly importing the necessary icon component for testing the new icon feature.
9-10: LGTM! Component instances created for testing.Created reusable icon component instances that can be used across multiple tests.
206-217: LGTM! Added mock data with icon property.The new mock data structure properly includes the icon property, providing good test cases with both populated and empty icon values.
229-233: LGTM! Test for icon property rendering.This test correctly verifies that the icon passed via the data property is properly rendered in the component.
260-270: LGTM! Test for icon slot functionality.This test properly verifies that the icon slot mechanism works correctly.
272-284: LGTM! Test for slot priority over icon attribute.This test verifies an important behavior - that when both an icon attribute and slot are provided, the slot content takes precedence. This ensures proper component composition patterns.
| @@ -153,6 +163,7 @@ interface IMenuItem { | |||
| interface IDataItem { | |||
| title: string | |||
| url: string | |||
| icon?: Commonent | |||
There was a problem hiding this comment.
Fix typo in IDataItem interface
There's a typo in the type definition for the icon property - "Commonent" should be "Component".
- icon?: Commonent
+ icon?: Component 📝 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.
| icon?: Commonent | |
| icon?: Component |
|
在标题中可以用:[nav-menu] 触发下e2e测试用例哈，可以参考下其他的mr或者贡献指南 |
In the title, you can use: [nav-menu] to trigger the e2e test case. You can refer to other mr or contribution guides. |
hashiqi12138
changed the title
| 'en-US': 'Customize the menu icon' | ||
| }, | ||
| mode: ['pc'], | ||
| pcDemo: 'menu-icon' |
There was a problem hiding this comment.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
新增 icon 属性及icon slot，支持配置菜单图标
Issue Number: ##3104
What is the new behavior?
新增 icon 属性及icon slot，支持配置菜单图标
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Tests
Style