fix: header-mixed layout side-menu active #5265
Conversation
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis pull request introduces modifications to the sidebar and menu handling across multiple files in the layout and preferences system. The changes primarily focus on refining the menu management logic, specifically altering the default sidebar preferences, updating the extra menu functionality, and simplifying the mixed menu handling. The modifications aim to provide more flexible menu configuration and improve the interaction between different menu components. Changes
Sequence DiagramsequenceDiagram
participant Layout as Layout Component
participant ExtraMenu as useExtraMenu
participant AccessStore as Access Store
Layout->>ExtraMenu: Call with mixHeaderMenus
ExtraMenu->>AccessStore: Retrieve menus
ExtraMenu-->>Layout: Return extraActiveMenu, extraMenus
Layout->>Layout: Update active menu path
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/effects/layouts/src/basic/menu/use-extra-menu.ts (2)
35-35: Accurate Extraction ofextraActiveMenuReferencing
menu.parents?.[parentLevel.value]ensures correct active menu tracking in mixed layouts. Be sure to test with parentless root menus to avoid undefined references.
74-74: Excess Blank LineMinor nitpick: This lone blank line might not be necessary, but leaving it is stylistically acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/@core/preferences/src/config.ts(1 hunks)packages/effects/layouts/src/basic/layout.vue(2 hunks)packages/effects/layouts/src/basic/menu/use-extra-menu.ts(6 hunks)packages/effects/layouts/src/basic/menu/use-mixed-menu.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/effects/layouts/src/basic/menu/use-mixed-menu.ts
🔇 Additional comments (14)
packages/@core/preferences/src/config.ts (1)
74-74: No Extra Collapsing by Default
Setting extraCollapse to false ensures that the sidebar won’t have additional collapsible behavior by default. Validate with UI tests to ensure this matches user expectations for the default layout.
packages/effects/layouts/src/basic/menu/use-extra-menu.ts (11)
3-3: Ensure Dependency Imports are Used Properly
The addition of ComputedRef is valid, but confirm that you remove any unused imports to keep the file clean.
12-12: Increased Flexibility in useExtraMenu
Accepting useRootMenus as an optional parameter is a neat approach to allow external menu definitions, improving reusability.
16-16: Fallback Logic for Menus
Using useRootMenus?.value ?? accessStore.accessMenus ensures a robust fallback. Double-check that useRootMenus properly resolves before usage to prevent runtime errors.
25-27: Dynamic Parent Level Computation
Good job introducing parentLevel based on the layout preference. Verify that all calling code accounts for both header-mixed-nav and other modes to prevent unexpected results.
60-60: Consistent Setting of Active Menu
Reusing menu.parents?.[parentLevel.value] ?? menu.path maintains consistency across layouts. Great move for preventing logic divergence.
75-78: Enhanced Readability with findRootMenuByPath
Locating menus by path simplifies understanding the active and root menus. Ensure thorough tests for complex nested menu structures.
79-79: Setting extraActiveMenu to Fallback
Defaulting to rootMenuPath or findMenu?.path covers edge cases for root-level items, which is crucial for robust menu highlighting.
80-80: Populating extraMenus with Root Menu Children
Setting extraMenus to rootMenu?.children ?? [] ensures the side extra menu displays all relevant subitems. Validate that empty arrays behave correctly in the UI.
87-87: Consistent Menu Highlighting on Hover
Using menu.parents?.[parentLevel.value] ?? menu.path again grants consistency in highlight logic for hovered items. Check ephemeral states quickly in QA cycles.
97-97: Invoking findRootMenuByPath with parentLevel
This helps adapt root menu lookups dynamically. Make sure the path detection logic is accurate, especially with nested or unusual route structures.
99-102: Storing and Restoring Submenus
Storing the path in defaultSubMap and updating extraRootMenus, extraActiveMenu, and extraMenus collectively ensures the UI remains in sync. Good job centralizing these updates.
packages/effects/layouts/src/basic/layout.vue (2)
110-119: Optional Parameter Usage in useExtraMenu
Passing mixHeaderMenus to useExtraMenu elegantly extends the menu source. Confirm that all relevant computed and reactive data in mixHeaderMenus is stable before passing it in to avoid race conditions.
277-277: Unified active-path Prop
Replacing the conditional expression with :active-path="extraActiveMenu" simplifies the logic and extinguishes potential branching inconsistencies between mixed nav states. Well done.
* fix: header-mixed layout side-menu active * fix: config test
Description
修复水平双列模式下的双列菜单中的一级菜单在点击时没有激活的问题。
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
Release Notes
Preferences
Layout Improvements
Performance