feat(nav-menu): 新增 icon 属性及icon slot，支持配置菜单图标 #3120
Conversation
WalkthroughThe changes extend the navigation menu functionality by adding an Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant NM as NavMenu Component
participant RL as Renderless Logic
participant IC as Icon Component
U->>NM: Interact with menu item
NM->>RL: Build menu data (including optional icon)
RL-->>NM: Return menu item data with `icon`
alt Icon exists
NM->>IC: Render custom icon via slot/default component
else No icon
NM-->>NM: Render menu without icon
end
NM-->>U: Display updated menu with icon support
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
ERR_PNPM_OPTIONAL_DEPS_REQUIRE_PROD_DEPS Optional dependencies cannot be installed without production dependencies ✨ 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 (
|
Walkthrough此PR为 Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| @@ -577,7 +578,7 @@ export const handleTitleMouseenter = | |||
| const text = target.textContent | |||
| const font = window.getComputedStyle(target).font | |||
| const rect = target.getBoundingClientRect() | |||
There was a problem hiding this comment.
The adjustment from rect.width + 2 to rect.width + 4 in the omitText function might affect text truncation behavior. Ensure that this change does not unintentionally alter the display of text in the navigation menu.
|
[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 |
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)
packages/renderless/types/nav-menu.type.ts (1)
16-16: Consider using a more specific type instead ofanyUsing
anytype for theiconproperty reduces type safety. Consider using a more specific type likeComponent | string | nullto better represent what the icon property can contain, which would provide better type checking and IDE autocompletion for developers.- icon?: any + icon?: Component | string | null
📜 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)
examples/sites/demos/pc/app/nav-menu/menu-icon.spec.ts (1)
3-11: Test looks good for the basic functionalityThe test correctly verifies the visibility of both the menu icon and its SVG element. This covers the basic functionality of the new feature.
examples/sites/demos/pc/app/nav-menu/webdoc/nav-menu.js (1)
55-66: New demo entry looks goodThe demo entry for the menu icon feature is well-structured and consistent with other demo entries in the file. The descriptions are clear and provided in both Chinese and English.
examples/sites/demos/apis/nav-menu.js (1)
131-140: New slot definition looks goodThe
iconslot is properly defined with clear descriptions in both languages, consistent with other slot definitions.examples/sites/demos/pc/app/nav-menu/menu-icon-composition-api.vue (4)
1-5: Clean template structure with proper component usage.The template is well-structured, using a simple container div with the
previewclass and properly binding themenuDatato the TinyNavMenu component.
7-11: Appropriate imports and setup configuration.The component correctly imports necessary dependencies:
- Vue's
reffor reactive state managementTinyNavMenucomponent from the library- Icon components that will be used in the menu data
12-139: Well-structured menu data with proper icon implementation.The menu data is properly structured as a reactive ref array with nested items. Icons are correctly implemented at various levels of the menu hierarchy:
- Root level (line 17)
- First-level children (line 28)
- Deep nested items (lines 51 and 57)
This provides a comprehensive demonstration of icon usage at different menu levels.
142-150: Appropriate styling with good practices.The CSS is properly scoped to the component and includes:
- A minimum height for the preview container to ensure proper display
- Removal of text decoration on hover for links inside the menu, improving visual consistency
These styles work well with the component's functionality.
examples/sites/demos/pc/app/nav-menu/menu-icon.vue (4)
1-14: Comprehensive demonstration of both icon implementation methods.The template effectively showcases two approaches to implement icons:
- Configuration mode (line 4): Using the icon property directly in the menu data
- Slot mode (lines 7-12): Using a named slot with conditional rendering based on item state
This provides users with flexibility in how they implement icons in their menus.
16-25: Proper component registration and icon implementation.The script correctly:
- Imports the necessary components
- Registers both the main component and the icon components
- Uses the icon factory functions properly
This approach makes the icons available for both the configuration and slot modes.
26-157: Well-organized data structure with icon configuration.The data method returns a well-structured menu configuration with icons assigned to various items. The implementation matches what we see in the Composition API example, providing consistency across examples.
161-173: Clean and purposeful CSS styling.The CSS is properly scoped and includes:
- Consistent minimum height for the preview
- Appropriate margin between multiple menu instances (line 167)
- Removal of text decoration on hover for menu links
These styles enhance the visual presentation of the demo.
packages/vue/src/nav-menu/src/pc.vue (4)
40-45: Well-implemented icon rendering for main menu items.The implementation correctly:
- Conditionally renders a div with class "menu-icon" when either slots.icon or item.icon exists
- Uses a named slot with appropriate props passed (item, index, selected state)
- Provides a default fallback that renders the item.icon as a component
- Applies appropriate CSS classes for styling
This approach provides flexibility while maintaining backward compatibility.
94-99: Consistent icon implementation for more menu items.The same pattern is correctly replicated for the "more" menu, ensuring consistent behavior and appearance. The component passes the appropriate context to the slot (item, index, selected state).
129-139: Proper icon handling for group titles in the sub-menu.The icon implementation for group titles in the sub-menu follows the same pattern as other menu items, providing consistency throughout the component. All necessary context is passed to the slot.
160-165: Consistent icon implementation for sub-menu items.The same pattern is applied to sub-menu items, maintaining consistency across all levels of the menu. This ensures that icons can be used at any level of the menu hierarchy.
packages/renderless/src/nav-menu/index.ts (2)
172-174: Proper data structure update to support the icon property.The
buildDatafunction now correctly includes theiconproperty from the menu item, enabling the icon functionality in the renderless implementation. This change ensures that icons specified in the data are properly passed to the component.
581-582: Tooltip width adjustment for better text display.The width adjustment in the
omitTextfunction call has been modified from +2 to +4 pixels, providing more space for the tooltip text. This subtle change improves the user experience by reducing the chance of text being unnecessarily truncated.packages/theme/src/nav-menu/index.less (12)
60-69: Looks good! Well structured icon styling.The
.menu-iconclass implementation provides consistent styling for icons within menu items. The use of CSS variables follows the project's convention and maintains a clean, maintainable approach.
78-80: Proper handling of SVG hover states.The hover state styling for SVG elements follows best practices by changing the fill color using the designated CSS variable.
87-89: Consistent selected state styling.The selected state styling for SVG icons matches the hover state, maintaining visual consistency when menu items are selected.
199-202: Properly implemented icon styling for selected state in more menu.The use of
!importanthere is appropriate since it ensures the selected state takes precedence in the component hierarchy.
230-240: Well-implemented icon styling for more menu items.The styling for icons in the more menu follows the same pattern as the main menu, maintaining consistency throughout the component.
289-299: Consistent implementation for sub-menu title icons.The styling for icons in sub-menu titles matches the pattern used elsewhere, ensuring visual consistency.
305-307: Proper styling for selected state in sub-menu titles.The SVG fill color change for selected sub-menu titles maintains visual consistency with other selected states.
336-346: Good addition of underline effect on hover.Adding the underline effect enhances the visual feedback for users when hovering over menu items.
365-374: Consistent icon styling for sub-items.The icon styling for sub-items follows the same pattern as other menu items, maintaining visual consistency.
391-393: Proper selected state styling for sub-items.The SVG fill color change for selected sub-items maintains visual consistency with other selected states.
401-403: Appropriate hover state styling for sub-items.The hover state styling for sub-items' SVG elements follows the same pattern as other hover states.
480-482: Good vertical alignment for more-button SVGs.Setting the vertical alignment ensures proper visual alignment of SVG icons in the more button.
packages/theme/src/nav-menu/vars.less (1)
78-85: Well-defined CSS variables for icon styling.The addition of these icon-related CSS variables follows best practices:
- Proper naming convention consistent with existing variables
- Clear comments explaining the purpose of each variable
- Reuse of base variables (
--tv-space-base,--tv-icon-size, etc.) for consistency- Separation of concerns with dedicated variables for different states (normal and hover)
This approach ensures the icon styling can be easily customized through the theming system.
packages/vue/src/nav-menu/__tests__/nav-menu.test.tsx (6)
1-1: Properly imported the new icon component.The import statement has been updated to include the new
IconSharecomponent alongside the existingIconTotal.
9-11: Good practice creating component instances.Creating component instances before using them in tests improves readability and makes the tests more maintainable.
206-217: Well-structured mock data for icon testing.The mock data includes variations with and without icons, which will help test different scenarios:
- An item with an icon component
- An item with an empty icon property
This provides good coverage for the new feature.
229-234: Good basic test for icon rendering.This test verifies that the icon component is properly rendered when specified in the data. The test is concise and focuses on a single aspect of functionality.
260-271: Proper testing of icon slot functionality.This test verifies that the icon slot works correctly, ensuring that custom icons can be provided through the slot mechanism.
272-284: Important test for slot priority.This test verifies that slot content takes precedence over the icon attribute, which is an important behavior to confirm. The assertions are clear and thorough, checking both the presence of the slotted icon and the absence of the attribute icon.
| @@ -153,6 +163,7 @@ interface IMenuItem { | |||
| interface IDataItem { | |||
| title: string | |||
| url: string | |||
| icon?: Commonent | |||
There was a problem hiding this comment.
Fix typo in type declaration
There is a typo in the type declaration 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 |
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?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Style
Tests