feat: new layout sidebar nav with full header
#5270
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 Changes
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: 1
🧹 Nitpick comments (4)
packages/effects/layouts/src/widgets/preferences/icons/header-sidebar-nav.vue (3)
1-8: Consider adding aviewBoxattribute for responsive scalability.Currently, the
<svg>element sets explicitwidth="104"andheight="66". Adding aviewBoxattribute (e.g.,viewBox="0 0 104 66") makes the icon scale nicely in different contexts without distortion.
140-140: Use consistent color assignment or accept a prop for theming.
fill="hsl(var(--primary))"is a powerful approach, enabling theme-based coloring. For consistency, consider using similar dynamic color approaches or a prop-based color configuration for the other shapes if you want them to adapt to theme changes as well.
9-174: Remove unnecessaryid,stroke="null", and extra attributes if not used.This file includes several unused attributes (e.g.,
id="svg_1",stroke="null") that might clutter the SVG. Consider removing them to keep the markup clean and focused.packages/locales/src/langs/en-US/preferences.json (1)
20-21: Clarify the naming of “Header Vertical.”“Header Vertical” might confuse some users since there’s also a sidebar involved. Consider rephrasing to something like “Header & Sidebar” to emphasize both the top header and sidebar usage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/@core/base/typings/src/app.d.ts(1 hunks)packages/@core/ui-kit/layout-ui/src/hooks/use-layout.ts(1 hunks)packages/@core/ui-kit/layout-ui/src/vben-layout.vue(2 hunks)packages/effects/layouts/src/widgets/preferences/blocks/layout/layout.vue(3 hunks)packages/effects/layouts/src/widgets/preferences/icons/header-sidebar-nav.vue(1 hunks)packages/effects/layouts/src/widgets/preferences/icons/index.ts(1 hunks)packages/locales/src/langs/en-US/preferences.json(1 hunks)packages/locales/src/langs/zh-CN/preferences.json(1 hunks)
🔇 Additional comments (9)
packages/effects/layouts/src/widgets/preferences/icons/index.ts (1)
6-6: New export looks good.
Exporting HeaderSidebarNav helps maintain a clear entry point for the new feature.
packages/@core/ui-kit/layout-ui/src/hooks/use-layout.ts (1)
32-36: Logical extension of 'isMixedNav'.
Expanding isMixedNav to recognize 'header-sidebar-nav' aligns well with the naming convention. Just ensure there’s no unexpected overlap between 'mixed-nav' and 'header-sidebar-nav' in UI logic or interactions.
packages/@core/base/typings/src/app.d.ts (1)
5-5: Clean and simple addition of new layout type.
You’ve cleanly extended the LayoutType to include 'header-sidebar-nav'. This change is straightforward and poses no conflicts.
packages/effects/layouts/src/widgets/preferences/blocks/layout/layout.vue (3)
14-14: Importing the new ‘HeaderSidebarNav’ component is consistent.
This step seamlessly integrates the new navigation layout component into the component set.
39-39: Adding 'header-sidebar-nav' to the component map.
This ensures the dynamic component rendering properly identifies and loads the new layout.
58-62: Registering the new layout in PRESET.
Your new localized entries and layout option in PRESET follow the existing pattern, providing a clear, user-friendly choice in the UI.
packages/@core/ui-kit/layout-ui/src/vben-layout.vue (2)
186-187: Ensure consistent naming for the new layout type.
The logic includes 'header-mixed-nav' and now 'header-sidebar-nav'. Confirm that your naming scheme is intuitive and consistent across the codebase so future contributors can understand the differences at a glance.
219-219: Double-check the conditional for currentLayout.value !== 'header-sidebar-nav'.
The new layout excludes the logic that adjusts the main content width in fixed mode. Verify whether 'header-sidebar-nav' should share this logic or truly be excluded. If it should also have a fixed sidebar width, consider removing it from this exclusion.
packages/locales/src/langs/zh-CN/preferences.json (1)
20-21: Great job localizing the new layout mode!
Adding "headerSidebarNav": "侧边导航" and "headerSidebarNavTip": "顶部通栏，侧边导航模式" neatly integrates the latest feature into the Chinese translation.
packages/effects/layouts/src/widgets/preferences/icons/header-sidebar-nav.vue
Show resolved
Hide resolved
Description
添加新的布局模式：顶部通栏+侧边导航模式
close #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:.