-
Notifications
You must be signed in to change notification settings - Fork 8.4k
feat: 修正菜单排序在二级菜单不生效问题 #7007
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
base: main
Are you sure you want to change the base?
feat: 修正菜单排序在二级菜单不生效问题 #7007
Conversation
|
WalkthroughA new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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 (1)
packages/@core/base/shared/src/utils/tree.ts (1)
113-122: Consider consistent immutability for all nodes.The current implementation creates new objects for nodes with children but returns the original item for leaf nodes. For more predictable behavior and consistency, consider always creating new objects:
return treeData.toSorted(sortFunction).map((item) => { const children = item[childProps]; if (children && Array.isArray(children) && children.length > 0) { return { ...item, [childProps]: sortTree(children, sortFunction, options), }; } - return item; + return { ...item }; });This ensures all returned nodes are new objects, maintaining consistent immutability throughout the tree structure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/@core/base/shared/src/utils/tree.ts(1 hunks)packages/utils/src/helpers/generate-menus.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/utils/src/helpers/generate-menus.ts (1)
packages/@core/base/shared/src/utils/tree.ts (1)
sortTree(125-125)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (4)
packages/utils/src/helpers/generate-menus.ts (2)
9-9: LGTM: Proper import for the new utility.The
sortTreeimport is correctly added to support recursive menu sorting.
84-84: Good fix: Recursive sorting now works for all menu levels.The replacement of
toSorted()withsortTree()correctly addresses the issue where sorting was only applied to top-level menus. The new implementation will recursively sort all nested menu levels by theorderproperty.packages/@core/base/shared/src/utils/tree.ts (2)
125-125: LGTM: Export properly updated.The
sortTreefunction is correctly added to the public exports, making it available for use in other modules.
113-122: No action required—toSorted()is already established in this codebase.The
toSorted()method is already used in multiple files throughout the project (packages/utils/src/helpers/generate-menus.tsandpackages/stores/src/modules/tabbar.ts), and the project's Node.js requirement (>=20.12.0) already supports ES2023 features since Node.js 20.0.0+. This change follows an existing pattern and does not introduce a new compatibility concern.Likely an incorrect or invalid review comment.
feat: 修正菜单排序在二级菜单不生效问题
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.