refactor(layout): declare mix layout on ProLayoutProps only#9600
refactor(layout): declare mix layout on ProLayoutProps only#9600chenshuai2144 merged 1 commit intomasterfrom
Conversation
- Keep PureSettings.layout as side | top only - Declare ProLayoutLayoutMode on ProLayout root props (includes mix) - Normalize mix to side for Header, SiderMenu, and menuRender callback - Export ProLayoutMenuRenderCallbackProps for menuRender typings Co-authored-by: 陈帅 <wasd2144@hotmail.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the layout mode handling in ProLayout by deprecating the 'mix' layout and introducing a normalization helper to treat it as 'side' at runtime. It also introduces new types for menu rendering callbacks to ensure consistent layout values. A suggestion was made to refine the ProLayoutMenuRenderCallbackProps type by making the layout property required, as it is always normalized before being passed to the callback.
| export type ProLayoutMenuRenderCallbackProps = Omit<ProLayoutProps, 'layout'> & { | ||
| layout?: NonNullable<ProSettings['layout']>; | ||
| }; |
There was a problem hiding this comment.
The layout property in ProLayoutMenuRenderCallbackProps is marked as optional, but the toMenuRenderCallbackProps helper function always provides a normalized value (either 'side' or 'top'). Making this property required would provide more accurate type information for users implementing the menuRender callback, ensuring they know the layout is always defined and normalized at that point.
| export type ProLayoutMenuRenderCallbackProps = Omit<ProLayoutProps, 'layout'> & { | |
| layout?: NonNullable<ProSettings['layout']>; | |
| }; | |
| export type ProLayoutMenuRenderCallbackProps = Omit<ProLayoutProps, 'layout'> & { | |
| layout: NonNullable<ProSettings['layout']>; | |
| }; |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9600 +/- ##
==========================================
- Coverage 88.77% 88.72% -0.05%
==========================================
Files 360 360
Lines 11159 11161 +2
Branches 4133 4131 -2
==========================================
- Hits 9906 9903 -3
- Misses 1104 1108 +4
- Partials 149 150 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dosubot
Bot
added
the
size:M
There was a problem hiding this comment.
Pull request overview
Refactors layout typing to keep legacy layout="mix" support only at the ProLayout root prop level while keeping internal settings/components (PureSettings, Header, SiderMenu) typed as 'side' | 'top', with runtime normalization of mix -> side.
Changes:
- Narrow
PureSettings.layoutback to'side' | 'top'and removemixfrom its docs/type. - Introduce
ProLayoutLayoutMode('side' | 'top' | 'mix') onProLayoutProps.layout, and normalizemix/undefinedto'side'when passing layout toHeader,SiderMenu, andmenuRender. - Export
ProLayoutLayoutModeandProLayoutMenuRenderCallbackPropsfromsrc/layout/index.tsx.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/layout/index.tsx | Re-exports new public types from ProLayout for external consumers. |
| src/layout/defaultSettings.ts | Narrows PureSettings.layout to `'side' |
| src/layout/ProLayout.tsx | Adds legacy-aware root layout type + normalization helper; applies normalized layout when rendering header/sider and invoking menuRender. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export type ProLayoutProps = Omit<GlobalTypes, 'layout'> & { | ||
| /** | ||
| * @name layout 的布局方式（根组件） | ||
| * @deprecated `layout="mix"` 仍会被接受，运行时按 `side` 处理 |
There was a problem hiding this comment.
The @deprecated tag on ProLayoutProps.layout will mark the entire layout prop as deprecated (including valid 'side' | 'top' usages) in TS tooling. If the intent is only to deprecate the 'mix' value, move the deprecation marker to the union member (e.g. deprecate 'mix' specifically) and keep the prop itself non-deprecated, or replace @deprecated with a descriptive note without the tag.
| * @deprecated `layout="mix"` 仍会被接受，运行时按 `side` 处理 | |
| * 注意：`layout="mix"` 仍会被接受，运行时按 `side` 处理 |
| ): NonNullable<ProSettings['layout']> => | ||
| layout === 'mix' || layout === undefined ? 'side' : layout; | ||
|
|
||
| /** `menuRender` 回调首参：与 Header 一致，不含历史 `mix` */ |
There was a problem hiding this comment.
The doc comment for ProLayoutMenuRenderCallbackProps says it's "与 Header 一致", but the type is based on ProLayoutProps (minus layout) and therefore includes many props that HeaderViewProps doesn't. Consider rewording to clarify that this type only guarantees the layout field matches HeaderViewProps (i.e., no legacy mix), rather than implying the whole shape is identical.
| /** `menuRender` 回调首参：与 Header 一致，不含历史 `mix` */ | |
| /** | |
| * `menuRender` 回调首参：基于 `ProLayoutProps`，但其 `layout` 字段与 Header 保持一致， | |
| * 不含历史 `mix` | |
| */ |
Summary
PureSettings.layoutindefaultSettingsis again only'side' | 'top'. The deprecated root valuelayout="mix"is typed onProLayoutviaProLayoutLayoutModeonProLayoutProps(Omit<GlobalTypes, 'layout'>so it does not widenSiderMenuProps/BaseMenuProps).Runtime:
mixis normalized tosidewhen passinglayoutintoHeaderandSiderMenu, and when invokingmenuRender(newProLayoutMenuRenderCallbackProps+toMenuRenderCallbackPropsso the callback receivesHeaderViewProps-compatible props).Exports
ProLayoutLayoutModeProLayoutMenuRenderCallbackProps(fromsrc/layout/index.tsx)Verification
pnpm run tscpnpm exec vitest run tests/layout/index.test.tsxSummary by CodeRabbit
发布说明