Refactor ModalForm components to use modalProps for title and width configuration#9504
Refactor ModalForm components to use modalProps for title and width configuration#9504zzjiaxiang wants to merge 3 commits intoant-design:masterfrom
Conversation
- Updated ModalForm instances to pass title through modalProps instead of directly. - Ensured consistency across various ModalForm demos and documentation. - Adjusted related documentation to reflect the changes in prop usage.
dosubot
bot
added
the
size:M
Summary of ChangesHello @zzjiaxiang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough本次变更将 ModalForm 的顶层 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| <DrawerForm | ||
| onOpenChange={setDrawerVisit} | ||
| title="Create New Form" | ||
| drawerProps={{ title: 'Create New Form' }} |
There was a problem hiding this comment.
此处的 DrawerForm 示例将 title 属性放在了 drawerProps 中。然而，根据 src/form/layouts/DrawerForm/index.tsx 中的定义，DrawerForm 仍然期望 title 作为其直接属性，而不是 drawerProps 的一部分。这导致了示例与组件实际 API 之间的不一致。请将 title 属性移出 drawerProps，或者更新 DrawerForm 的定义以匹配此用法。
| drawerProps={{ title: 'Create New Form' }} | |
| title="Create New Form" | |
| drawerProps={{}} |
src/form/layouts/ModalForm/index.tsx
Outdated
| <Modal | ||
| title={title} | ||
| width={width || 800} | ||
| {...modalProps} |
There was a problem hiding this comment.
在之前的实现中，如果 width 属性未提供，ModalForm 会默认使用 800px 的宽度 (width || 800)。现在，width 属性被移除，并且 modalProps 被直接展开。这意味着 Modal 组件将使用其自身的默认宽度（通常为 520px），而不是之前的 800px。这可能导致默认布局发生变化，建议在 modalProps 中显式设置一个默认宽度，以保持与之前行为的一致性，或者在文档中明确说明此默认宽度变更。
| {...modalProps} | |
| {...modalProps} | |
| width={modalProps?.width || 800} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/form/layouts/ModalForm/index.tsx (1)
21-64:⚠️ Potential issue | 🟠 Major破坏性变更：移除
title和width直接 prop，未提供迁移降级路径
title和width被移出ModalFormProps后，现有用户使用<ModalForm title="..." />或<ModalForm width={500} />的代码将直接产生 TypeScript 编译错误，且没有任何运行时警告提示迁移方向。组件内已有noteOnce的先例可供参考。建议在完全移除前，保留带有
@deprecated标记的 prop 并发出noteOnce警告，实现平滑迁移：💡 建议的兼容降级方案（提供过渡期）
export type ModalFormProps< T = Record<string, any>, U = Record<string, any>, > = Omit<FormProps<T>, 'onFinish' | 'title'> & CommonFormProps<T, U> & { onFinish?: (formData: T) => Promise<any>; submitTimeout?: number; trigger?: React.JSX.Element; open?: ModalProps['open']; onOpenChange?: (open: boolean) => void; + /** `@deprecated` 请使用 modalProps.title */ + title?: ModalProps['title']; + /** `@deprecated` 请使用 modalProps.width */ + width?: ModalProps['width']; /** `@description` antd Modal [props](https://ant.design/components/modal#api) */ modalProps?: Omit<ModalProps, 'open'>; }; function ModalForm<T = Record<string, any>, U = Record<string, any>>({ children, trigger, onOpenChange, modalProps, onFinish, submitTimeout, open: propsOpen, + title, + width, ...rest }: ModalFormProps<T, U>) { + noteOnce( + !title, + 'ModalForm 的 title prop 已废弃，请改用 modalProps.title', + ); + noteOnce( + !width, + 'ModalForm 的 width prop 已废弃，请改用 modalProps.width', + ); ... <Modal + title={title} + width={width} {...modalProps} open={open}Based on learnings: "Applies to
**/*.{ts,tsx}: Maintain backward compatibility and avoid breaking changes."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/ModalForm/index.tsx` around lines 21 - 64, Reintroduce title and width as deprecated props on ModalFormProps and accept them in the ModalForm parameter list so existing consumers don't get a TS error: add title?: ModalProps['title'] and width?: ModalProps['width'] with `@deprecated` JSDoc, then in ModalForm, if title or width are provided, merge them into modalProps (without overriding explicit modalProps values) and call noteOnce with a clear migration message (referencing ModalForm and ModalFormProps) so users see the deprecation warning at runtime; keep the new modalProps behavior as primary and ensure TypeScript still prefers modalProps when both are supplied.demos/form/ModalForm/modal-form-request-destroy-debug.tsx (1)
33-37:⚠️ Potential issue | 🟡 Minor同文件内
DrawerForm的title用法与ModalForm不一致本文件中
ModalForm已将title移入modalProps，而同一文件的DrawerForm仍使用顶层titleprop，两者风格不统一，阅读时容易产生混淆。若本 PR 仅针对
ModalForm，此处保持不变是合理的；若有意在 demo 层面保持一致，可将其对齐：<DrawerForm - title="Debug: destroyOnHidden + request (Drawer)" trigger={<Button type="primary">Open Drawer (Debug)</Button>} - drawerProps={{ destroyOnHidden: true }} + drawerProps={{ destroyOnHidden: true, title: 'Debug: destroyOnHidden + request (Drawer)' }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demos/form/ModalForm/modal-form-request-destroy-debug.tsx` around lines 33 - 37, The DrawerForm is using the top-level title prop while ModalForm in this file moved title into modalProps, causing inconsistent demo style; update the DrawerForm usage by removing the top-level title prop and placing the same title string into drawerProps (i.e., set drawerProps.title = "Debug: destroyOnHidden + request (Drawer)"), keeping any existing drawerProps like destroyOnHidden intact so the component API usage matches ModalForm's pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@demos/form/ModalForm/open-on-open-change.tsx`:
- Around line 115-118: 将 DrawerForm 的 title 从 drawerProps 中提取出来，改为直接使用 title
prop：在使用的组件 DrawerForm 上移除 drawerProps 中的 title（即不要传 drawerProps={{ title: ...
}}），并添加 title="Create New Form" 同时保留 onOpenChange={setDrawerVisit} 和
open={drawerVisit}，确保只使用 DrawerForm 的一级 API（组件名 DrawerForm，属性
title、drawerProps）。
In `@site/components/modal-form.en-US.md`:
- Line 52: The English docs entry for modalProps uses a hyperlink instead of the
TypeScript type; update the Type column for modalProps to match the Chinese doc
by replacing the link text with the actual type declaration Omit<ModalProps,
'open'> (and keep the note about 'open' not being supported), ensuring the
modalProps row shows the TypeScript type rather than the [props] link; locate
the modalProps table entry in modal-form.en-US.md and make this substitution.
---
Outside diff comments:
In `@demos/form/ModalForm/modal-form-request-destroy-debug.tsx`:
- Around line 33-37: The DrawerForm is using the top-level title prop while
ModalForm in this file moved title into modalProps, causing inconsistent demo
style; update the DrawerForm usage by removing the top-level title prop and
placing the same title string into drawerProps (i.e., set drawerProps.title =
"Debug: destroyOnHidden + request (Drawer)"), keeping any existing drawerProps
like destroyOnHidden intact so the component API usage matches ModalForm's
pattern.
In `@src/form/layouts/ModalForm/index.tsx`:
- Around line 21-64: Reintroduce title and width as deprecated props on
ModalFormProps and accept them in the ModalForm parameter list so existing
consumers don't get a TS error: add title?: ModalProps['title'] and width?:
ModalProps['width'] with `@deprecated` JSDoc, then in ModalForm, if title or width
are provided, merge them into modalProps (without overriding explicit modalProps
values) and call noteOnce with a clear migration message (referencing ModalForm
and ModalFormProps) so users see the deprecation warning at runtime; keep the
new modalProps behavior as primary and ensure TypeScript still prefers
modalProps when both are supplied.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
demos/form/ModalForm/modal-form-request-destroy-debug.tsxdemos/form/ModalForm/modal-form-reset.tsxdemos/form/ModalForm/modal-form-submitter.tsxdemos/form/ModalForm/modal-form.tsxdemos/form/ModalForm/open-on-open-change.tsxsite/components/modal-form.en-US.mdsite/components/modal-form.mdsrc/form/layouts/ModalForm/index.tsx
| <DrawerForm | ||
| onOpenChange={setDrawerVisit} | ||
| title="Create New Form" | ||
| drawerProps={{ title: 'Create New Form' }} | ||
| open={drawerVisit} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find DrawerForm source code
fd "DrawerForm" --type f \( -name "*.tsx" -o -name "*.ts" \) | grep -E "src|components" | head -20Repository: ant-design/pro-components
Length of output: 299
🏁 Script executed:
# Search for drawerProps in the codebase
rg "drawerProps" --type tsx --type ts -C 2 | head -50Repository: ant-design/pro-components
Length of output: 96
🏁 Script executed:
# Check DrawerForm documentation
fd "modal-form" --type f \( -name "*.md" -o -name "*.en-US.md" \) | head -10Repository: ant-design/pro-components
Length of output: 299
🏁 Script executed:
# Find DrawerForm source code
fd "DrawerForm" -e tsx -e ts src components | head -20Repository: ant-design/pro-components
Length of output: 125
🏁 Script executed:
# Search for drawerProps in the codebase
rg "drawerProps" -t ts -A 2 -B 2 | head -80Repository: ant-design/pro-components
Length of output: 3962
🏁 Script executed:
# Check the demo file to see ModalForm usage
cat demos/form/ModalForm/open-on-open-change.tsx | head -50Repository: ant-design/pro-components
Length of output: 1231
🏁 Script executed:
# Find DrawerForm component definition
rg "export.*DrawerForm|const DrawerForm|function DrawerForm" -t ts | head -10Repository: ant-design/pro-components
Length of output: 548
🏁 Script executed:
# Read DrawerForm source implementation
cat src/form/layouts/DrawerForm/index.tsx | head -150Repository: ant-design/pro-components
Length of output: 4138
🏁 Script executed:
# Look for ModalForm source to compare
cat src/form/layouts/ModalForm/index.tsx | head -150Repository: ant-design/pro-components
Length of output: 3866
🏁 Script executed:
# Check if there's a direct title prop in DrawerForm
rg "title" src/form/layouts/DrawerForm -C 3Repository: ant-design/pro-components
Length of output: 1799
🏁 Script executed:
# Check the full demo file to see the complete DrawerForm usage
cat demos/form/ModalForm/open-on-open-change.tsx | tail -50Repository: ant-design/pro-components
Length of output: 1241
DrawerForm 应使用直接 title prop 而非 drawerProps 来保持代码简洁
DrawerForm 的 API 支持直接的 title prop (title?: DrawerProps['title'])，demo 中不必要地通过 drawerProps={{ title: '...' }} 传递。改用 title="Create New Form" 更清晰，也与 DrawerForm 的一级 API 设计一致。
建议修改为：
<DrawerForm
onOpenChange={setDrawerVisit}
title="Create New Form"
open={drawerVisit}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@demos/form/ModalForm/open-on-open-change.tsx` around lines 115 - 118, 将
DrawerForm 的 title 从 drawerProps 中提取出来，改为直接使用 title prop：在使用的组件 DrawerForm 上移除
drawerProps 中的 title（即不要传 drawerProps={{ title: ... }}），并添加 title="Create New
Form" 同时保留 onOpenChange={setDrawerVisit} 和 open={drawerVisit}，确保只使用 DrawerForm
的一级 API（组件名 DrawerForm，属性 title、drawerProps）。
- Updated ModalForm component to ensure modalProps is safely spread, preventing potential errors when modalProps is undefined. - This change enhances the robustness of the ModalForm implementation.
- Revised the documentation for ModalForm parameters to specify that modalProps should omit the 'open' property. - Enhanced clarity and consistency in the documentation to align with recent changes in prop usage.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/form/layouts/ModalForm/index.tsx (1)
265-265:modalProps ?? {}中的空值合并操作是多余的TypeScript 会静默忽略对 null 或 undefined 值的对象展开操作，因此处理可能未定义的值时无需额外的守卫逻辑。对象展开不关心变量是否为 undefined，会静默处理，而数组展开则会抛出错误。
可以直接简化为
{...modalProps}：♻️ 简化建议
- {...(modalProps ?? {})} + {...modalProps}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/ModalForm/index.tsx` at line 265, The object spread uses an unnecessary nullish coalescing: replace the expression that spreads modalProps with a direct spread of modalProps (remove the "?? {}" guard) in the ModalForm component so JSX uses {...modalProps} instead of {...(modalProps ?? {})}; update the spread location where modalProps is applied in the ModalForm (index.tsx) to simplify the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/form/layouts/ModalForm/index.tsx`:
- Around line 264-266: The Modal spread removed the previous default width
(width={width || 800}) causing an unintended shrink to antd's default; restore a
default width before spreading modalProps so callers can still override it by
setting modalProps.width. Update the Modal props construction around the Modal
component (symbols: Modal, modalProps, width, open) to set a default width value
(800) and then spread ...(modalProps ?? {}) so modalProps.width, if present,
overrides the default.
---
Nitpick comments:
In `@src/form/layouts/ModalForm/index.tsx`:
- Line 265: The object spread uses an unnecessary nullish coalescing: replace
the expression that spreads modalProps with a direct spread of modalProps
(remove the "?? {}" guard) in the ModalForm component so JSX uses
{...modalProps} instead of {...(modalProps ?? {})}; update the spread location
where modalProps is applied in the ModalForm (index.tsx) to simplify the code.
| <Modal | ||
| title={title} | ||
| width={width || 800} | ||
| {...modalProps} | ||
| {...(modalProps ?? {})} | ||
| open={open} |
There was a problem hiding this comment.
破坏性变更：移除 width 默认值 800px
原实现中存在 width={width || 800}，为所有未显式设置宽度的用户提供了 800px 默认值。重构后改为 {...(modalProps ?? {})}，若用户未在 modalProps 中指定 width，Modal 将回退到 antd 的默认宽度 520px。这对所有依赖默认 800px 宽度的现有用户来说是不可见的破坏性变更。
建议在 spread 之前声明默认 width，使 modalProps.width 仍可覆盖该默认值：
🐛 修复方案：恢复默认 width
<Modal
+ width={800}
{...(modalProps ?? {})}
open={open}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Modal | |
| title={title} | |
| width={width || 800} | |
| {...modalProps} | |
| {...(modalProps ?? {})} | |
| open={open} | |
| <Modal | |
| width={800} | |
| {...(modalProps ?? {})} | |
| open={open} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/form/layouts/ModalForm/index.tsx` around lines 264 - 266, The Modal
spread removed the previous default width (width={width || 800}) causing an
unintended shrink to antd's default; restore a default width before spreading
modalProps so callers can still override it by setting modalProps.width. Update
the Modal props construction around the Modal component (symbols: Modal,
modalProps, width, open) to set a default width value (800) and then spread
...(modalProps ?? {}) so modalProps.width, if present, overrides the default.
|
不明白为什么要移除外部api，不兼容升级吗？ |
2 participants
将外层的
title和width统一收敛到modalProps里面. 这两个都属于antd modal的属性Summary by CodeRabbit
重构
文档 / 示例