-
Notifications
You must be signed in to change notification settings - Fork 277
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): [tree-menu] Add static data and unused topic variables for rectification #2803
Conversation
WalkthroughThe pull request introduces modifications to the tree menu component across multiple files, focusing on updating data handling, descriptions, and styling. Changes include updating the Chinese description for the menu API's Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
WalkthroughThis pull request introduces static data and unused topic variables to the tree-menu component. It includes updates to the documentation and fixes a bug related to data handling. The changes involve adding static data to the tree-menu component and modifying related test cases and styles. Changes
|
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: 1
🧹 Nitpick comments (3)
examples/sites/demos/pc/app/tree-menu/basic-usage-composition-api.vue (1)
9-173
: Standardize the tree data structure.The tree data structure has inconsistencies that could affect maintainability:
- Inconsistent usage of the
url
property (present in some nodes, missing in others)- Varying ID formats (e.g., 100 vs 60101)
Consider standardizing:
- Add
url
property consistently where applicable- Use a consistent ID format across all nodes
Example standardization:
{ id: 100, - label: '组件总览' + label: '组件总览', + url: 'overview' }examples/sites/demos/pc/app/tree-menu/basic-usage.vue (1)
12-179
: Well-structured hierarchical data with good organization.The treeData implementation:
- Uses unique IDs for each node
- Follows a logical grouping structure
- Maintains consistent nesting levels
- Includes optional URL property where needed
Consider adding JSDoc type definitions for better code documentation and type safety.
data() { return { + /** @type {{ + * id: number, + * label: string, + * url?: string, + * children?: Array<{ + * id: number, + * label: string, + * url?: string, + * children?: Array<any> + * }> + * }[]} */ treeData: [packages/theme/src/tree-menu/index.less (1)
157-161
: Consistent use of variables for layout and styling.The implementation:
- Uses variables for padding and scrollbar
- Properly positions node numbers
- Applies hover font weight
- Sets node content height
Consider adding RTL (right-to-left) support for better internationalization.
Also applies to: 195-196, 211-211, 254-254
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
examples/sites/demos/apis/menu.js
(1 hunks)examples/sites/demos/pc/app/tree-menu/basic-usage-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/tree-menu/basic-usage.spec.ts
(1 hunks)examples/sites/demos/pc/app/tree-menu/basic-usage.vue
(2 hunks)examples/sites/demos/pc/app/tree-menu/data-resource-composition-api.vue
(0 hunks)examples/sites/demos/pc/app/tree-menu/data-resource.spec.ts
(1 hunks)examples/sites/demos/pc/app/tree-menu/data-resource.vue
(0 hunks)examples/sites/demos/pc/app/tree-menu/webdoc/tree-menu.js
(1 hunks)packages/theme/src/tree-menu/index.less
(5 hunks)packages/theme/src/tree-menu/vars.less
(2 hunks)
💤 Files with no reviewable changes (2)
- examples/sites/demos/pc/app/tree-menu/data-resource.vue
- examples/sites/demos/pc/app/tree-menu/data-resource-composition-api.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (8)
examples/sites/demos/pc/app/tree-menu/data-resource.spec.ts (1)
8-8
: Consider using a more specific selector for the tree menu.Removing
.nth(1)
makes the locator target all.tiny-tree-menu
instances on the page. This could lead to flaky tests if multiple tree menus exist. Consider using a more specific selector or data-testid attribute.examples/sites/demos/pc/app/tree-menu/basic-usage.spec.ts (1)
3-25
: Well-structured test coverage for tree menu functionality!The test cases comprehensively cover:
- Tree node expansion/collapse
- Node selection state
- Filter functionality with both positive and negative cases
examples/sites/demos/pc/app/tree-menu/basic-usage-composition-api.vue (1)
2-2
: LGTM! Proper usage of composition API.The template correctly binds the reactive
treeData
to the component using the composition API.examples/sites/demos/pc/app/tree-menu/basic-usage.vue (1)
1-3
: LGTM! Component usage is clear and concise.The template correctly implements the tiny-tree-menu component with appropriate props.
examples/sites/demos/pc/app/tree-menu/webdoc/tree-menu.js (1)
12-13
: Documentation updates improve clarity and consistency.The changes:
- Clarify static data usage in basic demo
- Rename "Data Source" to "Server data" for better clarity
- Update descriptions to match implementation
The translations are accurate and maintain consistency between Chinese and English.
Also applies to: 20-21, 24-26
packages/theme/src/tree-menu/vars.less (2)
42-61
: Well-structured CSS variables for input and node positioning.Good use of design tokens and clear organization of related variables. The variables provide fine-grained control over input element positioning and node content layout.
76-83
: Good addition of theme-related variables.The new variables enhance theming capabilities:
- Background color uses design token
- Scrollbar dimensions are configurable
- Font weight for hover state is customizable
packages/theme/src/tree-menu/index.less (1)
28-28
: Good implementation of theming variables.The changes properly implement the new CSS variables for:
- Background color
- Input padding and margins
- Input line positioning
Also applies to: 140-152
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: #2706
colses #2706
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Release Notes
Documentation
Style
Bug Fixes
Refactor