feat: header mixed layout #5263
Conversation
|
|
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
packages/@core/base/typings/src/app.d.tsOops! Something went wrong! :( ESLint: 9.17.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs WalkthroughThis pull request introduces a new layout type called 'header-mixed-nav', which enhances the application's layout flexibility. The changes span multiple packages, adding support for a mixed navigation header layout. The modifications include updating type definitions, creating new computed properties, adjusting menu handling logic, and introducing a new icon for the layout option. The implementation allows for more dynamic and customizable navigation configurations. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (8)
packages/utils/src/helpers/find-menu-by-path.ts (1)
24-24: Ensure clarity in parameter usage.Adding the
level = 0parameter is a good enhancement. Consider documenting in a JSDoc comment how it influences the indexing ofparents, particularly iflevelmight exceed the array bounds.packages/effects/layouts/src/basic/menu/use-extra-menu.ts (3)
3-3: Potential overhead with nextTick.
Check ifnextTick()is necessary at line 56 or if ref updates would suffice immediately.
20-20: Initialize extraRootMenus.
Using a ref forextraRootMenuslooks good. Consider validating or resetting it on layout change if needed.
70-76: Side menu mouse leave logic.
Focus on streamlined code by removing commented-out lines (70-73). This helps improve clarity.- // const { findMenu, rootMenu, rootMenuPath } = findRootMenuByPath( - // menus.value, - // route.path, - // ); calcExtraMenus(route.path);packages/effects/layouts/src/basic/menu/use-mixed-menu.ts (1)
25-27: Conditionally split with isHeaderMixedNav.
This logic is sound, but be sure any future layout expansions remain consistent here.packages/effects/layouts/src/widgets/preferences/icons/header-mixed-nav.vue (2)
1-8: Consider providing accessibility attributes for the SVG.Currently, the markup relies on purely visual attributes, which screen readers may ignore. Consider adding
role="img"and/oraria-label(or marking it as decorative witharia-hidden="true") to improve accessibility.
97-129: Avoid excessiveidattributes if not actively referenced.Unless the
idattributes (e.g.,svg_10,svg_11) are used for advanced styling or scripting, consider removing them to reduce noise in the markup.packages/effects/layouts/src/basic/layout.vue (1)
117-118: Extended mixed-menu support.Introducing
mixedSidebarActiveandmixHeaderMenushelps unify the logic for multiple mixed layout types, allowing the header-based mixed mode to share code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/@core/base/typings/src/app.d.ts(1 hunks)packages/@core/preferences/src/use-preferences.ts(3 hunks)packages/@core/ui-kit/layout-ui/src/hooks/use-layout.ts(1 hunks)packages/@core/ui-kit/layout-ui/src/vben-layout.vue(7 hunks)packages/effects/layouts/src/basic/layout.vue(6 hunks)packages/effects/layouts/src/basic/menu/use-extra-menu.ts(5 hunks)packages/effects/layouts/src/basic/menu/use-mixed-menu.ts(4 hunks)packages/effects/layouts/src/widgets/preferences/blocks/layout/layout.vue(3 hunks)packages/effects/layouts/src/widgets/preferences/blocks/layout/sidebar.vue(1 hunks)packages/effects/layouts/src/widgets/preferences/icons/header-mixed-nav.vue(1 hunks)packages/effects/layouts/src/widgets/preferences/icons/index.ts(1 hunks)packages/locales/src/langs/zh-CN/preferences.json(1 hunks)packages/utils/src/helpers/find-menu-by-path.ts(1 hunks)
🔇 Additional comments (32)
packages/effects/layouts/src/widgets/preferences/icons/index.ts (1)
5-5: Smooth integration of new layout icon export.
The addition of HeaderMixedNav export aligns seamlessly with your new layout type. Great to see consistency across the codebase.
packages/utils/src/helpers/find-menu-by-path.ts (1)
26-26: Validate the presence of parent paths.
findMenu?.parents?.[level] could potentially be undefined if parents is absent or shorter than level. You might want to handle edge cases or return early if parents[level] is not found.
packages/@core/ui-kit/layout-ui/src/hooks/use-layout.ts (1)
34-40: Excellent addition of isHeaderMixedNav.
The new computed property cleanly extends the useLayout hook, enabling detection of 'header-mixed-nav'. This change aligns with the new layout option and maintains consistent naming conventions.
Also applies to: 44-44
packages/@core/base/typings/src/app.d.ts (1)
3-3: Layout type extension is consistent.
The addition of 'header-mixed-nav' in LayoutType complements the new layout functionality without breaking existing types.
packages/effects/layouts/src/widgets/preferences/blocks/layout/sidebar.vue (1)
47-47: Ensure consistent naming.
Including 'header-mixed-nav' here aligns with the new layout type. Verify if 'mixed-nav' remains relevant or is partially replaced by 'header-mixed-nav'.
packages/effects/layouts/src/widgets/preferences/blocks/layout/layout.vue (3)
12-12: Import is correct.
The new HeaderMixedNav import follows the consistent naming convention for icons. Looks good.
37-37: Clean expansion of layout.
Adding 'header-mixed-nav': HeaderMixedNav extends the existing record effectively.
61-65: Addition of headerTwoColumn layout.
Adding a new preset for 'header-mixed-nav' is coherent with the design. Ensure the UI translations for headerTwoColumn and headerTwoColumnTip exist.
packages/effects/layouts/src/basic/menu/use-extra-menu.ts (3)
52-58: Asynchronous default menu selection.
Waiting for the DOM update is fine, but ensure that any dependent code also awaits the same cycle if needed.
91-115: Centralized extra menu calculation.
calcExtraMenus significantly improves code organization and maintainability. Confirm if all calls to calcExtraMenus handle potential absence of rootMenu.
117-120: Reactive watch on route & layout.
Watching both route.path and preferences.app.layout ensures the menu updates properly. Good job.
packages/effects/layouts/src/basic/menu/use-mixed-menu.ts (6)
18-19: Extra references for root menu state.
Storing the mixedRootMenuPath and mixExtraMenus is straightforward and keeps logic organized.
22-22: Destructured isHeaderMixedNav.
Neatly integrates the new layout type within existing preferences.
61-63: Header menus with new layout.
mixHeaderMenus effectively merges or replaces headerMenus depending on isHeaderMixedNav. Looks good.
72-74: Sidebar active path.
Using mixedRootMenuPath for the active path properly aligns with the “header-mixed-nav” use case.
133-135: Refined side menus calculation.
Good approach to set the correct root path and store extra menus. Validates well for hierarchical menus.
163-165: Exposing computed references.
Exposing mixedSidebarActive, mixHeaderMenus, and mixExtraMenus fosters reusability across layouts.
packages/effects/layouts/src/widgets/preferences/icons/header-mixed-nav.vue (1)
22-26: Good use of theme-based color tokens.
Using hsl(var(--primary)) ensures the icon remains consistent with the application’s theme, making it more maintainable and easier to adapt for dark/light themes.
packages/@core/preferences/src/use-preferences.ts (2)
85-87: Introduction of isHeaderMixedNav is consistent with new layout.
This provides a clear computed property for detecting the 'header-mixed-nav' layout. Implementation is straightforward and matches the project's structure.
100-105: Verify the side-mode logic for 'header-mixed-nav'.
Including isHeaderMixedNav.value in isSideMode might cause confusion about whether the “header mixed” layout is truly side-based. Confirm that this is intended, especially if future logic expects 'header-mixed-nav' to behave differently from side-based modes.
packages/effects/layouts/src/basic/layout.vue (5)
42-42: Properly retrieving isHeaderMixedNav from preferences.
Good job referencing the new isHeaderMixedNav property. This seamlessly integrates into the existing layout checks.
87-89: Unified approach to collapsible logos.
By extending the condition with isHeaderMixedNav.value, you ensure that the logo collapses consistently across the new layout mode.
93-96: Consistent header visibility checks.
Including isHeaderMixedNav.value ensures that the header navigation is shown in the new layout mode alongside the existing ones.
165-165: Accurate type annotation for setup context slots.
Using SetupContext['slots'] improves clarity and type safety when referencing slot properties.
278-279: Dynamic active path for mixed menu.
Conditionally using mixedSidebarActive or extraActiveMenu promotes consistency when toggling between header-based and side-based mixed modes.
packages/@core/ui-kit/layout-ui/src/vben-layout.vue (6)
90-90: New isHeaderMixedNav import.
The additional computed property keeps the logic consistent across layout variations. No issues observed.
116-118: Conditional collapse width for header-mixed layouts.
Adding isHeaderMixedNav.value ensures the sidebar respects the new layout setting when collapsed, preserving consistent UI behavior.
151-154: Avoid hidden sidebar conflicts.
Explicitly excluding isHeaderMixedNav.value from hidden states ensures the sidebar remains properly visible in the new layout variant.
159-159: Handle normal vs. mixed nav usage.
Correctly applying isHeaderMixedNav.value || isSidebarMixedNav.value means the code gracefully differentiates between layouts, especially for non-mobile states.
185-186: Comprehensive addition to side-mode detection.
Including 'header-mixed-nav' in the isSideMode check ensures consistent features — such as sidebars — remain active in the new layout.
223-223: Maintaining correct layout expansions.
Using (isSidebarMixedNav.value || isHeaderMixedNav.value) to conditionally expand side menus avoids unexpected overlaps or hidden areas, especially during hover.
packages/locales/src/langs/zh-CN/preferences.json (1)
20-22: Well-structured translations that clearly describe the new layout options!
The translations are clear, consistent, and follow good practices:
- Descriptive Chinese terms that accurately reflect the layout behavior
- Consistent terminology with existing layout options
- Clear tooltip explaining the dual-column and horizontal menu coexistence
* feat: new layout header-mixed * fix: header-mixed layout update * feat: layout preference update * fix: extra menus follow layout setting
Description
新增布局：水平菜单+双列菜单混合。类似混合模式，但左侧使用双列菜单
close #5219
request #5253
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
New Features
header-mixed-navwith mixed navigation header configurationLocalization
UI Improvements