Feat(tree-menu)：[tree-menu] 树折叠时，支持悬浮展示子菜单列表，无子菜单时，悬浮展示tooltip #3183
Conversation
WalkthroughThis pull request enhances the tree menu component by adding two new properties— Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant T as TreeMenu (pc.vue)
participant P as PopMenu (pop-menu.vue)
participant N as MenuNode (menu-node.vue)
U->>T: Click on tree node
T->>T: Check expandMenuPopable & isExpand state
alt Node has children
T->>P: Render pop-up menu
P->>N: Render submenu items
U->>P: Click on submenu item
P->>T: Update current node state
else No children
T->>T: Display tooltip with node label
end
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/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. examples/sites/demos/pc/app/tree-menu/pop-sub-menu.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.
✨ 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为树形菜单组件添加了一个新功能：在树折叠时支持悬浮展示子菜单列表，并在无子菜单时悬浮展示tooltip。此功能通过新增属性 Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| setup() { | ||
| const treeMenuVm = hooks.inject('tree-menu') | ||
|
|
||
| function onClickItem(item) { |
There was a problem hiding this comment.
The function onClickItem directly modifies the state of treeMenuVm by calling setCurrentNode. Ensure that this operation is safe and does not lead to unintended side effects, especially in a reactive environment where state changes can trigger additional computations or renders.
|
[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 adds a new feature to the tree menu component: support for floating display submenu lists when tree collapses, and suspend display tooltip when there is no submenu. This function is implemented by adding the properties 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: 3
🧹 Nitpick comments (9)
examples/sites/demos/pc/app/tree-menu/pop-sub-menu.spec.ts (1)
1-26: Well-structured test for the new featureThe test verifies essential tree menu functionality including node visibility, interaction, and filtering. It validates the key user interactions that would be expected with the new submenu hover display feature.
However, this test doesn't appear to specifically test the new hover functionality mentioned in the PR title ("树折叠时，支持悬浮展示子菜单列表"). Consider adding test coverage for this specific feature to ensure it works as expected.
packages/vue/src/tree-menu/src/menu-node.vue (2)
39-44: Props lack type definitionsThe props are defined without TypeScript types, which reduces type safety and IDE support. Add proper type definitions to improve code quality.
Add proper type definitions to the props:
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: Unnecessary empty setup functionThe setup function returns an empty object, which is redundant. If no reactive state or computed properties are needed, you can simplify by removing the empty return.
Simplify the setup function:
setup() { - return {} }Or remove it entirely if no other functionality is planned:
-setup() { - return {} -}packages/theme/src/tree-menu/index.less (1)
350-413: Comprehensive styling for pop-up menu panelThe styling for the pop-up menu panel is well-structured and covers all necessary visual aspects including padding, border-radius, box-shadow, and item states (hover, active, current). These styles properly implement the UI requirements for the new hover-based submenu display functionality.
One improvement would be to add comments to explain the purpose of the CSS variables being used, especially for the transform properties that handle positioning.
packages/vue/src/tree-menu/src/pop-menu.vue (2)
1-2: Consider removing theeslint-disable vue/no-mutating-propsdirective
Disabling this lint rule can mask potential issues if props are ever mutated, which can break Vue’s unidirectional data flow.
88-104: Potential performance concern for large trees
The recursive approach is correct, but repeated checks on large, deeply nested structures might affect performance. Consider adding a short-circuit or memoization if the tree size grows.packages/vue/src/tree-menu/src/pc.vue (1)
81-90: Handle tooltip for empty child nodes
Displaying a tooltip for nodes without submenus ensures a clear user experience. Consider edge cases like extremely long or missing labels to prevent tooltip overflow or blank pop-ups.packages/theme/src/tree-menu/vars.less (2)
87-90: Clarify Comment for Shadow Property
The comment on line 89 currently repeats "弹出面板 圆角," which is appropriate for a border-radius property (line 88) but misleading here. Since the property is for box-shadow, consider updating the comment to something like "弹出面板 阴影" to accurately reflect its purpose.
103-104: Correct Comment for Padding-Left Property
The comment on line 103 incorrectly states "弹出面板 选中文本色" while the property defined on line 104 sets the left padding. Please update the comment to describe the left padding, e.g., "弹出面板 内边距" or "弹出面板 左侧内边距," to avoid confusion.
📜 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(4 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 (17)
packages/vue/src/tree-menu/src/props.ts (1)
70-74: New properties look good and follow established patterns.The addition of
expandMenuPopableandpopperClassproperties enhances the tree menu component functionality, allowing for pop-up submenu display when collapsed. The property types, default values, and placement in the file are consistent with existing code patterns.examples/sites/demos/pc/app/tree-menu/webdoc/tree-menu.js (1)
153-164: Demo entry follows project conventions.The new demo entry for pop-up submenus is properly structured with bilingual names and descriptions, consistent with other demo entries in the file. It's appropriately placed in the list and references the corresponding Vue file.
examples/sites/demos/pc/app/tree-menu/pop-sub-menu.vue (4)
1-20: Component template structure is well-organized.The template demonstrates both light and dark mode variants of the pop-up submenu feature. The
expand-menu-popableproperty is correctly applied to both instances, and the dark mode properly uses thepopperClassproperty for custom styling.
22-149: Data structure and component setup are appropriate.The script section properly imports required components and icons, and defines a comprehensive data structure for the demo. The tree data includes both nodes with and without children, which is excellent for demonstrating this feature.
151-161: Responsive layout using Flexbox.The scoped styles implement a clean, responsive layout using Flexbox, which is a good approach. The spacing between menu examples is well-defined.
162-184: Well-structured CSS custom properties for dark theme.The CSS for the dark theme is well-organized using CSS custom properties. All relevant component parts are styled consistently. The approach of using a single class name passed to both the component and the popper is an elegant solution.
examples/sites/demos/apis/tree-menu.js (1)
345-367: API documentation is clear and comprehensive.The new properties
expand-menu-popableandpopper-classare well-documented with clear descriptions in both Chinese and English. The properties are correctly configured with appropriate types, default values, and mode settings, and are linked to the demo example.examples/sites/demos/pc/app/tree-menu/pop-sub-menu-composition-api.vue (2)
1-3: Component correctly implements the new featureThe template properly implements the new
expand-menu-popableproperty on thetiny-tree-menucomponent, which aligns with the PR objective of supporting hover-based submenu display when the tree is collapsed.
51-54:❓ Verification inconclusive
Empty URL attributes
There are several menu items with empty URL attributes (
url: ''). Consider either removing these empty URL properties or filling them with appropriate values.Check if these empty URLs are intentional or if they should point to actual resources:
🏁 Script executed:
#!/bin/bash # Check if there are other examples of empty URLs in tree-menu components grep -r "url: ''" --include="*.vue" examples/sites/demos/pc/app/tree-menu/Length of output: 33548
Attention: Clarification Needed on Empty URL Attributes in Demo Menu Components
Our verification shows that the use of empty URL attributes (
url: '') is consistent across all tree-menu demo Vue files (including the snippet inpop-sub-menu-composition-api.vue, lines 51–54). If these empty URLs are intended as placeholders (for example, to indicate non-clickable or demo-only items), please add a comment or documentation to clarify this decision. Otherwise, if these properties are expected to link to actual resources in the future, please update them accordingly for consistency across the demos.
- Affected file snippet (pop-sub-menu-composition-api.vue, lines 51–54):
{ id: 30102, label: 'Datepicker 日期', url: '' }, { id: 30103, label: 'Dropdown 下拉框', url: '' }, { id: 30104, label: 'DropTimes 下拉时间', url: '' }, { id: 30105, label: 'Input 输入框', url: '' }packages/theme/src/tree-menu/index.less (2)
20-21: Well-defined class prefixes for new functionalityThe new CSS class prefixes for pop-up menu elements follow the established naming conventions of the codebase, making the code more maintainable.
345-348: Simple and clean styling for pop-up menuThe
.@{tree-pop-menu-prefix-cls}class withline-height: initialproperly resets the line height for pop-up menus, ensuring proper rendering of menu items.packages/vue/src/tree-menu/src/pop-menu.vue (1)
83-85: Clarify the logic for current node
hasCurrentreturnsfalsewhen the node itself isisCurrent, while it recurses for child nodes. Verify if excluding the node’s own state is intended or if this needs adjustment to include the node itself.packages/renderless/src/tree-menu/vue.ts (1)
80-80: Ensure safe usage of the shared VM injection
Providing'tree-menu'in the renderless function allows child components to access the main tree menu context. Verify that child components do not unintentionally override this injection, and confirm they rely on this pattern to avoid undefined references.Also applies to: 131-131
packages/vue/src/tree-menu/src/pc.vue (3)
91-96: Pop-up menu integration
The conditional rendering ofTinyTreeMenuPopMenusupports hover-based submenus. Confirm that nested child data is valid and won’t break the pop-up for deeper levels.
98-100: Graceful fallback rendering
Falling back toTinyTreeMenuNodewhenexpandMenuPopableorstate.isExpandis disabled maintains consistent behavior across the tree menu.
117-119: Validate component imports and dependencies
Newly importedTinyTooltip,TinyTreeMenuPopMenu, andTinyTreeMenuNodecomponents should be verified in package dependencies. Ensure they’re properly listed in the correct configuration files and that no version conflicts arise.packages/theme/src/tree-menu/vars.less (1)
85-108: Overall Approval for New Pop-Up Panel Custom Properties
The new CSS custom properties for the pop-up panel (padding, radius, shadow, item dimensions, and colors) are well structured and follow a consistent naming convention. They provide enhanced styling flexibility for the tree menu component in collapsed mode.
| customIcon: iconApp() | ||
| }, |
There was a problem hiding this comment.
Missing icon imports will cause runtime errors
The component references custom icon functions like iconApp(), iconExpressSearch(), etc., but these functions are not imported in the script section.
Add the necessary imports at the top of your script section:
import { TinyTreeMenu } from '@opentiny/vue'
import { ref } from 'vue'
+import {
+ iconApp,
+ iconExpressSearch,
+ iconTotal,
+ iconUnfreeze,
+ iconFile
+} from '@opentiny/vue-icon'📝 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.
| customIcon: iconApp() | |
| }, | |
| <script> | |
| import { TinyTreeMenu } from '@opentiny/vue' | |
| import { ref } from 'vue' | |
| import { | |
| iconApp, | |
| iconExpressSearch, | |
| iconTotal, | |
| iconUnfreeze, | |
| iconFile | |
| } from '@opentiny/vue-icon' | |
| // ... rest of your script code | |
| export default { | |
| // component definition, etc. | |
| data() { | |
| return { | |
| customIcon: iconApp() | |
| } | |
| } | |
| } | |
| </script> |
| import { defineComponent } from '@opentiny/vue-common' | ||
| import Popover from '@opentiny/vue-popover' | ||
|
|
||
| export default defineComponent({ | ||
| name: 'TreeMenuNode', | ||
| componentName: 'TreeMenuNode', | ||
| components: { | ||
| TinyPopover: Popover | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Imported but unused component
The component imports Popover and registers it as TinyPopover, but it's not used anywhere in the template.
Either remove the unused import and component registration, or implement the Popover functionality if it's meant to be part of this component:
import { defineComponent } from '@opentiny/vue-common'
-import Popover from '@opentiny/vue-popover'
export default defineComponent({
name: 'TreeMenuNode',
componentName: 'TreeMenuNode',
- components: {
- TinyPopover: Popover
- },📝 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.
| import { defineComponent } from '@opentiny/vue-common' | |
| import Popover from '@opentiny/vue-popover' | |
| export default defineComponent({ | |
| name: 'TreeMenuNode', | |
| componentName: 'TreeMenuNode', | |
| components: { | |
| TinyPopover: Popover | |
| }, | |
| import { defineComponent } from '@opentiny/vue-common' | |
| export default defineComponent({ | |
| name: 'TreeMenuNode', | |
| componentName: 'TreeMenuNode', | |
| // other component options can be added here if needed | |
| }) |
|
在标题中可以用:[tree-menu] 触发下e2e测试用例哈，可以参考下其他的mr或者贡献指南 |
You can use: [tree-menu] to trigger the e2e test case. You can refer to other mr or contribution guides. |
hashiqi12138
changed the title
|
e2e测试报错解决一下 |
e2e tests are kept resolved |
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?
Issue Number: #3105
What is the new behavior?
树折叠时，支持悬浮展示子菜单列表，无子菜单时，悬浮展示tooltip
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit