-
Notifications
You must be signed in to change notification settings - Fork 285
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
Feat(tree-menu):树折叠时,支持悬浮展示子菜单列表,无子菜单时,悬浮展示tooltip #3131
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces multiple enhancements to the tree menu component across various modules. It adds two new properties for controlling menu expansion and pop-up styling, updates the renderless function to use dependency injection, and introduces several new Vue components—including ones for pop-up submenus and node display. New Playwright tests verify node behavior and filtering functionality. In addition, the style sheets are updated with new LESS variables and CSS custom properties for improved pop-up menu appearance, and the tree menu display now integrates tooltips and pop-up menus for enhanced interactivity. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant TT as TinyTreeMenu
participant MN as TreeMenuNode
participant PM as TreeMenuPopMenu
participant TTp as TinyTooltip
U->>TT: Interacts with tree menu
TT->>MN: Renders node with data
MN->>PM: Check for children nodes
alt Node has children
PM-->>MN: Render pop-up menu with items
U->>PM: Selects a menu item
PM-->>TT: Update selected state
else Node has no children
MN->>TTp: Show tooltip with extra info
end
TT-->>U: Reflect state change in UI
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/tree-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/tree-menu/pop-sub-menu-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/tree-menu/pop-sub-menu.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.
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ 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 (
|
[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 |
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 (10)
examples/sites/demos/pc/app/tree-menu/pop-sub-menu.spec.ts (1)
1-26
: Consider expanding test coverage for the popup submenu featureThe test effectively covers basic tree menu functionality like node expansion, visibility, and filtering. However, it doesn't explicitly test the popup submenu functionality that is the focus of this PR.
Consider adding specific test cases that:
- Verify the popup behavior when hovering over collapsed nodes
- Test the tooltip display for nodes without submenus
- Check the interaction with the newly added
expandMenuPopable
propertytest('静态数据', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tree-menu#basic-usage') const wrap = page.locator('#basic-usage') const treeMenu = wrap.locator('.tiny-tree-menu') const treeNode = treeMenu.locator('.tiny-tree-node__wrapper > .tiny-tree-node') const treeNodeContent = treeNode.locator('> .tiny-tree-node__content') await expect(treeNode.filter({ hasText: /^环境准备$/ })).toBeHidden() await treeNodeContent.filter({ hasText: /^使用指南$/ }).click() await expect(treeNode.filter({ hasText: /^环境准备$/ })).toBeVisible() await treeNode.filter({ hasText: /^环境准备$/ }).click() await expect(treeNode.filter({ hasText: /^环境准备$/ })).toHaveClass(/is-current/) await treeNodeContent.filter({ hasText: /^使用指南$/ }).click() await expect(treeNode.filter({ hasText: /^环境准备$/ })).toBeHidden() // 过滤功能 await treeMenu.locator('.tiny-input__inner').fill('新增组件') await expect(page.getByTitle('新增组件')).toBeVisible() await expect(treeNodeContent.filter({ hasText: /^使用指南$/ })).toBeHidden() await treeMenu.locator('.tiny-input__inner').clear() await expect(treeNodeContent.filter({ hasText: /^使用指南$/ })).toBeVisible() + + // 测试折叠子菜单弹出功能 + // Set tree to collapsed state + const collapseButton = treeMenu.locator('.tiny-tree-menu__collapse-btn') + await collapseButton.click() + + // Test hover behavior on node with children + await treeNodeContent.filter({ hasText: /^使用指南$/ }).hover() + const popupMenu = page.locator('.tiny-tree-menu-pop-menu') + await expect(popupMenu).toBeVisible() + + // Test tooltip on node without children + const leafNode = treeNodeContent.filter({ hasText: /^环境准备$/ }) + await leafNode.hover() + const tooltip = page.locator('.tiny-popover') + await expect(tooltip).toBeVisible() })packages/vue/src/tree-menu/src/menu-node.vue (4)
1-1
: Remove ESLint disable comment if not neededThe ESLint directive suggests there's prop mutation in the component, but the provided code doesn't show any direct prop mutations. If this was included preventively, consider adding a comment explaining why or remove it if unnecessary.
17-17
: Consider usingundefined
instead ofvoid 0
For better readability, consider using
undefined
directly instead ofvoid 0
. Both achieve the same result, butundefined
is more commonly recognized.- <a class="tree-node-body" :title="getTitle(data.label)" :href="data.url || void 0"> + <a class="tree-node-body" :title="getTitle(data.label)" :href="data.url || undefined">
39-44
: Add type definitions to props for better type safetyThe prop definitions lack type information, which could lead to type-related issues. Consider adding proper TypeScript types to improve type safety and developer experience.
props: { - node: {}, - data: {}, - suffixIcon: {}, - getTitle: {} + node: { + type: Object, + required: true + }, + data: { + type: Object, + required: true + }, + suffixIcon: { + type: [Object, Function], + default: null + }, + getTitle: { + type: Function, + required: true + } },
46-48
: Implement the setup function or remove it if not neededThe
setup()
function returns an empty object, suggesting it might not be fully implemented. Either implement the function with necessary reactive state and logic, or remove it if not needed.If no reactivity is needed:
- setup() { - return {} - }packages/vue/src/tree-menu/src/pop-menu.vue (3)
1-1
: Consider removing ESLint disable comment if possibleThe ESLint disable comment suggests there might be mutating props issues. Consider refactoring to avoid prop mutation rather than disabling the lint rule.
83-106
: Translate comment to English for consistencyThe comment on line 85 is in Chinese. Consider translating it to English to maintain code consistency.
- if (node.isCurrent === true) { - return false // 输入对象自身 isCurrent 为 true,直接返回 false + if (node.isCurrent === true) { + return false // If the input node itself is current, return false directly
76-81
: Consider validating node existence before updatingThe
onClickItem
function directly sets the current node without checking if the node object is valid.function onClickItem(item) { - treeMenuVm && treeMenuVm.setCurrentNode(item) + if (treeMenuVm && item) { + treeMenuVm.setCurrentNode(item) + } }examples/sites/demos/pc/app/tree-menu/pop-sub-menu.vue (1)
156-160
: Consider responsive design improvementsThe fixed margin of 300px might cause layout issues on smaller screens. Consider using a percentage-based or responsive approach.
.menu-fix { - margin-right: 300px; + margin-right: 10%; + min-width: 200px; }packages/theme/src/tree-menu/vars.less (1)
81-104
: Well-organized CSS variables for pop-up panel stylingThe new CSS variables for the pop-up panel are consistently named and well-structured. They provide comprehensive styling options for the new tree menu pop-up functionality.
One suggestion: consider adding brief comments in English alongside the Chinese comments for better international developer experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
examples/sites/demos/apis/tree-menu.js
(1 hunks)examples/sites/demos/pc/app/tree-menu/pop-sub-menu-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/tree-menu/pop-sub-menu.spec.ts
(1 hunks)examples/sites/demos/pc/app/tree-menu/pop-sub-menu.vue
(1 hunks)examples/sites/demos/pc/app/tree-menu/webdoc/tree-menu.js
(1 hunks)packages/renderless/src/tree-menu/vue.ts
(2 hunks)packages/theme/src/tree-menu/index.less
(3 hunks)packages/theme/src/tree-menu/vars.less
(1 hunks)packages/vue/src/tree-menu/src/menu-node.vue
(1 hunks)packages/vue/src/tree-menu/src/pc.vue
(3 hunks)packages/vue/src/tree-menu/src/pop-menu.vue
(1 hunks)packages/vue/src/tree-menu/src/props.ts
(1 hunks)
🔇 Additional comments (20)
packages/vue/src/tree-menu/src/props.ts (2)
70-73
: LGTM: New property added to support popup submenusThe
expandMenuPopable
property is well-defined with appropriate type and default value offalse
, which aligns with the PR objective to support displaying submenu lists on hover when the tree is collapsed.
74-74
: LGTM: New property for styling popup menusThe
popperClass
property allows for custom styling of the popup menu. This complements theexpandMenuPopable
property and follows the component's pattern for style customization properties.examples/sites/demos/pc/app/tree-menu/webdoc/tree-menu.js (1)
151-162
: LGTM: Well-structured demo entry for the new featureThe new demo entry is well-organized with proper localization for both Chinese and English users. The descriptions clearly explain the feature's purpose: displaying submenu lists when the tree is collapsed.
examples/sites/demos/apis/tree-menu.js (2)
345-356
: Well-structured property definition for expand-menu-popableThe new
expand-menu-popable
property is correctly defined with proper boolean type, default value, and bilingual descriptions that clearly explain its purpose in the context of tree menu functionality.The English description could be slightly improved for grammar and clarity, but the implementation is solid and follows the component's established patterns.
357-367
: Proper implementation of popper-class propertyThe
popper-class
property is well-defined with appropriate type (string) and default value. The bilingual descriptions clearly explain that this class is for styling the pop-up window when hovering to display submenus.Both new properties correctly reference the same demo example 'pop-sub-menu' which helps users understand how they work together.
examples/sites/demos/pc/app/tree-menu/pop-sub-menu-composition-api.vue (3)
1-3
: Clear demonstration of the new functionalityThe template correctly uses the TinyTreeMenu component with the new
expand-menu-popable
property enabled, along with the necessaryshow-expand
property to demonstrate the submenu popup functionality.
5-8
: Proper imports using Composition APIThe script correctly imports the TinyTreeMenu component from @opentiny/vue and uses Vue's ref for reactivity, following Vue's Composition API best practices with script setup.
9-122
: Comprehensive demonstration data structureThe treeData implementation provides a good demonstration of different scenarios:
- Nodes with and without child items
- Nodes with customIcons
- Nested hierarchies of different depths
- Items with and without URLs
This gives users a thorough example of how the component handles various data structures and node types with the new popup functionality.
packages/renderless/src/tree-menu/vue.ts (2)
80-80
: Good implementation of dependency injection parameterThe renderless function parameters now include
provide
from ISharedRenderlessFunctionParams, which is essential for enabling the dependency injection pattern in Vue.
131-132
: Effective use of dependency injection for component communicationAdding
provide('tree-menu', vm)
makes the tree-menu's view model available to child components through Vue's dependency injection, which is an excellent approach for enabling the popup submenu functionality to interact with the main tree menu without direct prop passing.This approach follows Vue's best practices for component communication in more complex component hierarchies.
packages/theme/src/tree-menu/index.less (4)
20-21
: Good naming conventions for CSS class prefixesThe new LESS variables for popup menu class prefixes follow the established naming conventions in the codebase, making the code more maintainable and consistent.
217-217
: Improved layout handling with flexboxAdding
display: flex
to the tree-node-name and settingflex-shrink: 0
for SVG icons improves layout control and prevents icons from being compressed in constrained spaces, which enhances the user experience with consistent element sizing.Also applies to: 225-225
343-346
: Initial styling for pop-up menuThe initial styling for the tree pop menu provides the line-height reset necessary for proper rendering. This ensures the popup menu maintains consistent spacing regardless of parent container styling.
348-411
: Comprehensive styling for pop-up menu panelThe styling for the pop-up menu panel is thorough and well-structured, including:
- Proper use of CSS variables for theming consistency
- Careful handling of nested elements and their states
- Appropriate styling for hover, active, and selected states
- Good use of flexbox for alignment and layout
- Proper handling of transformation for positioning
All the necessary visual states are covered, making the popup experience smooth and visually consistent with the rest of the component.
packages/vue/src/tree-menu/src/pc.vue (3)
81-100
: Great enhancement to tree menu interactivity!The conditional rendering logic elegantly handles different node states:
- Shows tooltips for nodes without children
- Displays pop-up menus for nodes with children
- Falls back to simple node display when not in expanded state
This implementation aligns perfectly with the PR objective of supporting hover displays for collapsed trees.
117-120
: LGTM: Clean component importsThe imports for the new tooltip, pop-menu, and menu-node components are correctly added.
144-146
: LGTM: Component registrationThe new components are properly registered in the components section.
packages/vue/src/tree-menu/src/pop-menu.vue (1)
15-48
: Well-structured pop-up menu implementationThe TinyPopover implementation with conditional rendering based on child nodes presence is well done. The class composition with conditional first-level styling is particularly good.
examples/sites/demos/pc/app/tree-menu/pop-sub-menu.vue (2)
1-20
: Well-designed demo showcasing both light and dark themesThe template structure provides a clear comparison between the light and dark themes of the tree menu. Good use of the new
expand-menu-popable
property to demonstrate the pop-up submenu functionality.
162-184
: Comprehensive CSS custom properties for dark themeThe dark theme styling is thorough, covering all necessary components. Good use of CSS custom properties to override the default styles.
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?
树折叠时,支持悬浮展示子菜单列表,无子菜单时,悬浮展示tooltip
Issue Number: #3105
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Tests
Style