Refactor DrawerForm: use antd resizable api#9502
Refactor DrawerForm: use antd resizable api#9502zzjiaxiang wants to merge 10 commits intoant-design:masterfrom
Conversation
- Set the title and size properties for DrawerForm in multiple demo files to enhance consistency. - Removed deprecated width property and adjusted drawerProps accordingly. - Updated documentation to reflect changes in DrawerForm properties.
dosubot
bot
added
the
size:L
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! 此拉取请求重构了 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
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough概览本次更改重构了 DrawerForm 组件，移除了可拖拽/可调整大小的功能，整合了 drawer 配置属性，简化了组件的 API 表面和实现，并相应更新了演示和文档。 变更
代码审查工作量估算🎯 4 (复杂) | ⏱️ ~45 分钟 诗
🚥 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)
📝 Coding Plan
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
The pull request refactors the DrawerForm component by moving title and width properties into drawerProps for better consistency and removes the custom resize logic in favor of Ant Design's built-in resizable drawer functionality. The changes simplify the DrawerForm component's API and align it with Ant Design's latest features. The documentation and examples have been updated accordingly.
I am having trouble creating individual review comments. Click here to see my feedback.
demos/form/ModalForm/drawer-form-nested.tsx (35-37)
The title and width props are being removed from the DrawerForm component directly. They should now be passed within the drawerProps object to maintain consistency with the refactored DrawerForm API.
demos/form/ModalForm/drawer-form-nested.tsx (61)
The title prop is being removed from the DrawerForm component directly. It should now be passed within the drawerProps object to maintain consistency with the refactored DrawerForm API.
demos/form/ModalForm/drawer-form.tsx (28-35)
The title and resize props are being removed from the DrawerForm component directly. These functionalities are now handled within drawerProps for a more unified API.
demos/form/ModalForm/modal-form-request-destroy-debug.tsx (32)
The title prop is being removed from the DrawerForm component directly. It should now be passed within the drawerProps object to maintain consistency with the refactored DrawerForm API.
demos/form/ModalForm/open-on-open-change.tsx (117)
The title prop is being removed from the DrawerForm component directly. It should now be passed within the drawerProps object to maintain consistency with the refactored DrawerForm API.
site/components/modal-form.md (70-72)
The title, width, and resize properties are removed from the DrawerForm API documentation. This reflects the refactoring where these properties are now managed within drawerProps.
src/form/layouts/DrawerForm/index.tsx (62-72)
The title, width, and resize props are removed from the DrawerFormProps interface. These properties are now expected to be passed within the drawerProps object.
src/form/layouts/DrawerForm/index.tsx (81-83)
The title, width, and resize props are destructured from the component props. These are no longer directly accepted by DrawerForm and should be removed.
src/form/layouts/DrawerForm/index.tsx (92-109)
The resizeInfo memoization and related logic for custom resizing are removed. This functionality is now handled by Ant Design's native resizable prop within DrawerProps.
src/form/layouts/DrawerForm/index.tsx (119-123)
The resizableDrawer state and drawerWidth state, along with their initialization, are removed. These were part of the custom resize implementation which is now deprecated.
src/form/layouts/DrawerForm/index.tsx (189-191)
The conditional logic for resizableDrawer and setDrawerWidth is removed. This was part of the custom resize implementation which is no longer used.
src/form/layouts/DrawerForm/index.tsx (192)
The resizableDrawer dependency is removed from the useEffect hook's dependency array, as it is no longer relevant after the custom resize logic removal.
}, [propsOpen, open]);
src/form/layouts/DrawerForm/index.tsx (212)
The setResizableDrawer call is removed from the onClick handler. This was part of the custom resize implementation which is no longer used.
src/form/layouts/DrawerForm/index.tsx (216)
The setResizableDrawer and resizableDrawer dependencies are removed from the useMemo hook's dependency array, as they are no longer relevant after the custom resize logic removal.
}, [setOpen, trigger, open]);
src/form/layouts/DrawerForm/index.tsx (311-336)
The cbHandleMouseMove and cbHandleMouseUp callbacks, which implemented the custom drawer resizing logic, are removed. This functionality is now handled by Ant Design's native resizable prop.
src/form/layouts/DrawerForm/index.tsx (342-346)
The destroyOnHidden, title, and size props are removed from the Drawer component directly. These are now expected to be passed within the drawerProps object.
src/form/layouts/DrawerForm/index.tsx (373-389)
The conditional rendering of the sidebar-dragger div and its associated onMouseDown logic are removed. This was part of the custom resize implementation which is no longer used.
src/form/layouts/DrawerForm/style.ts (1-43)
The style.ts file, which contained custom styling for the resizable drawer, is completely removed. This is consistent with the refactoring to use Ant Design's native resizable drawer functionality.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
> [!CAUTION]
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)src/form/layouts/DrawerForm/index.tsx (3)
21-53:⚠️ Potential issue | 🟠 Major移除
title/width属于 Breaking Change，需提供迁移路径
title和width从DrawerFormProps中删除，意味着所有在<DrawerForm title="..." width={600}>上直接使用这两个属性的存量代码将静默失效（TypeScript 用户会收到编译报错，JS 用户会无声无息地丢失这两个属性）。建议先以
@deprecated方式保留这两个 prop，内部转发给drawerProps，并在 CHANGELOG 中说明迁移路径，待下一个 major 版本再彻底移除。export type DrawerFormProps< T = Record<string, any>, U = Record<string, any>, > = Omit<FormProps, 'onFinish' | 'title'> & CommonFormProps<T, U> & { + /** + * `@deprecated` 请使用 drawerProps.title 代替 + */ + title?: DrawerProps['title']; + /** + * `@deprecated` 请使用 drawerProps.width 代替 + */ + width?: DrawerProps['width']; ... drawerProps?: Omit<DrawerProps, 'open'>; };对应在组件函数内部：
function DrawerForm<T = Record<string, any>, U = Record<string, any>>({ children, trigger, drawerProps, + title, + width, ... }: DrawerFormProps<T, U>) { ... // 将 deprecated 的 prop 合并到 drawerProps + const mergedDrawerProps = { title, width, ...drawerProps };根据编码规范："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/DrawerForm/index.tsx` around lines 21 - 53, DrawerFormProps currently removed title and width which is a breaking change; restore them as optional (mark with `@deprecated`) in the DrawerFormProps type and update the DrawerForm component to forward any deprecated title/width into drawerProps (e.g., if title !== undefined set drawerProps.title = title; if width !== undefined set drawerProps.width = width) while keeping onFinish/submitTimeout behavior unchanged; also ensure the component emits a console.warn or logger deprecation message when deprecated props are used and document the migration in the CHANGELOG so these props can be removed in the next major version.
256-270:⚠️ Potential issue | 🟡 Minor
afterOpenChange与queueMicrotask双重触发onOpenChange
setOpen通过queueMicrotask已经异步通知了onOpenChangeCallback（Line 98–100），而afterOpenChange回调又在抽屉动画完成后再次调用onOpenChange?.(open)（Line 266），导致每次开/关操作onOpenChange均被触发两次。对于受控模式（通过
propsOpen控制），还会叠加useEffect（Line 136）中的第三次调用。建议从
afterOpenChange中移除onOpenChange?.(open)调用，仅保留对用户自定义drawerProps.afterOpenChange的透传：afterOpenChange={(open) => { if (!open && drawerProps?.destroyOnHidden) { resetFields(); } drawerProps?.afterOpenChange?.(open); - onOpenChange?.(open); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/DrawerForm/index.tsx` around lines 256 - 270, The afterOpenChange handler in Drawer currently calls onOpenChange?.(open) which duplicates notifications already dispatched asynchronously by setOpen (via queueMicrotask in the onOpenChangeCallback) and can cause a third call from the useEffect in controlled mode; remove the onOpenChange?.(open) invocation from the afterOpenChange block and only forward to drawerProps?.afterOpenChange?.(open) so user-provided drawerProps callback still runs but the duplicate onOpenChange call is eliminated.
134-138:⚠️ Potential issue | 🟡 Minor
onOpenChange未加入useEffect依赖数组 — 存在陈旧闭包风险
onOpenChange在 effect 内部直接被调用，但未被列入依赖数组。若调用方传入的是非稳定引用的函数，该 effect 将始终持有其旧版本。-}, [propsOpen, open]); +}, [propsOpen, open, onOpenChange]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/DrawerForm/index.tsx` around lines 134 - 138, The effect in DrawerForm (useEffect that checks if open && propsOpen and calls onOpenChange?.(true)) uses onOpenChange without listing it in the dependency array, risking a stale closure; update the effect to include onOpenChange in the dependency array (i.e., [propsOpen, open, onOpenChange]) or alternatively stabilize the callback passed into the component (e.g., wrap the prop with useCallback/useRef) so that the onOpenChange used inside useEffect is always current.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/form/layouts/DrawerForm/index.tsx`:
- Around line 21-53: DrawerFormProps currently removed title and width which is
a breaking change; restore them as optional (mark with `@deprecated`) in the
DrawerFormProps type and update the DrawerForm component to forward any
deprecated title/width into drawerProps (e.g., if title !== undefined set
drawerProps.title = title; if width !== undefined set drawerProps.width = width)
while keeping onFinish/submitTimeout behavior unchanged; also ensure the
component emits a console.warn or logger deprecation message when deprecated
props are used and document the migration in the CHANGELOG so these props can be
removed in the next major version.
- Around line 256-270: The afterOpenChange handler in Drawer currently calls
onOpenChange?.(open) which duplicates notifications already dispatched
asynchronously by setOpen (via queueMicrotask in the onOpenChangeCallback) and
can cause a third call from the useEffect in controlled mode; remove the
onOpenChange?.(open) invocation from the afterOpenChange block and only forward
to drawerProps?.afterOpenChange?.(open) so user-provided drawerProps callback
still runs but the duplicate onOpenChange call is eliminated.
- Around line 134-138: The effect in DrawerForm (useEffect that checks if open
&& propsOpen and calls onOpenChange?.(true)) uses onOpenChange without listing
it in the dependency array, risking a stale closure; update the effect to
include onOpenChange in the dependency array (i.e., [propsOpen, open,
onOpenChange]) or alternatively stabilize the callback passed into the component
(e.g., wrap the prop with useCallback/useRef) so that the onOpenChange used
inside useEffect is always current.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
demos/form/ModalForm/drawer-form-nested.tsxdemos/form/ModalForm/drawer-form.tsxdemos/form/ModalForm/modal-form-request-destroy-debug.tsxdemos/form/ModalForm/open-on-open-change.tsxsite/components/modal-form.mdsrc/form/layouts/DrawerForm/index.tsxsrc/form/layouts/DrawerForm/style.ts
💤 Files with no reviewable changes (2)
- site/components/modal-form.md
- src/form/layouts/DrawerForm/style.ts
zzjiaxiang
changed the title
…rect drawerProps links - Removed the 'resize' and 'width' properties from the documentation as they are no longer supported. - Updated the 'drawerProps' link to point to the correct antd documentation. - Cleaned up the 'open' property description for clarity.
There was a problem hiding this comment.
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/DrawerForm/index.tsx (2)
256-267:⚠️ Potential issue | 🟡 Minor避免
onOpenChange被调用两次。
setOpen已通过onOpenChangeCallback触发回调，这里又在afterOpenChange里调用一次，会导致内部触发的开关事件回调重复，可能引发重复副作用。🧩 建议修改
afterOpenChange={(open) => { if (!open && drawerProps?.destroyOnHidden) { resetFields(); } drawerProps?.afterOpenChange?.(open); - onOpenChange?.(open); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/DrawerForm/index.tsx` around lines 256 - 267, The afterOpenChange handler on the Drawer is calling onOpenChange again which duplicates the callback already invoked via setOpen/onOpenChangeCallback; update the Drawer afterOpenChange implementation (the anonymous function passed to the Drawer component) to only run local logic (e.g., resetFields and drawerProps?.afterOpenChange?.(open)) and remove the direct call to onOpenChange?.(open) (or add a guard to skip calling onOpenChange when the change originated from setOpen/onOpenChangeCallback) so the onOpenChange callback is not invoked twice.
21-53:⚠️ Potential issue | 🟠 Major为避免破坏性变更，建议保留顶层兼容字段并做弃用映射。
当前移除
title / width(size) / destroyOnHidden的顶层字段会让现有调用方在升级时直接编译/运行失败。建议临时保留这些字段（标注@deprecated），并在运行时合并到drawerProps，同时noteOnce提示迁移，以便平滑升级。🛠️ 建议实现（示例）
export type DrawerFormProps< T = Record<string, any>, U = Record<string, any>, > = Omit<FormProps, 'onFinish' | 'title'> & CommonFormProps<T, U> & { + /** `@deprecated` 请使用 drawerProps.title */ + title?: DrawerProps['title']; + /** `@deprecated` 请使用 drawerProps.size */ + size?: DrawerProps['size']; + /** `@deprecated` 请使用 drawerProps.width */ + width?: DrawerProps['width']; + /** `@deprecated` 请使用 drawerProps.destroyOnHidden */ + destroyOnHidden?: DrawerProps['destroyOnHidden']; /** `@name` antd的Drawer组件[props](https://ant-design.antgroup.com/components/drawer#api) */ drawerProps?: Omit<DrawerProps, 'open'>; };function DrawerForm<T = Record<string, any>, U = Record<string, any>>({ children, trigger, drawerProps, + title, + size, + width, + destroyOnHidden, onFinish, submitTimeout, onOpenChange, open: propsOpen, ...rest }: DrawerFormProps<T, U>) { + noteOnce( + !title && !size && !width && !destroyOnHidden, + 'DrawerForm 的 title/size/width/destroyOnHidden 已废弃，请迁移到 drawerProps。', + ); + + const mergedDrawerProps = useMemo( + () => ({ + ...drawerProps, + title: drawerProps?.title ?? title, + size: drawerProps?.size ?? size, + width: drawerProps?.width ?? width, + destroyOnHidden: drawerProps?.destroyOnHidden ?? destroyOnHidden, + }), + [drawerProps, title, size, width, destroyOnHidden], + );<Drawer - {...drawerProps} + {...mergedDrawerProps} open={open}As per coding guidelines, 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/DrawerForm/index.tsx` around lines 21 - 53, The PR removed top-level props (title, width/size, destroyOnHidden) from DrawerFormProps which will break callers; restore these top-level fields as `@deprecated` in DrawerFormProps and at runtime merge their values into drawerProps when present (e.g., detect deprecatedTitle/deprecatedWidth/deprecatedDestroyOnHidden and copy them into drawerProps before rendering), emit a one-time migration warning (noteOnce) advising users to move to drawerProps, and keep existing props like onFinish, submitTimeout, trigger, open, onOpenChange untouched; update type definitions and the component's props handling to prefer drawerProps values but fall back to the deprecated top-level values when provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/form/layouts/DrawerForm/index.tsx`:
- Around line 256-267: The afterOpenChange handler on the Drawer is calling
onOpenChange again which duplicates the callback already invoked via
setOpen/onOpenChangeCallback; update the Drawer afterOpenChange implementation
(the anonymous function passed to the Drawer component) to only run local logic
(e.g., resetFields and drawerProps?.afterOpenChange?.(open)) and remove the
direct call to onOpenChange?.(open) (or add a guard to skip calling onOpenChange
when the change originated from setOpen/onOpenChangeCallback) so the
onOpenChange callback is not invoked twice.
- Around line 21-53: The PR removed top-level props (title, width/size,
destroyOnHidden) from DrawerFormProps which will break callers; restore these
top-level fields as `@deprecated` in DrawerFormProps and at runtime merge their
values into drawerProps when present (e.g., detect
deprecatedTitle/deprecatedWidth/deprecatedDestroyOnHidden and copy them into
drawerProps before rendering), emit a one-time migration warning (noteOnce)
advising users to move to drawerProps, and keep existing props like onFinish,
submitTimeout, trigger, open, onOpenChange untouched; update type definitions
and the component's props handling to prefer drawerProps values but fall back to
the deprecated top-level values when provided.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
site/components/modal-form.en-US.mdsite/components/modal-form.mdsrc/form/layouts/DrawerForm/index.tsx
✅ Files skipped from review due to trivial changes (1)
- site/components/modal-form.en-US.md
🚧 Files skipped from review as they are similar to previous changes (1)
- site/components/modal-form.md
… and Chinese versions - Fixed the link for 'drawerProps' in both English and Chinese documentation to point to the correct antd documentation. - Ensured consistency in property descriptions across language versions.
There was a problem hiding this comment.
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/DrawerForm/index.tsx (2)
134-138:⚠️ Potential issue | 🟡 Minor
useEffect缺少onOpenChange依赖 / 应使用稳定 ref 包装器第 136 行在 effect 内部直接调用了
onOpenChange?.(true)，但onOpenChange未出现在依赖数组中，存在过期闭包风险。与其他位置的用法保持一致，应改为使用已有的稳定 ref 包装器onOpenChangeCallback。🐛 建议修复
useEffect(() => { if (open && propsOpen) { - onOpenChange?.(true); + onOpenChangeCallback(true); } }, [propsOpen, open, onOpenChangeCallback]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/DrawerForm/index.tsx` around lines 134 - 138, The effect currently calls onOpenChange directly (useEffect(() => { if (open && propsOpen) { onOpenChange?.(true); } }, [propsOpen, open])) causing a missing-deps stale-closure risk; replace the direct call with the stable ref wrapper onOpenChangeCallback (e.g. call onOpenChangeCallback.current or the wrapper function) inside the effect so you don't need to add onOpenChange to the deps, keeping the dependency array as [propsOpen, open] and using onOpenChangeCallback to invoke the handler safely.
261-267:⚠️ Potential issue | 🟠 Major
onOpenChange被重复调用（每次开/关触发两次）
setOpen内部已通过queueMicrotask(() => onOpenChangeCallback(next))调用了onOpenChange（line 99）。而afterOpenChange回调（动画结束后触发）又再次直接调用了onOpenChange?.(open)（line 266）。这会导致每次打开或关闭抽屉时，消费者的onOpenChange回调被触发两次。如果
afterOpenChange中调用onOpenChange的目的是在动画结束后再次通知（行为有别于setOpen中的即时通知），应该通过drawerProps?.afterOpenChange向消费者暴露动画结束事件，而非复用onOpenChange。如果目的是保持与 antd 原生afterOpenChange一致的语义，则应移除此处对onOpenChange的直接调用。🐛 建议修复
afterOpenChange={(open) => { if (!open && drawerProps?.destroyOnHidden) { resetFields(); } drawerProps?.afterOpenChange?.(open); - onOpenChange?.(open); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/DrawerForm/index.tsx` around lines 261 - 267, The afterOpenChange handler currently calls onOpenChange directly causing duplicate notifications because setOpen already queues onOpenChange; remove the direct call to onOpenChange in the afterOpenChange block and only invoke drawerProps?.afterOpenChange?.(open) (and keep resetFields logic) so consumers receive the animation-end event via drawerProps.afterOpenChange instead of a second onOpenChange invocation; locate the logic around afterOpenChange in DrawerForm (the afterOpenChange arrow function) and delete the onOpenChange?.(open) call.
🧹 Nitpick comments (2)
src/form/layouts/DrawerForm/index.tsx (2)
278-283: Footer<div>使用了内联样式根据编码规范，应使用 Tailwind 类替代内联
style属性。♻️ 建议重构
<div ref={footerDomRef} - style={{ - display: 'flex', - justifyContent: 'flex-end', - }} + className="flex justify-end" />As per coding guidelines: "Always use Tailwind classes for styling HTML elements; avoid using CSS or tags."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/DrawerForm/index.tsx` around lines 278 - 283, The footer div in DrawerForm (look for the <div> with inline style={{ display: 'flex', justifyContent: 'flex-end' }} near the footer/render section of DrawerForm/index.tsx) uses inline styles; replace these with Tailwind utility classes (e.g., className="flex justify-end" or other appropriate Tailwind spacing/alignment utilities) on that element and remove the style prop so styling follows the project's Tailwind convention.
156-161:triggerDom中的 toggle 建议使用函数式更新
setOpen(!open)在useMemo的闭包中捕获了open，这使得open必须作为依赖项（line 161）。每次open状态变化都会重新创建triggerDom。使用函数式更新可以去掉该依赖，并避免快速连续点击场景下的过期值问题。♻️ 建议重构
onClick: async (e: any) => { - setOpen(!open); + setOpen((prev) => !prev); trigger.props?.onClick?.(e); }, - }, [setOpen, trigger, open]); + }, [setOpen, trigger]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/DrawerForm/index.tsx` around lines 156 - 161, The toggle in triggerDom uses setOpen(!open) inside the useMemo closure which captures stale open and forces open to be a dependency; change this to the functional updater form setOpen(prev => !prev) inside the onClick handler so you can remove open from the useMemo dependency and avoid race/stale-state on rapid clicks; keep calling trigger.props?.onClick?.(e) after updating state and ensure you update the dependency array to [setOpen, trigger] (or as appropriate) for the memoized triggerDom.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/form/layouts/DrawerForm/index.tsx`:
- Around line 134-138: The effect currently calls onOpenChange directly
(useEffect(() => { if (open && propsOpen) { onOpenChange?.(true); } },
[propsOpen, open])) causing a missing-deps stale-closure risk; replace the
direct call with the stable ref wrapper onOpenChangeCallback (e.g. call
onOpenChangeCallback.current or the wrapper function) inside the effect so you
don't need to add onOpenChange to the deps, keeping the dependency array as
[propsOpen, open] and using onOpenChangeCallback to invoke the handler safely.
- Around line 261-267: The afterOpenChange handler currently calls onOpenChange
directly causing duplicate notifications because setOpen already queues
onOpenChange; remove the direct call to onOpenChange in the afterOpenChange
block and only invoke drawerProps?.afterOpenChange?.(open) (and keep resetFields
logic) so consumers receive the animation-end event via
drawerProps.afterOpenChange instead of a second onOpenChange invocation; locate
the logic around afterOpenChange in DrawerForm (the afterOpenChange arrow
function) and delete the onOpenChange?.(open) call.
---
Nitpick comments:
In `@src/form/layouts/DrawerForm/index.tsx`:
- Around line 278-283: The footer div in DrawerForm (look for the <div> with
inline style={{ display: 'flex', justifyContent: 'flex-end' }} near the
footer/render section of DrawerForm/index.tsx) uses inline styles; replace these
with Tailwind utility classes (e.g., className="flex justify-end" or other
appropriate Tailwind spacing/alignment utilities) on that element and remove the
style prop so styling follows the project's Tailwind convention.
- Around line 156-161: The toggle in triggerDom uses setOpen(!open) inside the
useMemo closure which captures stale open and forces open to be a dependency;
change this to the functional updater form setOpen(prev => !prev) inside the
onClick handler so you can remove open from the useMemo dependency and avoid
race/stale-state on rapid clicks; keep calling trigger.props?.onClick?.(e) after
updating state and ensure you update the dependency array to [setOpen, trigger]
(or as appropriate) for the memoized triggerDom.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
site/components/modal-form.en-US.mdsite/components/modal-form.mdsrc/form/layouts/DrawerForm/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- site/components/modal-form.md
- site/components/modal-form.en-US.md
…ial errors - Updated the DrawerForm component to safely spread drawerProps, handling cases where it may be undefined.
There was a problem hiding this comment.
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/DrawerForm/index.tsx (2)
134-138:⚠️ Potential issue | 🟠 Major
useEffect中直接调用onOpenChange但未将其列入依赖数组，存在闭包陈旧问题
onOpenChange在 effect 内部被直接调用（第 136 行），但依赖数组仅有[propsOpen, open]，违反了react-hooks/exhaustive-deps规则。若onOpenChange在父组件中是内联函数，effect 将始终持有其旧引用。文件内已有通过
useRefFunction包装的稳定引用onOpenChangeCallback，应直接使用。🐛 建议修复
useEffect(() => { if (open && propsOpen) { - onOpenChange?.(true); + onOpenChangeCallback(true); } }, [propsOpen, open]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/DrawerForm/index.tsx` around lines 134 - 138, The effect currently calls onOpenChange directly but only lists [propsOpen, open] in its dependency array, causing a stale-closure bug; change the effect to call the stable wrapper onOpenChangeCallback (the useRefFunction-wrapped stable reference) instead of onOpenChange, and keep the dependency array as [propsOpen, open] so the callback used inside useEffect is always the stable onOpenChangeCallback reference; update the useEffect that checks if (open && propsOpen) to invoke onOpenChangeCallback?.(true).
134-138:⚠️ Potential issue | 🟡 Minor通过受控
propsOpen打开抽屉时，onOpenChange(true)会被重复触发两次当外部通过
propsOpen将抽屉设为打开时：
- 第 134–138 行的
useEffect（open && propsOpen均为true）同步调用一次onOpenChange(true)；- 抽屉动画结束后，第 266 行的
afterOpenChange再次调用onOpenChange?.(open)。消费方会连续收到两次内容相同的
true通知。可考虑移除useEffect中的onOpenChange调用，统一由afterOpenChange负责通知状态变更（afterOpenChange对打开和关闭均会触发，语义更完整）。💡 建议修复
useEffect(() => { - if (open && propsOpen) { - onOpenChange?.(true); - } - }, [propsOpen, open]); + // 已由 afterOpenChange 统一处理 onOpenChange 回调，此处无需重复调用 + }, []);若此
useEffect有其他意图（如 SSR 场景下的特殊处理），请在代码中添加注释说明。Also applies to: 261-267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/DrawerForm/index.tsx` around lines 134 - 138, 当前在 DrawerForm 的 useEffect 中（监听 propsOpen 和 open）会在受控 propsOpen 打开时调用 onOpenChange(true)，但 afterOpenChange（第 266 行）在动画结束后也会再次调用 onOpenChange，导致重复通知。请移除 useEffect 中对 onOpenChange 的调用（在 useEffect 里引用的标识为 useEffect, propsOpen, open, onOpenChange），统一由 afterOpenChange 来一次性通知打开/关闭；如果该 useEffect 有 SSR 或其它特殊意图，请在代码中保留并添加明确注释说明其必要性。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/form/layouts/DrawerForm/index.tsx`:
- Around line 134-138: The effect currently calls onOpenChange directly but only
lists [propsOpen, open] in its dependency array, causing a stale-closure bug;
change the effect to call the stable wrapper onOpenChangeCallback (the
useRefFunction-wrapped stable reference) instead of onOpenChange, and keep the
dependency array as [propsOpen, open] so the callback used inside useEffect is
always the stable onOpenChangeCallback reference; update the useEffect that
checks if (open && propsOpen) to invoke onOpenChangeCallback?.(true).
- Around line 134-138: 当前在 DrawerForm 的 useEffect 中（监听 propsOpen 和 open）会在受控
propsOpen 打开时调用 onOpenChange(true)，但 afterOpenChange（第 266 行）在动画结束后也会再次调用
onOpenChange，导致重复通知。请移除 useEffect 中对 onOpenChange 的调用（在 useEffect 里引用的标识为
useEffect, propsOpen, open, onOpenChange），统一由 afterOpenChange 来一次性通知打开/关闭；如果该
useEffect 有 SSR 或其它特殊意图，请在代码中保留并添加明确注释说明其必要性。
|
调整大小可以用ant6的，但是属性移除了旧系统没办法平滑升级 |
项目里面 peerDependencies 和官网已经指定了需要 antd > 6. 同时 pro2 -> pro3 已经有许多破坏性变更了, 我觉得咱们直接从 antd6 开始 把 Breaking Change 写到 Changelog 里面,不需要保留兼容性代码. |
|
title , width(size) 这两个放出来吧 |
…and documentation - Set the title and width properties for DrawerForm in multiple demo files to enhance consistency. - Updated documentation to reflect changes in DrawerForm properties, including the addition of title and width attributes. - Ensured clarity and accuracy in property descriptions across English and Chinese documentation.
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/DrawerForm/index.tsx (2)
142-146:⚠️ Potential issue | 🟡 MinoruseEffect 依赖数组缺少
onOpenChange
onOpenChange在 effect 内部被调用，但未包含在依赖数组中。这可能导致闭包捕获过期的onOpenChange引用，从而引发难以追踪的 bug。🔧 建议修复
useEffect(() => { if (open && propsOpen) { onOpenChange?.(true); } - }, [propsOpen, open]); + }, [propsOpen, open, onOpenChange]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/DrawerForm/index.tsx` around lines 142 - 146, The useEffect in DrawerForm (the effect that checks if (open && propsOpen) then calls onOpenChange?.(true)) is missing onOpenChange from its dependency array; update the effect to include onOpenChange in the dependencies (or memoize onOpenChange with useCallback and then include that) so the effect always uses the current callback reference—i.e., ensure useEffect([...]) contains propsOpen, open, and onOpenChange (or the memoized callback) to avoid stale closures.
21-59:⚠️ Potential issue | 🟠 Major移除
resize属性导致测试失败
resize属性已从DrawerFormProps类型定义中移除，但tests/form/drawerForm.test.tsx中的drawerForm support onResize测试仍然使用此属性，并尝试查询预期的.ant-pro-form-drawer-sidebar-dragger元素（第 634、642、660 行）。该测试在当前代码下将失败，因为拖拽元素不会被渲染。需要更新或删除这个测试用例。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/DrawerForm/index.tsx` around lines 21 - 59, The type DrawerFormProps no longer includes the resize prop, so update the failing test in tests/form/drawerForm.test.tsx (the "drawerForm support onResize" case) to stop using resize and stop asserting on the presence of the .ant-pro-form-drawer-sidebar-dragger element; specifically, remove the resize prop usage when rendering DrawerForm and delete or replace the queries/assertions that check for '.ant-pro-form-drawer-sidebar-dragger'. If you want to keep a behavior check, change the test to assert the current supported behavior (e.g., drawerProps/width or absence of a dragger) rather than expecting onResize to render a draggable sidebar.
🧹 Nitpick comments (2)
src/form/layouts/DrawerForm/index.tsx (1)
57-58: 属性命名与 antd API 语义不一致
width属性被映射到 Drawer 的size属性。在 antd v6 中，size接受'default' | 'large' | number类型。建议在文档或 JSDoc 注释中明确说明这一映射关系，避免使用者混淆。Also applies to: 267-269
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/DrawerForm/index.tsx` around lines 57 - 58, The prop named width in the DrawerForm typings is misleading because it actually maps to antd DrawerProps['size']; update the JSDoc and public API docs to clarify this mapping (e.g., "width?: DrawerProps['size'] — maps to Drawer size ('default' | 'large' | number) in antd v6") and do the same for the other occurrence related to the DrawerForm props/typing; locate the width declaration in the DrawerForm props/interface and the corresponding usage in the DrawerForm component and add the clear annotation so consumers know it accepts antd size values rather than a raw pixel width.demos/form/ModalForm/drawer-form.tsx (1)
1-10: 导入顺序不符合编码规范根据编码规范，导入顺序应为：React → 依赖 → antd → pro-components → types。当前
useState的导入位置在 pro-components 和 antd 之后。♻️ 建议修复
+import { useState } from 'react'; import { PlusOutlined } from '@ant-design/icons'; import { DrawerForm, ProForm, ProFormDateRangePicker, ProFormSelect, ProFormText, } from '@ant-design/pro-components'; import { Button, Form, message } from 'antd'; -import { useState } from 'react';根据编码规范：
Follow import order: React → dependencies → antd → pro-components → types🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demos/form/ModalForm/drawer-form.tsx` around lines 1 - 10, The import order in drawer-form.tsx is incorrect; add an explicit React import (import React from 'react') at the top and reorder imports to follow the convention: React first, then external dependencies (e.g., PlusOutlined), then antd (Button, Form, message), then pro-components (DrawerForm, ProForm, ProFormDateRangePicker, ProFormSelect, ProFormText), and finally hooks/types (move useState to after those), so locate symbols like useState, PlusOutlined, DrawerForm/ProForm and adjust their import lines accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@site/components/modal-form.en-US.md`:
- Line 66: 在 site/components/modal-form.en-US.md 的参数表中，修正 open
参数那一行的默认值列为空的问题：找到表格行包含 "open" 列的条目并将其默认值单元格由空白改为
"-"，以与其他无默认值的参数保持一致（参考表格中其他行的默认值格式）；仅修改该单元格内容，不改变其他列或表格结构。
In `@site/components/modal-form.md`:
- Line 71: The antd documentation link in the modal-form.md table for
drawerProps is missing a slash between the domain and path; update the link
referenced in the drawerProps row so it uses the correct URL format (e.g.,
https://ant.design/components/drawer#api) to point to the Drawer API
documentation and ensure the table cell still describes drawerProps and type
`Omit<DrawerProps, 'open'>` unchanged.
---
Outside diff comments:
In `@src/form/layouts/DrawerForm/index.tsx`:
- Around line 142-146: The useEffect in DrawerForm (the effect that checks if
(open && propsOpen) then calls onOpenChange?.(true)) is missing onOpenChange
from its dependency array; update the effect to include onOpenChange in the
dependencies (or memoize onOpenChange with useCallback and then include that) so
the effect always uses the current callback reference—i.e., ensure
useEffect([...]) contains propsOpen, open, and onOpenChange (or the memoized
callback) to avoid stale closures.
- Around line 21-59: The type DrawerFormProps no longer includes the resize
prop, so update the failing test in tests/form/drawerForm.test.tsx (the
"drawerForm support onResize" case) to stop using resize and stop asserting on
the presence of the .ant-pro-form-drawer-sidebar-dragger element; specifically,
remove the resize prop usage when rendering DrawerForm and delete or replace the
queries/assertions that check for '.ant-pro-form-drawer-sidebar-dragger'. If you
want to keep a behavior check, change the test to assert the current supported
behavior (e.g., drawerProps/width or absence of a dragger) rather than expecting
onResize to render a draggable sidebar.
---
Nitpick comments:
In `@demos/form/ModalForm/drawer-form.tsx`:
- Around line 1-10: The import order in drawer-form.tsx is incorrect; add an
explicit React import (import React from 'react') at the top and reorder imports
to follow the convention: React first, then external dependencies (e.g.,
PlusOutlined), then antd (Button, Form, message), then pro-components
(DrawerForm, ProForm, ProFormDateRangePicker, ProFormSelect, ProFormText), and
finally hooks/types (move useState to after those), so locate symbols like
useState, PlusOutlined, DrawerForm/ProForm and adjust their import lines
accordingly.
In `@src/form/layouts/DrawerForm/index.tsx`:
- Around line 57-58: The prop named width in the DrawerForm typings is
misleading because it actually maps to antd DrawerProps['size']; update the
JSDoc and public API docs to clarify this mapping (e.g., "width?:
DrawerProps['size'] — maps to Drawer size ('default' | 'large' | number) in antd
v6") and do the same for the other occurrence related to the DrawerForm
props/typing; locate the width declaration in the DrawerForm props/interface and
the corresponding usage in the DrawerForm component and add the clear annotation
so consumers know it accepts antd size values rather than a raw pixel width.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d7a354fa-e6d1-4afd-9913-e8c95623d2df
📒 Files selected for processing (6)
demos/form/ModalForm/drawer-form-nested.tsxdemos/form/ModalForm/drawer-form.tsxdemos/form/ModalForm/modal-form-request-destroy-debug.tsxsite/components/modal-form.en-US.mdsite/components/modal-form.mdsrc/form/layouts/DrawerForm/index.tsx
✅ Files skipped from review due to trivial changes (1)
- demos/form/ModalForm/modal-form-request-destroy-debug.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- demos/form/ModalForm/drawer-form-nested.tsx
…tency - Changed the title and width properties in the DrawerForm demo files to ensure uniformity across examples. - Removed redundant title definitions to streamline the code. - Added a note regarding the removal of the deprecated 'resize' property in the DrawerForm component.
- Updated the title property in the ModalForm demo to ensure consistency with other examples. - Removed the redundant title definition from drawerProps to streamline the code.
…'open' property to false - Updated the documentation for both ModalForm and DrawerForm to specify that the default value for the 'open' property is now false. - Ensured consistency in property descriptions across English and Chinese versions.
… drawerProps types - Updated the documentation for ModalForm and DrawerForm to specify the type of modalProps and drawerProps as `Omit<ModalProps, 'open'>` and `Omit<DrawerProps, 'open'>` respectively. - Ensured consistency in property descriptions across English and Chinese versions.
…ode and improve clarity.
done |
3 participants
title,width(size)和destroyOnHidden属性都属于drawerProps里面的属性, 所以去掉专门的声明,统一集成到drawerProps里面.antd6支可持调整大小了,这里移除 pro 的代码.
这几处更改都是
Breaking ChangeSummary by CodeRabbit
发布说明
Refactor
Documentation