-
-
Notifications
You must be signed in to change notification settings - Fork 103
feat: add resizable drawer feature #528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough本次变更为 Drawer 增加可拖拽调整尺寸(resizable)能力,新增 useDrag Hook、拖拽句柄与样式;扩展 Props(defaultWidth、defaultHeight、resizable)并导出 Placement;新增示例与文档、动效扩展和相关单元测试;同时重构样式为基于 prefix 的命名。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 用户
participant Dragger as Resizable Dragger
participant useDrag as useDrag Hook
participant Popup as DrawerPopup
participant App as 调用方(resizable 回调)
User->>Dragger: mousedown
Dragger->>useDrag: onMouseDown
useDrag->>Popup: onResizeStart(size)
Popup->>App: resizable.onResizeStart()
loop 拖拽中
User->>useDrag: mousemove
useDrag->>Popup: onResize(newSize)
Popup->>App: resizable.onResize(newSize)
Popup->>Popup: 更新 currentSize 与 样式
end
User->>useDrag: mouseup
useDrag->>Popup: onResizeEnd(finalSize)
Popup->>App: resizable.onResizeEnd(finalSize)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @QdabuliuQ, 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 introduces a significant new feature to the Drawer component, allowing it to be resizable. Users can now dynamically adjust the width or height of the drawer by dragging its edges, providing greater flexibility in UI layouts. This enhancement includes new properties to enable the feature and provides callbacks for managing the resizing process, along with dedicated styling and a new internal component to handle the drag interactions. The changes are fully documented and include a new example for easy adoption.
Highlights
- Resizable Drawer Feature: The Drawer component now supports a
resizable
prop, which when set totrue
, allows users to change the drawer's size by dragging its edges. This enhances user interaction and flexibility. - New Resize Event Callbacks: Three new callback functions,
onResize
,onResizeStart
, andonResizeEnd
, have been added to the Drawer component. These provide developers with hooks to execute custom logic during different stages of the resizing process, such as updating state or performing validations. - Introduction of ResizableLine Component: A dedicated
ResizableLine
component has been introduced to encapsulate the logic for handling mouse events (mousedown, mousemove, mouseup) and calculating the new size of the drawer during a drag operation. This modular approach keeps the core Drawer component cleaner. - Styling for Resizable Handles: New CSS rules have been added to style the resizable line, including visual feedback for hover and dragging states. This ensures a clear and intuitive user experience when interacting with the resize handles.
- Documentation and Example Updates: Comprehensive updates to the documentation and a new interactive example have been provided. This allows other developers to quickly understand how to implement and utilize the new resizable drawer feature in their applications.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a resizable drawer feature, which is a great addition. The implementation is mostly solid, with a new ResizableLine
component handling the drag logic and updates to the Drawer
and DrawerPopup
components to support it. I've found a few issues: an incorrect CSS value for overflow
, a bug in the animation styles for top/bottom placements, and an API inconsistency in the new resize event handlers. The documentation and examples have been updated accordingly. My review includes suggestions to fix these issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
src/inter.ts (1)
1-13
: 命名不一致:接口使用 section,文档与样式使用 content,建议新增别名并标注弃用
DrawerClassNames
/DrawerStyles
中是section
,但 README 与样式体系(如-content
)更贴近 “content”。为避免使用者困惑与类型不匹配,建议在类型上新增content
,并将section
标注为 deprecated,保证向后兼容。建议变更:
export interface DrawerClassNames { mask?: string; wrapper?: string; - section?: string; + /** @deprecated 请使用 `content` */ + section?: string; + content?: string; resizableLine?: string; } export interface DrawerStyles { - mask?: React.CSSProperties; - wrapper?: React.CSSProperties; - section?: React.CSSProperties; + mask?: React.CSSProperties; + wrapper?: React.CSSProperties; + /** @deprecated 请使用 `content` */ + section?: React.CSSProperties; + content?: React.CSSProperties; resizableLine?: React.CSSProperties; }(如代码中有读取
section
的实现,建议同时支持content ?? section
。)
🧹 Nitpick comments (18)
tsconfig.json (1)
16-16
: 明确排除 .dumi 目录,避免被通配重新包含当前 include 使用 "/*.ts"、"/*.tsx" 会再次把 .dumi 目录纳入编译范围。若期望排除 .dumi(以及构建产物),建议补充 exclude,提高 ts 编译性能并避免噪音类型报错。
建议变更:
- "include": [".dumirc.ts", "**/*.ts", "**/*.tsx"] + "include": [".dumirc.ts", "**/*.ts", "**/*.tsx"], + "exclude": [".dumi", "node_modules", "dist"]src/inter.ts (1)
5-5
: 新增 resizableLine 类型后请同步 README 的 classNames/styles 说明接口已增加
resizableLine?: string
与resizableLine?: React.CSSProperties
,但 README 的 API 表格中classNames
与styles
的对象结构未包含该键,文档与实现不一致,建议同步更新文档。我可以附带一段 README 的表格更新 diff,是否需要我起一个跟进 PR?
Also applies to: 12-12
README.md (3)
47-48
: 补充 resizableLine 的类型说明,保持与代码一致
classNames
/styles
的类型签名未包含resizableLine
,与src/inter.ts
定义不一致,建议补上:建议变更(文档示意):
-| classNames | { mask?: string; content?: string; wrapper?: string; } | - | pass className to target area | -| styles | { mask?: CSSProperties; content?: CSSProperties; wrapper?: CSSProperties; } | - | pass style to target area | +| classNames | { mask?: string; content?: string; wrapper?: string; resizableLine?: string; } | - | pass className to target area | +| styles | { mask?: CSSProperties; content?: CSSProperties; wrapper?: CSSProperties; resizableLine?: CSSProperties; } | - | pass style to target area |
55-55
: 文档类型更精确:placement 可标注为联合类型代码侧已导出
Placement
类型(如 PR 摘要所述),文档仍是string
。建议文档同步为更精确的联合类型('left' | 'right' | 'top' | 'bottom'
或引用Placement
),提升消费者可读性。
59-59
: 拼写错误:React.createElement
React.creactElement
应为React.createElement
。建议变更:
- | getContainer | string \| func \| HTMLElement | `body` | Return the mount node for Drawer. if is `null` use React.creactElement | + | getContainer | string \| func \| HTMLElement | `body` | Return the mount node for Drawer. If it is `null`, use React.createElement |assets/index.less (3)
62-107
: 提升可用性:防止拖拽时选中文本与多指缩放干扰建议为可拖拽线增加
user-select: none
与touch-action: none
,在移动端和触摸板上会更稳。建议变更:
&-resizable-line { position: absolute; z-index: 2; pointer-events: auto; + user-select: none; + touch-action: none;
68-74
: 避免硬编码品牌色,改为可配置变量
#1890ff
是 Ant Design v4 的主色,rc-drawer 作为独立库建议使用可配置变量,避免与主题脱钩。可新增局部变量,默认值保留原色以兼容。建议变更:
+@resizer-active-color: #1890ff; // 可按需覆写 ... &-resizable-line { @@ - &:hover { - background-color: #1890ff !important; - } + &:hover { + background-color: @resizer-active-color !important; + } @@ - &-dragging { - background-color: #1890ff !important; - } + &-dragging { + background-color: @resizer-active-color !important; + }
110-117
: z-index 定义重复且数值不一致,建议统一本文件前部
.@{prefixCls}
已设置z-index: @zIndex
(当前为 1050),此处再次设置为 9999。两处相同选择器的重复定义会以较晚出现者为准,建议统一到变量,避免维护困惑与潜在层级问题。建议其一(更易维护):改为使用变量
- z-index: 9999; + z-index: @zIndex;建议其二(若确需抬高层级):统一在顶部提升变量值
-@zIndex: 1050; +@zIndex: 9999;请确认期望行为,并选择其一。
docs/demo/resizable.md (1)
2-2
: 标题大小写与导航不一致,建议统一Front matter 中的 title 为小写“resizable”,而 nav.title 为“Resizable”。为保持文档一致性与站点导航显示统一,建议首字母大写。
-title: resizable +title: Resizabletests/index.spec.tsx (1)
479-516
: 增强用例稳定性并覆盖回调行为断言
- 使用 getContainer={false},减少与 Portal/文档滚动相关的偶发性影响。
- 将拖拽序列包裹在 act 中,避免 React 状态更新告警。
- 覆盖 onResize/onResizeStart/onResizeEnd 回调是否触发及参数正确性的断言,提升行为验证完整性。
it('should support resizable', () => { - const { unmount } = render( - <Drawer resizable open placement="left" width={200} />, - ); + const onResizeStart = jest.fn(); + const onResize = jest.fn(); + const onResizeEnd = jest.fn(); + const { unmount } = render( + <Drawer + resizable + open + placement="left" + width={200} + getContainer={false} + onResizeStart={onResizeStart} + onResize={onResize} + onResizeEnd={onResizeEnd} + />, + ); // Mock getBoundingClientRect for the content wrapper to simulate real DOM dimensions const contentWrapper = document.querySelector( '.rc-drawer-content-wrapper', ) as HTMLElement; const mockGetBoundingClientRect = jest.fn( () => ({ width: 200, height: 400, top: 0, left: 0, bottom: 400, right: 200, x: 0, y: 0, toJSON: () => ({}), }) as DOMRect, ); contentWrapper.getBoundingClientRect = mockGetBoundingClientRect; const resizableLine = document.querySelector('.rc-drawer-resizable-line'); expect(resizableLine).toBeTruthy(); - // Simulate drag from 200px to 100px (should reduce width by 100px) - fireEvent.mouseDown(resizableLine, { clientX: 200 }); - fireEvent.mouseMove(document, { clientX: 100, clientY: 0 }); - fireEvent.mouseUp(document, { clientX: 100, clientY: 0 }); + // Simulate drag from 200px to 100px (should reduce width by 100px) + act(() => { + fireEvent.mouseDown(resizableLine!, { clientX: 200 }); + fireEvent.mouseMove(document, { clientX: 100, clientY: 0 }); + fireEvent.mouseUp(document, { clientX: 100, clientY: 0 }); + }); expect(document.querySelector('.rc-drawer-content-wrapper')).toHaveStyle({ width: '100px', }); + expect(onResizeStart).toHaveBeenCalledTimes(1); + expect(onResize).toHaveBeenCalled(); + // 最后一笔应为 100 宽度(或高度,取决于 placement) + expect(onResize).toHaveBeenLastCalledWith(100); + expect(onResizeEnd).toHaveBeenCalledTimes(1); unmount(); });docs/examples/resizable.tsx (3)
8-8
: 建议优化类型导入路径从'@/index'导入类型可能导致构建路径解析问题。建议使用相对路径或明确的包名导入。
-import type { Placement } from '@/index'; +import type { Placement } from 'rc-drawer';
42-42
: key属性可能导致不必要的重新挂载使用placement作为key会在每次placement变化时强制重新挂载Drawer组件,这可能不是期望的行为。考虑移除key属性或使用更稳定的标识符。
-key={placement}
47-52
: 改进示例文案的用户体验文案可以更清晰地说明调整大小的操作方式。
-<p> - You can drag the drawer edge to resize (only works when - "Resizable" is checked). -</p> +<p> + 拖拽抽屉边缘可调整大小(仅在勾选"Resizable"时有效)。 +</p>src/ResizableLine.tsx (4)
24-24
: 重新考虑minSize默认值默认minSize为100可能过大,特别是对于移动端或小屏幕设备。建议设置更小的默认值或根据方向动态调整。
-minSize = 100, +minSize = 48,
82-84
: maxSize验证逻辑可以简化当前的maxSize检查有些冗余,可以简化条件判断。
-// Only apply maxSize if it's a valid positive number -if (maxSize !== undefined && maxSize > 0 && newSize > maxSize) { +if (maxSize && newSize > maxSize) {
100-112
: handleMouseUp中的重复代码与handleMouseDown中的父元素获取逻辑重复,建议提取为共用函数。
+const getParentSize = React.useCallback(() => { + const parentElement = lineRef.current?.parentElement; + if (!parentElement) return 0; + const rect = parentElement.getBoundingClientRect(); + return isHorizontal ? rect.width : rect.height; +}, [isHorizontal]); const handleMouseUp = React.useCallback(() => { if (isDragging) { onDraggingChange?.(false); - // Get the final size after resize - const parentElement = lineRef.current?.parentElement; - if (parentElement) { - const rect = parentElement.getBoundingClientRect(); - const finalSize = isHorizontal ? rect.width : rect.height; + const finalSize = getParentSize(); + if (finalSize > 0) { onResizeEnd?.(finalSize); - } } -}, [isDragging, onResizeEnd, isHorizontal, onDraggingChange]); +}, [isDragging, onResizeEnd, onDraggingChange, getParentSize]);
135-140
: resizeLineStyle可以优化style对象的构建可以更高效,避免每次渲染时创建新对象。
-const resizeLineStyle: React.CSSProperties = { - position: 'absolute', - zIndex: 2, - cursor: isHorizontal ? 'col-resize' : 'row-resize', - ...style, -}; +const resizeLineStyle = React.useMemo<React.CSSProperties>(() => ({ + position: 'absolute', + zIndex: 2, + cursor: isHorizontal ? 'col-resize' : 'row-resize', + ...style, +}), [isHorizontal, style]);src/DrawerPopup.tsx (1)
297-301
: 状态初始化可以优化建议为currentSize提供更明确的初始值,而不是undefined。
-const [currentSize, setCurrentSize] = React.useState<number>(); +const [currentSize, setCurrentSize] = React.useState<number | undefined>(undefined);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
README.md
(1 hunks)assets/index.less
(2 hunks)docs/demo/resizable.md
(1 hunks)docs/examples/assets/motion.less
(1 hunks)docs/examples/getContainer-false.tsx
(1 hunks)docs/examples/resizable.tsx
(1 hunks)src/Drawer.tsx
(3 hunks)src/DrawerPopup.tsx
(5 hunks)src/ResizableLine.tsx
(1 hunks)src/index.ts
(1 hunks)src/inter.ts
(1 hunks)tests/index.spec.tsx
(1 hunks)tsconfig.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
docs/examples/resizable.tsx (4)
docs/examples/getContainer-false.tsx (1)
open
(21-114)src/index.ts (1)
Placement
(5-5)src/Drawer.tsx (1)
Placement
(15-15)src/DrawerPopup.tsx (1)
Placement
(26-26)
src/ResizableLine.tsx (3)
src/index.ts (1)
Placement
(5-5)src/Drawer.tsx (1)
Placement
(15-15)src/DrawerPopup.tsx (1)
Placement
(26-26)
🔇 Additional comments (11)
README.md (1)
67-76
: LGTM:新增 resizable 与 resize 回调文档与实现一致新文档项与 PR 目标一致,描述清晰。
assets/index.less (1)
33-35
: 确认移除 overflow: hidden 的影响
content-wrapper
注释掉overflow: hidden
能让拖拽线溢出 3px 可点,但同时也可能导致内容溢出时出现视觉残留。确认是否在拖拽中通过行内样式或状态条件控制overflow
,以避免滚动条/阴影露出。docs/examples/getContainer-false.tsx (1)
55-56
: 示例新增 resizable 用法,LGTM在 getContainer={false} 的场景展示可拖拽调整尺寸是合理的,变更清晰可读。
src/index.ts (1)
3-5
: 导出 Placement 类型,LGTM类型面向外部暴露更完整,便于下游消费与类型提示。
src/Drawer.tsx (2)
56-59
: 解构新增回调以便传递,LGTMonResize/onResizeStart/onResizeEnd 在此解构后下传,符合预期的数据流。
120-123
: 回调透传到 DrawerPopup,LGTM事件处理集合中纳入 resize 系列回调,保持 API 一致性与集中传递,合理。
src/DrawerPopup.tsx (5)
80-84
: LGTM!可调整尺寸相关的API设计合理新增的resizable相关属性和回调函数API设计清晰,类型定义完整。
397-397
: 动画类名条件逻辑很好在拖拽时禁用动画类名是明智的设计,避免拖拽过程中的动画干扰。
406-420
: ResizableLine组件集成正确ResizableLine的属性传递和事件处理集成得很好,maxSize和isDragging状态管理恰当。
393-393
: wrapperRef 使用验证通过已在
src/DrawerPopup.tsx
中确认wrapperRef
的定义与使用场景一致:
- 定义为
React.useRef<HTMLDivElement>(null)
- 在
calculateMaxSize
回调及 JSX 中通过ref={wrapperRef}
正确绑定至 DOM 元素未发现引用不一致或遗漏,使用正常。
304-313
: calculateMaxSize函数需要添加边界检查函数缺少对rect的null检查,可能导致运行时错误。
const calculateMaxSize = React.useCallback(() => { if (wrapperRef.current) { const rect = wrapperRef.current.parentElement?.getBoundingClientRect(); - const newMaxSize = + if (!rect) return; + const newMaxSize = placement === 'left' || placement === 'right' - ? (rect?.width ?? 0) - : (rect?.height ?? 0); + ? rect.width + : rect.height; setMaxSize(newMaxSize); } }, [placement]);Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/DrawerPopup.tsx (1)
358-362
: overflow 设置为 'none' 是无效值,应按场景使用 'visible' 或 'hidden'这里将
style.overflow
设为'none'
,CSS 不支持该值。若要让拖拽手柄(有 -3px 偏移)在 wrapper 外可见,应在 resizable 时使用'visible'
;而非 resizable 场景建议维持'hidden'
(与历史行为一致,避免 wrapper 出现滚动条)。- if (resizable) { - style.overflow = 'none'; - } else { - style.overflow = 'auto'; - } + if (resizable) { + style.overflow = 'visible'; + } else { + style.overflow = 'hidden'; + }
🧹 Nitpick comments (9)
assets/index.less (1)
62-107
: 拖拽手柄建议增强可用性与可访问性(触控/选择/命中区域)当前手柄宽高为 6px,hover/dragging 才着色,缺少移动端和防选中文本的样式,实际交互可能较难命中并与内容选择冲突。
建议增强:
- 添加
touch-action: none; user-select: none;
避免移动端滚动/文本选择干扰。- 默认
background-color: transparent
,hover/dragging 再显色。- 可考虑将热区扩大到 8–10px(按需)。
示例增补(可合入
&-resizable-dragger
基类内):&-resizable-dragger { position: absolute; z-index: 2; pointer-events: auto; + background-color: transparent; + touch-action: none; + user-select: none;src/inter.ts (1)
1-13
: 新增 dragger 自定义入口命名与定位合理,建议补充注释以稳定 API 语义新增
dragger?: string
与dragger?: React.CSSProperties
能覆盖拖拽线的 className 与 style,命名与现有讨论一致。为便于下游理解与避免误用(与 handle/section 概念混淆),建议在接口上补充 JSDoc 注释,说明其仅作用于“可调整尺寸的分隔线/热区”。示例注释(非必须):
/** resizable 模式下的拖拽手柄(分隔线)容器样式/类名 */ dragger?: string; dragger?: React.CSSProperties;docs/examples/resizable.tsx (3)
14-19
: 移除多余类型断言,直接类型化 buttons 数组
placement
的状态已是Placement
,此处对每一项使用as Placement
属于冗余断言。直接在数组上声明类型更清晰安全。- const buttons = [ - { placement: 'left' as Placement, label: 'Left Drawer' }, - { placement: 'right' as Placement, label: 'Right Drawer' }, - { placement: 'top' as Placement, label: 'Top Drawer' }, - { placement: 'bottom' as Placement, label: 'Bottom Drawer' }, - ]; + const buttons: Array<{ placement: Placement; label: string }> = [ + { placement: 'left', label: 'Left Drawer' }, + { placement: 'right', label: 'Right Drawer' }, + { placement: 'top', label: 'Top Drawer' }, + { placement: 'bottom', label: 'Bottom Drawer' }, + ];
42-42
: 移除冗余断言
placement
已是Placement
类型,无需再次断言。- placement={placement as Placement} + placement={placement}
30-36
: 给按钮添加 type="button" 以避免在 Form 环境误触发提交示例中按钮若被嵌入到 form 内,默认 type="submit" 可能触发表单提交。加上
type="button"
更稳妥。- <button + <button + type="button" key={placement} onClick={() => openDrawer(placement)} style={{ padding: '8px 16px' }} >src/DrawerPopup.tsx (3)
347-366
: 在拖拽时禁用过渡已处理,但建议对 width/height 的单位与数值混用做防御当前在
currentSize
存在时用数值覆盖width/height
,与可能的字符串(如'20vw'
)互斥,这符合预期。但在开启拖拽后从一个字符串单位切换为纯数值时,视觉上会突然以 px 为单位进行变化,这是可接受的设计权衡。若要更平滑,可以在开始拖拽时解析并固定为像素基准(可选)。
298-313
: maxSize 计算仅依赖 parentElement 尺寸,嵌套/inline 场景需再确认
wrapperRef.current.parentElement?.getBoundingClientRect()
在getContainer={false}
或复杂嵌套下可能不是你期望的可用空间(例如父级有 padding/滚动容器)。建议后续结合实际容器(portal 容器或视口)确认,或暴露maxSize
/getMaxSize
参数以供高级用法覆盖。
416-417
: 拖拽手柄无语义/无可访问性支持,建议增加角色与可见文本隐藏说明(可选)可考虑为手柄添加
role="separator"
、aria-orientation
,并通过aria-label
或title
提示“拖拽以调整尺寸”。键盘可达的可调整大小(如使用左右/上下箭头)属于更高阶改进,可择机迭代。src/hooks/useDrag.ts (1)
1-26
: 建议新增 Pointer 事件支持以兼容触屏设备(可选)当前仅监听 mouse 事件。实际在移动端/触摸屏上会缺乏交互。可以使用 Pointer 事件统一处理(
pointerdown/move/up
),或补充touch*
事件。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
assets/index.less
(2 hunks)docs/examples/resizable.tsx
(1 hunks)src/DrawerPopup.tsx
(5 hunks)src/hooks/useDrag.ts
(1 hunks)src/inter.ts
(1 hunks)tests/index.spec.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/index.spec.tsx
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/hooks/useDrag.ts (1)
src/DrawerPopup.tsx (1)
Placement
(26-26)
src/DrawerPopup.tsx (1)
src/hooks/useDrag.ts (1)
useDrag
(28-161)
docs/examples/resizable.tsx (4)
docs/examples/getContainer-false.tsx (1)
open
(21-114)src/DrawerPopup.tsx (1)
Placement
(26-26)src/index.ts (1)
Placement
(5-5)src/Drawer.tsx (1)
Placement
(15-15)
🔇 Additional comments (5)
assets/index.less (1)
30-35
: content-wrapper 注释掉 overflow: hidden 的影响需要确认这里将
overflow: hidden;
注释掉(Line 33)。结合下方第二块样式中顶层容器仍是overflow: hidden;
,以及组件运行时又通过内联样式动态设置overflow
,可能出现拖拽手柄被裁剪或滚动条出现的不一致行为。建议:明确策略(例如“仅在 resizable 时让 wrapper overflow: visible,其余场景 hidden”),避免多个层级对 overflow 的相互覆盖造成视觉/命中异常。配合组件代码修正(参见
DrawerPopup.tsx
对 overflow 的设置问题)。docs/examples/resizable.tsx (1)
8-8
: 确认路径别名是否在示例构建环境中可用
import type { Placement } from '@/index';
依赖@
路径别名。若 docs 示例单独构建或运行,需要确保 tsconfig/webpack 等已声明paths
/alias。如需验证,请检查根目录下 tsconfig/构建配置是否有
@/*
的 alias 映射到src/*
。src/hooks/useDrag.ts (3)
28-41
: 适配新签名,内部统一从 ref 读取容器[ suggest_essential_refactor ]
export default function useDrag(options: UseDragOptions): UseDragReturn { const { prefixCls = 'resizable', direction, className, style, minSize = 100, maxSize, disabled = false, - container, + containerRef, onResize, onResizeEnd, onResizeStart, } = options;
64-73
: mousedown 时从 ref 读取容器并回调 startSize[ suggest_essential_refactor ]
- // Get the current size of the container - if (container) { - const rect = container.getBoundingClientRect(); + // Get the current size of the container + const container = containerRef?.current; + if (container) { + const rect = container.getBoundingClientRect(); const currentSize = isHorizontal ? rect.width : rect.height; setStartSize(currentSize); onResizeStart?.(currentSize); } }, - [disabled, isHorizontal, container, onResizeStart], + [disabled, isHorizontal, containerRef, onResizeStart],
113-125
: mouseup 时同样从 ref 读取容器最终尺寸[ suggest_essential_refactor ]
- const handleMouseUp = React.useCallback(() => { + const handleMouseUp = React.useCallback(() => { if (isDragging && !disabled) { setIsDragging(false); // Get the final size after resize - if (container) { - const rect = container.getBoundingClientRect(); + const container = containerRef?.current; + if (container) { + const rect = container.getBoundingClientRect(); const finalSize = isHorizontal ? rect.width : rect.height; onResizeEnd?.(finalSize); } } - }, [isDragging, disabled, container, onResizeEnd, isHorizontal]); + }, [isDragging, disabled, containerRef, onResizeEnd, isHorizontal]);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/DrawerPopup.tsx (2)
358-362
: overflow 样式设置逻辑可能需要调整当前实现中,
resizable
为true
时设置overflow: 'visible'
,这可能会导致内容溢出。建议保持overflow: 'hidden'
,因为内部内容区域已经有独立的滚动处理。if (resizable) { - style.overflow = 'visible'; + style.overflow = 'hidden'; } else { style.overflow = 'hidden'; }
333-345
: 容器引用传递问题会导致拖拽起始尺寸测量失败当前通过
container: wrapperRef.current
传递的是值而非引用,首次渲染时wrapperRef.current
为null
,这会导致useDrag
hook 内部无法正确获取容器元素,进而影响拖拽尺寸的计算。修改为传递 ref 引用:
const { dragElementProps, isDragging } = useDrag({ prefixCls: `${prefixCls}-resizable`, direction: placement, className: drawerClassNames?.dragger, style: styles?.dragger, minSize: 0, maxSize, disabled: !resizable, - container: wrapperRef.current, + containerRef: wrapperRef, onResize: handleResize, onResizeStart: handleResizeStart, onResizeEnd: handleResizeEnd, });同时需要更新
useDrag
hook 的接口定义,将container
参数改为containerRef: React.RefObject<HTMLElement>
。
🧹 Nitpick comments (3)
src/DrawerPopup.tsx (3)
302-312
: calculateMaxSize 可能在父容器未渲染时计算不准确
calculateMaxSize
方法依赖于父容器的尺寸,但在某些情况下(如初始渲染或父容器尺寸变化)可能获取不到准确的尺寸。建议添加边界检查和重试机制。const calculateMaxSize = React.useCallback(() => { if (wrapperRef.current) { const rect = wrapperRef.current.parentElement?.getBoundingClientRect(); + if (!rect || rect.width === 0 || rect.height === 0) { + // 延迟重试,等待容器渲染完成 + requestAnimationFrame(() => calculateMaxSize()); + return; + } const newMaxSize = placement === 'left' || placement === 'right' ? (rect?.width ?? 0) : (rect?.height ?? 0); setMaxSize(newMaxSize); } }, [placement]);
368-370
: 需要监听容器尺寸变化当前仅在初始化时计算
maxSize
,如果容器尺寸发生变化(如窗口大小调整),maxSize
不会更新。建议添加 ResizeObserver 监听容器尺寸变化。// Initialize maxSize calculation React.useEffect(() => { calculateMaxSize(); + + if (!wrapperRef.current?.parentElement) return; + + const resizeObserver = new ResizeObserver(() => { + calculateMaxSize(); + }); + + resizeObserver.observe(wrapperRef.current.parentElement); + + return () => { + resizeObserver.disconnect(); + }; }, [calculateMaxSize]);
416-416
: 拖拽手柄需要添加无障碍属性拖拽手柄缺少必要的无障碍属性,如
role
、aria-label
等,这会影响屏幕阅读器用户的使用体验。-{resizable && <div {...dragElementProps} />} +{resizable && ( + <div + {...dragElementProps} + role="separator" + aria-label="调整抽屉大小" + aria-orientation={placement === 'left' || placement === 'right' ? 'vertical' : 'horizontal'} + aria-valuemin={0} + aria-valuemax={maxSize} + aria-valuenow={currentSize} + tabIndex={0} + /> +)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/examples/assets/motion.less
(1 hunks)src/DrawerPopup.tsx
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/examples/assets/motion.less
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/DrawerPopup.tsx (1)
src/hooks/useDrag.ts (1)
useDrag
(28-161)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #528 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 5 6 +1
Lines 144 258 +114
Branches 53 97 +44
==========================================
+ Hits 144 258 +114 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
74-76
: onResize/Start/End 描述存在误导,建议更正措辞并明确返回参数含义
- onResize 的描述明显是从键盘事件复制而来,容易误解。
- 建议明确 size 的语义(px,且随 placement 决定是宽度或高度),并统一三者的触发时机说明。
可在 README 中对这三行做如下修改:
-| onResize | (size: number) => void | - | Trigger when mouse keyup on drawer panel +| onResize | (size: number) => void | - | Fires while resizing via dragging. size is current width/height in px depending on placement. -| onResizeStart | () => void | - | Trigger when resize starts +| onResizeStart | () => void | - | Fires when resizing starts (mouse/touch down on the dragger). -| onResizeEnd | () => void | - | Trigger when resize ends +| onResizeEnd | () => void | - | Fires when resizing ends (mouse/touch up). Use onResize for latest size.如实现里希望在结束时也拿到最终尺寸,可考虑将 onResizeEnd 的签名扩展为
(size: number) => void
(需与实现保持一致后再改文档)。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~68-~68: There might be a mistake here.
Context: ... | | onMouseOver | React.MouseEventH...
(QB_NEW_EN)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (2)
src/Drawer.tsx (1)
29-38
: 重复并且不一致的类型定义:resizable 在 DrawerProps 与 DrawerPopupProps 中冲突
DrawerProps
继承了DrawerPopupProps
,而此处又重新声明了resizable
,且把onResize
标为必填;DrawerPopupProps
中对应字段是可选。TS 会因为重复字段且类型不一致报错,同时也与 README 的“非受控示例(resizable={{}})”相矛盾。建议删除重复定义,或直接复用
DrawerPopupProps['resizable']
,并保持全部回调为可选:styles?: DrawerStyles; - /** Default width for uncontrolled resizable drawer */ + /** Default width for uncontrolled resizable drawer */ defaultWidth?: number | string; - /** Default height for uncontrolled resizable drawer */ + /** Default height for uncontrolled resizable drawer */ defaultHeight?: number | string; - /** Resizable configuration - object with optional callbacks */ - resizable?: { - onResize: (size: number) => void; - onResizeStart?: () => void; - onResizeEnd?: () => void; - }; + /** Resizable configuration - object with optional callbacks */ + resizable?: DrawerPopupProps['resizable'];src/DrawerPopup.tsx (1)
399-407
: 与注释相反:当 resizable 开启时应允许拖拽手柄溢出显示注释写明“Allow dragger to be visible outside wrapper bounds”,但实际两支路都设置为
'hidden'
,会导致手柄被裁剪。建议在可调整尺寸时设为'visible'
,未开启时保持'hidden'
。const dynamicWrapperStyle = React.useMemo(() => { const style: React.CSSProperties = { ...wrapperStyle }; if (resizable) { - // Allow dragger to be visible outside wrapper bounds - style.overflow = 'hidden'; + // Allow dragger to be visible outside wrapper bounds + style.overflow = 'visible'; } else { style.overflow = 'hidden'; } return style; }, [wrapperStyle, resizable]);
🧹 Nitpick comments (2)
src/DrawerPopup.tsx (2)
352-361
: maxSize 计算仅在父元素尺寸变更时有效,建议补充窗口 resize 监听(可选)当前仅在挂载和拖拽开始时计算一次
maxSize
。若容器在窗口尺寸变化中改变,maxSize
不会更新。可在open
或 windowresize
时重算,以提升鲁棒性。示例(可按需精简):
React.useEffect(() => { calculateMaxSize(); -}, [calculateMaxSize]); +}, [calculateMaxSize, open]); + +React.useEffect(() => { + const handler = () => calculateMaxSize(); + window.addEventListener('resize', handler); + return () => window.removeEventListener('resize', handler); +}, [calculateMaxSize]);
80-88
: 可进一步提升:a11y 与触摸支持(可选)
- 建议为 dragger 增加无障碍语义(如 role="separator"、aria-orientation,必要时加 aria-valuemin/max/now)。
- 建议后续拓展 pointer/touch 事件以支持移动端。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
README.md
(3 hunks)docs/examples/resizable.tsx
(1 hunks)src/Drawer.tsx
(3 hunks)src/DrawerPopup.tsx
(7 hunks)tests/index.spec.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/examples/resizable.tsx
- tests/index.spec.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/DrawerPopup.tsx (2)
src/util.ts (1)
parseWidthHeight
(5-15)src/hooks/useDrag.ts (1)
useDrag
(28-158)
🪛 LanguageTool
README.md
[grammar] ~114-~114: There might be a mistake here.
Context: ...ull / className / id / tagName / array | | levelMove | number | array | f...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ... | | maskClosable | boolean ...
(QB_NEW_EN)
[grammar] ~127-~127: There might be a mistake here.
Context: ... | | onMouseOver | React.MouseEventHand...
(QB_NEW_EN)
[grammar] ~129-~129: There might be a mistake here.
Context: ... | | onClick | React.MouseEventHand...
(QB_NEW_EN)
🔇 Additional comments (7)
README.md (1)
20-21
: “Basic Usage” 标题微调 OK新增标题层级合理,无功能性问题。
src/Drawer.tsx (2)
61-64
: 解构新增的非受控尺寸 props 并透传 OK
defaultWidth/defaultHeight/resizable
的解构与透传方式正确,未引入副作用。
140-143
: 透传 resizable/defaultWidth/defaultHeight 到 DrawerPopup 的位置正确顺序在覆盖位之后,行为可控;与
...props
合并不会引入冲突。src/DrawerPopup.tsx (4)
264-316
: 宽高决策逻辑合理,已兼容受控/非受控(按是否提供 onResize 判定)
- 横向时优先
currentSize
,否则在非受控时用defaultWidth
,否则用width
;- 纵向同理。
搭配parseWidthHeight
的字符串数值转换,逻辑清晰。
460-460
: 渲染拖拽手柄位置与条件判断正确只在提供
resizable
时渲染,且dragElementProps
已附带类名/样式/事件,符合预期。
385-397
: useDrag 接入参数完备,containerRef 传递方式正确已使用
containerRef
(而非.current
的值)传入,避免首渲染为 null 的测量问题。
318-325
: 事件透传与可访问性无回归原有鼠标/键盘事件透传保持,焦点哨兵与 ESC 关闭逻辑未改变。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/DrawerPopup.tsx (1)
390-401
: resizable 时建议使用 overflow: visible,以确保拖拽条可见且可交互注释已说明“允许 dragger 超出容器可见”,但代码两分支都设置为
'hidden'
,与目标相悖。过去也有机器人指出 overflow 值问题(此前有人误用'none'
)。建议如下:const dynamicWrapperStyle = React.useMemo(() => { const style: React.CSSProperties = { ...wrapperStyle }; if (resizable) { - // Allow dragger to be visible outside wrapper bounds - style.overflow = 'hidden'; + // Allow dragger to be visible outside wrapper bounds + style.overflow = 'visible'; } else { style.overflow = 'hidden'; } return style; }, [wrapperStyle, resizable]);请本地验证:拖拽把手是否需要超出内容区域;若样式层通过绝对定位/负 margin 已保证在可视区内,亦可保留
hidden
。建议配合 CSS 查看 dragger 的定位与命中区域是否受影响。
🧹 Nitpick comments (4)
src/DrawerPopup.tsx (4)
367-375
: 转发尺寸参数:onResizeStart/onResizeEnd 不应丢弃 size内部
useDrag
会传入startSize
/finalSize
,当前实现未接收参数,导致信息丢失。建议按下调整:- const handleResizeStart = React.useCallback(() => { - calculateMaxSize(); - resizable?.onResizeStart?.(); - }, [resizable, calculateMaxSize]); + const handleResizeStart = React.useCallback( + (startSize: number) => { + calculateMaxSize(); + resizable?.onResizeStart?.(startSize); + }, + [resizable, calculateMaxSize], + ); - const handleResizeEnd = React.useCallback(() => { - resizable?.onResizeEnd?.(); - }, [resizable]); + const handleResizeEnd = React.useCallback( + (endSize: number) => { + resizable?.onResizeEnd?.(endSize); + }, + [resizable], + );
403-405
: 窗口尺寸变化时重新计算 maxSize,避免约束失效当前仅在挂载/开始拖拽时计算
maxSize
,浏览器窗口缩放或父容器尺寸变化会导致约束过时。建议在resize
事件中同步:- React.useEffect(() => { - calculateMaxSize(); - }, [calculateMaxSize]); + React.useEffect(() => { + calculateMaxSize(); + const handler = () => calculateMaxSize(); + window.addEventListener('resize', handler); + return () => window.removeEventListener('resize', handler); + }, [calculateMaxSize]);如果 Drawer 常嵌套在可变尺寸容器中,可考虑用
ResizeObserver
监听wrapperRef.current.parentElement
,进一步提升鲁棒性。需要我补一版useSizeObserver
吗?Also applies to: 349-357
327-347
: 数值字符串到 number 的一致化处理(微调)
updateCurrentSize
仅在typeof targetSize === 'number'
时更新内部值;若外部以'300'
(字符串)传入,在此不会初始化currentSize
。为了与后续parseWidthHeight
保持一致,可在此也进行一次解析:- if (typeof targetSize === 'number') { - setCurrentSize(targetSize); - } else { - setCurrentSize(undefined); - } + const parsed = parseWidthHeight(targetSize); + if (typeof parsed === 'number') { + setCurrentSize(parsed); + } else { + setCurrentSize(undefined); + }
451-451
: 为拖拽条增加基础无障碍语义(A11y)给 dragger 增加
role="separator"
和方向,有助于读屏器理解其用途。后续可考虑键盘箭头调整尺寸。- {resizable && <div {...dragElementProps} />} + {resizable && ( + <div + {...dragElementProps} + role="separator" + aria-orientation={isHorizontal ? 'vertical' : 'horizontal'} + aria-label="Resize drawer" + /> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
README.md
(2 hunks)src/Drawer.tsx
(4 hunks)src/DrawerPopup.tsx
(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Drawer.tsx
- README.md
🧰 Additional context used
🧬 Code graph analysis (1)
src/DrawerPopup.tsx (2)
src/util.ts (1)
parseWidthHeight
(5-15)src/hooks/useDrag.ts (1)
useDrag
(28-158)
🔇 Additional comments (3)
src/DrawerPopup.tsx (3)
376-388
: useDrag 的 containerRef 传递方式正确,避免初次渲染 null 带来的起始尺寸测量问题此前通过
.current
传值会在首次渲染为null
,现在传入ref
本身由 hook 内部读取,问题已规避。
438-449
: 拖拽中屏蔽 motion className 的做法妥当
!isDragging && motionClassName
可避免拖拽过程与过渡动画相互干扰,体验更顺滑。
80-88
: 无需同步类型或更新文档,样式对子组件 pointer-events 已正确设置
- 类型定义:
src/DrawerPopup.tsx
与src/Drawer.tsx
中对resizable
的声明均为对象配置,并且示例(docs/examples/resizable.tsx
、docs/demo/resizable.md
)已使用对象写法,无需同步或变更。- 样式检查:全局样式中
.drawer-popup
根元素虽设置pointer-events: none;
,但对内层.drawer-popup-inline
、.drawer-popup-content
、.drawer-popup-handle
等都明确覆盖为pointer-events: auto;
(assets/index.less 行 15、27、59、66),确保拖拽柄可正常接收事件。该评论建议已验证,无需额外变更。
rc 的仓库第一次提交运行ci需要member ap 才行,有需要的话可以群里催一下。 或者手动@大佬帮忙,@yoyo837 @li-jia-nan ![]() |
ap了 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (1)
src/DrawerPopup.tsx (1)
500-535
: 同步 DrawerProps.resizable 类型与文档脚本检查未在
src/Drawer.tsx
的DrawerProps
中找到resizable
定义,文档和示例也缺少用法。
- 在
src/Drawer.tsx
的DrawerProps
接口中添加resizable?
联合类型(例如boolean
或包含回调的对象类型)。- 在
README.md
及docs/
(或对应 demo 示例)中新增<Drawer resizable />
的简写用法及其回调参数说明。
♻️ Duplicate comments (5)
src/DrawerPopup.tsx (5)
80-87
: resizable 类型需支持布尔简写,回调参数签名需与 useDrag 对齐目前仅定义为对象,且
onResizeStart/onResizeEnd
无参数;与 PR 目标(<Drawer resizable />
)及useDrag
行为不一致。建议:
- 将类型改为
boolean | { … }
;- 将回调签名改为携带尺寸参数。
应用如下补丁:
// resizable defaultWidth?: number | string; defaultHeight?: number | string; - resizable?: { - onResize?: (size: number) => void; - onResizeStart?: () => void; - onResizeEnd?: () => void; - }; + resizable?: boolean | { + onResize?: (size: number) => void; + onResizeStart?: (startSize: number) => void; + onResizeEnd?: (endSize: number) => void; + };
399-407
: 转发尺寸参数到 onResizeStart/onResizeEnd,保持与 useDrag 对齐当前丢弃了
useDrag
传入的尺寸参数。- const handleResizeStart = React.useCallback(() => { - calculateMaxSize(); - resizable?.onResizeStart?.(); - }, [resizable, calculateMaxSize]); + const handleResizeStart = React.useCallback((startSize: number) => { + calculateMaxSize(); + resizableCfg?.onResizeStart?.(startSize); + }, [resizableCfg, calculateMaxSize]); - const handleResizeEnd = React.useCallback(() => { - resizable?.onResizeEnd?.(); - }, [resizable]); + const handleResizeEnd = React.useCallback((endSize: number) => { + resizableCfg?.onResizeEnd?.(endSize); + }, [resizableCfg]);
422-433
: overflow 注释与实现不一致,应在可拖拽时使用visible
注释写明“允许 dragger 超出 wrapper 可见”,但实现两分支均为
'hidden'
。请修正:const dynamicWrapperStyle = React.useMemo(() => { const style: React.CSSProperties = { ...wrapperStyle }; - if (resizable) { - // Allow dragger to be visible outside wrapper bounds - style.overflow = 'hidden'; - } else { - style.overflow = 'hidden'; - } + if (enableResizable) { + // Allow dragger to be visible outside wrapper bounds + style.overflow = 'visible'; + } else { + style.overflow = 'hidden'; + } return style; - }, [wrapperStyle, resizable]); + }, [wrapperStyle, enableResizable]);
388-397
: 受控模式下不应更新内部状态,仅透传回调与上文统一,避免受控场景下内部状态污染。
- const handleResize = React.useCallback( - (size: number) => { - // In controlled mode, only trigger callback without updating internal state - if (!isControlledMode) { - setCurrentSize(size); - } - resizable?.onResize?.(size); - }, - [resizable, isControlledMode], - ); + const handleResize = React.useCallback( + (size: number) => { + if (!isControlled) { + setCurrentSize(size); + } + resizableCfg?.onResize?.(size); + }, + [resizableCfg, isControlled], + );
311-343
: 尺寸优先级需调整为“受控优先 → 内部拖拽态 → 默认值”当前把
currentSize
放在width/height
之前,受控模式下可能被内部状态覆盖。建议简化并修正优先级:- if (isHorizontal) { - let finalWidth: number | string; - if (isControlledMode) { - // In controlled mode, prioritize external width - finalWidth = width !== undefined ? width : defaultWidth; - } else if (currentSize !== undefined) { - // In uncontrolled mode, user dragged size takes precedence - finalWidth = currentSize; - } else if (width !== undefined) { - // Fallback to provided width - finalWidth = width; - } else { - // Final fallback to default - finalWidth = defaultWidth; - } - wrapperStyle.width = parseWidthHeight(finalWidth); - } else { - let finalHeight: number | string; - if (isControlledMode) { - // In controlled mode, prioritize external height - finalHeight = height !== undefined ? height : defaultHeight; - } else if (currentSize !== undefined) { - // In uncontrolled mode, user dragged size takes precedence - finalHeight = currentSize; - } else if (height !== undefined) { - // Fallback to provided height - finalHeight = height; - } else { - // Final fallback to default - finalHeight = defaultHeight; - } - wrapperStyle.height = parseWidthHeight(finalHeight); - } + if (isHorizontal) { + const finalWidth = (width ?? currentSize ?? defaultWidth); + wrapperStyle.width = parseWidthHeight(finalWidth); + } else { + const finalHeight = (height ?? currentSize ?? defaultHeight); + wrapperStyle.height = parseWidthHeight(finalHeight); + }
🧹 Nitpick comments (6)
tests/index.spec.tsx (4)
7-7
: 测试直接引入内部 Hook,建议隔离为独立用例或通过公开 API 验证直接从
src/hooks/useDrag
引入会放大内部重构对用例的脆弱性。建议:
- 将此用例迁移到专门的 hooks 测试文件;
- 或仅通过
<Drawer resizable />
的公开 API 验证回退到容器尺寸的逻辑。
571-637
: min/max 约束用例建议同时断言回调与最终 DOM 一致当前仅断言最终 DOM 宽度到 0 与 500。建议补充断言
onResize
在越界时收到的值已被 clamp(尤其是 maxSize=父容器 500 的路径)。
720-750
: props 优先级用例合理,但建议新增 resizable 布尔简写用例当前验证了 prop 优先 default 的优先级。根据 PR 目标与讨论,
<Drawer resizable />
布尔简写应受支持,建议补充对应用例以防回归。
752-802
: 请在受控模式下补充拖拽尺寸不变的断言并确认 onResize 回调被触发当前用例只通过
rerender
验证了受控更新,但并未覆盖“拖拽不会修改受控宽高”这一行为。建议在 tests/index.spec.tsx 中,针对宽度和高度的受控分支,各自加入拖拽模拟并断言:
- 选中拖拽句柄
.rc-drawer-resizable-dragger
(已在全局样式/组件中定义)- 通过
fireEvent.mouseDown
、mouseMove
、mouseUp
模拟拖拽- 期望 DOM 尺寸保持受控值不变
- 同时验证注入的
onResize
回调被调用示例 Diff(仅示意,行号请根据实际位置调整):
@@ tests/index.spec.tsx:752 - const { rerender: rerender1, unmount: unmount1 } = render( + const onResize1 = jest.fn(); + const { rerender: rerender1, unmount: unmount1 } = render( <Drawer open placement="left" width={200} - resizable={{ onResize: () => {} }} + resizable={{ onResize: onResize1 }} > @@ tests/index.spec.tsx:760 rerender1( <Drawer open placement="left" width={300} - resizable={{ onResize: () => {} }} + resizable={{ onResize: onResize1 }} > @@ tests/index.spec.tsx:765 - expect(contentWrapper).toHaveStyle({ width: '300px' }); + expect(contentWrapper).toHaveStyle({ width: '300px' }); + // 模拟拖拽:受控宽度不应变化,且 onResize1 被调用 + const dragger1 = document.querySelector('.rc-drawer-resizable-dragger')!; + fireEvent.mouseDown(dragger1, { clientX: 200 }); + fireEvent.mouseMove(document, { clientX: 400 }); + fireEvent.mouseUp(document, { clientX: 400 }); + expect(contentWrapper).toHaveStyle({ width: '300px' }); + expect(onResize1).toHaveBeenCalled(); @@ tests/index.spec.tsx:775 - const { rerender: rerender2, unmount: unmount2 } = render( + const onResize2 = jest.fn(); + const { rerender: rerender2, unmount: unmount2 } = render( <Drawer open placement="top" height={150} - resizable={{ onResize: () => {} }} + resizable={{ onResize: onResize2 }} > @@ tests/index.spec.tsx:783 - expect(contentWrapper).toHaveStyle({ height: '250px' }); + expect(contentWrapper).toHaveStyle({ height: '250px' }); + // 模拟拖拽:受控高度不应变化,且 onResize2 被调用 + const dragger2 = document.querySelector('.rc-drawer-resizable-dragger')!; + fireEvent.mouseDown(dragger2, { clientY: 200 }); + fireEvent.mouseMove(document, { clientY: 100 }); + fireEvent.mouseUp(document, { clientY: 100 }); + expect(contentWrapper).toHaveStyle({ height: '250px' }); + expect(onResize2).toHaveBeenCalled();src/DrawerPopup.tsx (2)
356-372
: 在受控模式下应避免回写内部currentSize
当前会根据
width/height/default*
设定currentSize
,可能与受控意图冲突。建议在受控模式下清空内部currentSize
,其起始值由 DOM 量测保证;非受控再按默认值/props number 初始化。- const updateCurrentSize = React.useCallback(() => { + const updateCurrentSize = React.useCallback(() => { + if (isControlled) { + setCurrentSize(undefined); + return; + } let targetSize: number | string | undefined; @@ - if (typeof targetSize === 'number') { + if (typeof targetSize === 'number') { setCurrentSize(targetSize); } else { setCurrentSize(undefined); } - }, [isHorizontal, width, height, defaultWidth, defaultHeight]); + }, [isHorizontal, width, height, defaultWidth, defaultHeight, isControlled]);
483-483
: 为拖拽把手补充无障碍语义与可点交互保证建议给把手增加语义与交互属性,提升可访问性并防止外层
pointer-events
风险时失效:- {resizable && <div {...dragElementProps} />} + {enableResizable && ( + <div + {...dragElementProps} + role="separator" + aria-orientation={isHorizontal ? 'vertical' : 'horizontal'} + // 确保可交互 + style={{ pointerEvents: 'auto', ...dragElementProps.style }} + /> + )}同时请在样式侧确认未对 wrapper 施加
pointer-events: none
(社区已有反馈),避免拖拽失效。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/DrawerPopup.tsx
(7 hunks)tests/index.spec.tsx
(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/DrawerPopup.tsx (2)
src/util.ts (1)
parseWidthHeight
(5-15)src/hooks/useDrag.ts (1)
useDrag
(28-158)
tests/index.spec.tsx (1)
src/hooks/useDrag.ts (1)
useDrag
(28-158)
🔇 Additional comments (6)
tests/index.spec.tsx (4)
639-687
: 纵向拖拽用例覆盖良好通过
placement="top"
验证了 Y 轴拖拽方向与数值更新,断言清晰可读。LGTM。
688-718
: 默认尺寸兜底覆盖到位验证了 number 与 string 两条路径下的默认宽/高设置,覆盖恰当。LGTM。
804-854
: 直接验证 useDrag 回退容器尺寸的用例很有价值用例严谨验证了
currentSize === undefined
时的起始尺寸与回调参数,覆盖到 X 轴位移与最终 endSize。LGTM。
480-569
: 我已添加脚本以验证 DrawerProps 中的width
属性声明,以及 DrawerPopup 中isControlledMode
的实现逻辑,以确认当前测试对“受控/非受控”模式的处理是否符合预期。执行结果出来后,我们将据此决定是否需要修改测试或添加新的断言。src/DrawerPopup.tsx (2)
408-420
: useDrag 集成整体 OK
containerRef
、maxSize
、currentSize
与回调链路衔接正确,isDragging
用于抑制 motion 类名也合理。LGTM。
471-475
: 拖拽期间抑制过渡动画的处理很到位
!isDragging && motionClassName
规避了拖拽与过渡的冲突。LGTM。
styles, | ||
drawerRender, | ||
resizable, | ||
defaultWidth, | ||
defaultHeight, | ||
} = props; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
区分启用标志与配置对象,避免联合类型下的属性访问风险
在联合类型落地后,请引入派发变量:
const enableResizable = !!resizable;
const resizableCfg = typeof resizable === 'object' ? resizable : undefined;
后续仅对 resizableCfg?.onResize*
取属性,避免布尔分支的类型错误。
- drawerRender,
- resizable,
+ drawerRender,
+ resizable,
defaultWidth,
defaultHeight,
} = props;
+ const enableResizable = !!resizable;
+ const resizableCfg = typeof resizable === 'object' ? resizable : undefined;
📝 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.
styles, | |
drawerRender, | |
resizable, | |
defaultWidth, | |
defaultHeight, | |
} = props; | |
const { | |
styles, | |
drawerRender, | |
resizable, | |
defaultWidth, | |
defaultHeight, | |
} = props; | |
const enableResizable = !!resizable; | |
const resizableCfg = typeof resizable === 'object' ? resizable : undefined; |
🤖 Prompt for AI Agents
In src/DrawerPopup.tsx around lines 137 to 142, resizable is a union (boolean |
object) so directly accessing properties risks type errors; introduce two
dispatched variables: create an enable flag (e.g., enableResizable =
!!resizable) and a config variable (e.g., resizableCfg = typeof resizable ===
'object' ? resizable : undefined), then everywhere only read handler/option
properties from resizableCfg (e.g., resizableCfg?.onResize*) and use
enableResizable for boolean checks to avoid accessing properties when resizable
is false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/DrawerPopup.tsx (3)
80-87
: 将resizable
改为boolean | { … }
,并让回调携带尺寸参数PR 目标与社区期望是
<Drawer resizable />
简写;当前类型仅对象且onResizeStart/onResizeEnd
无尺寸参数,API 与实现和需求不一致。// resizable defaultWidth?: number | string; defaultHeight?: number | string; - resizable?: { - onResize?: (size: number) => void; - onResizeStart?: () => void; - onResizeEnd?: () => void; - }; + resizable?: boolean | { + onResize?: (size: number) => void; + onResizeStart?: (startSize: number) => void; + onResizeEnd?: (endSize: number) => void; + };同时请在 README/类型导出处同步更新。
137-142
: 区分启用标志与配置对象,避免联合类型属性访问风险在联合类型落地后请引入派发变量,后续只从
resizableCfg
读取回调属性。styles, drawerRender, - resizable, + resizable, defaultWidth, defaultHeight, } = props; +const enableResizable = !!resizable; +const resizableCfg = typeof resizable === 'object' ? resizable : undefined;
368-375
: 透传尺寸参数:与useDrag
对齐,并从resizableCfg
读取
useDrag
会传入startSize/finalSize
,当前实现丢弃了该信息。建议统一携带尺寸参数,便于外部受控更新。-const handleResizeStart = React.useCallback(() => { - calculateMaxSize(); - resizable?.onResizeStart?.(); -}, [resizable, calculateMaxSize]); +const handleResizeStart = React.useCallback( + (startSize: number) => { + calculateMaxSize(); + resizableCfg?.onResizeStart?.(startSize); + }, + [resizableCfg, calculateMaxSize], +); -const handleResizeEnd = React.useCallback(() => { - resizable?.onResizeEnd?.(); -}, [resizable]); +const handleResizeEnd = React.useCallback( + (endSize: number) => { + resizableCfg?.onResizeEnd?.(endSize); + }, + [resizableCfg], +);
🧹 Nitpick comments (3)
src/DrawerPopup.tsx (3)
322-324
:maxSize
与wrapperRef
的引入是必要的;可考虑监听父容器尺寸变更为了在容器尺寸变化(窗口缩放、布局变更)时动态更新
maxSize
,建议用ResizeObserver
监听wrapperRef.current.parentElement
。可以在
calculateMaxSize
附近增加:+React.useEffect(() => { + const parent = wrapperRef.current?.parentElement; + if (!parent) return; + const ro = new ResizeObserver(() => calculateMaxSize()); + ro.observe(parent); + return () => ro.disconnect(); +}, [calculateMaxSize]);
347-355
:maxSize
计算正确;可补空父节点兜底已用可选链处理父节点为空。可进一步增加早退以避免多余 setState。
- if (wrapperRef.current) { + if (wrapperRef.current?.parentElement) { const rect = wrapperRef.current.parentElement?.getBoundingClientRect();
452-452
: 渲染条件与可访问性增强
- 建议改用
enableResizable
作为渲染条件以配合联合类型。- 为拖拽条增加 ARIA 语义,提升可访问性(屏幕阅读器可识别为分隔条)。
-{resizable && <div {...dragElementProps} />} +{enableResizable && ( + <div + role="separator" + aria-orientation={isHorizontal ? 'vertical' : 'horizontal'} + aria-valuemin={0} + aria-valuemax={maxSize || undefined} + aria-valuenow={typeof currentSize === 'number' ? currentSize : undefined} + {...dragElementProps} + /> +)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/DrawerPopup.tsx
(7 hunks)tests/index.spec.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/index.spec.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/DrawerPopup.tsx (2)
src/util.ts (1)
parseWidthHeight
(5-15)src/hooks/useDrag.ts (1)
useDrag
(28-158)
🔇 Additional comments (6)
src/DrawerPopup.tsx (6)
14-14
: 引入 useDrag 没问题依赖位置与用途匹配,命名规范。
264-267
:currentSize
与方向判定初始化合理本地状态与
placement
推导出的isHorizontal
清晰,符合后续计算使用。
288-312
: 受控/非受控优先级正确优先使用受控的
width/height
,再回退到拖拽态currentSize
,最后才是默认值,符合预期。
325-346
: 初始化/同步currentSize
的策略合理仅在数值型时写入
currentSize
,其它情况回退到实时测量,契合拖拽以像素为单位的实现。
377-389
:useDrag
参数传递正确已使用
containerRef: wrapperRef
(避免初次渲染取空值)、传入currentSize
、方向与样式类名均合理。
439-449
: 拖拽时关闭动效类名,合适的 UX 处理
!isDragging && motionClassName
可避免拖拽过程与动效冲突。
const handleResize = React.useCallback( | ||
(size: number) => { | ||
// In controlled mode, only trigger callback without updating internal state | ||
if (!isControlled) { | ||
setCurrentSize(size); | ||
} | ||
resizable?.onResize?.(size); | ||
}, | ||
[resizable, isControlled], | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
回调应从 resizableCfg
读取;受控分支逻辑 OK
在联合类型下直接访问 resizable?.onResize
会有布尔分支类型风险;同时本地状态仅在非受控时更新是正确的。
const handleResize = React.useCallback(
(size: number) => {
// In controlled mode, only trigger callback without updating internal state
if (!isControlled) {
setCurrentSize(size);
}
- resizable?.onResize?.(size);
+ resizableCfg?.onResize?.(size);
},
- [resizable, isControlled],
+ [resizableCfg, isControlled],
);
📝 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.
const handleResize = React.useCallback( | |
(size: number) => { | |
// In controlled mode, only trigger callback without updating internal state | |
if (!isControlled) { | |
setCurrentSize(size); | |
} | |
resizable?.onResize?.(size); | |
}, | |
[resizable, isControlled], | |
); | |
const handleResize = React.useCallback( | |
(size: number) => { | |
// In controlled mode, only trigger callback without updating internal state | |
if (!isControlled) { | |
setCurrentSize(size); | |
} | |
resizableCfg?.onResize?.(size); | |
}, | |
[resizableCfg, isControlled], | |
); |
🤖 Prompt for AI Agents
In src/DrawerPopup.tsx around lines 357 to 366, the code currently accesses
resizable?.onResize directly which risks unsafe access under the union type;
instead read the callback from resizableCfg (e.g. const onResize = typeof
resizableCfg === 'object' ? resizableCfg.onResize : undefined) and invoke that
extracted onResize callback, keeping the existing behavior of only calling
setCurrentSize when !isControlled; also update the useCallback dependency array
to include resizableCfg (or the extracted onResize) instead of resizable so the
callback is correctly updated when the config changes.
const dynamicWrapperStyle = React.useMemo(() => { | ||
const style: React.CSSProperties = { ...wrapperStyle }; | ||
|
||
if (resizable) { | ||
// Allow dragger to be visible outside wrapper bounds | ||
style.overflow = 'hidden'; | ||
} else { | ||
style.overflow = 'hidden'; | ||
} | ||
|
||
return style; | ||
}, [wrapperStyle, resizable]); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
溢出处理应在可调整时设为 visible
,否则拖拽条可能被裁剪
当前两分支均为 'hidden'
,会导致拖拽句柄超出包裹元素时不可见/不可交互。
- if (resizable) {
- // Allow dragger to be visible outside wrapper bounds
- style.overflow = 'hidden';
- } else {
- style.overflow = 'hidden';
- }
+ if (enableResizable) {
+ // Allow dragger to be visible outside wrapper bounds
+ style.overflow = 'visible';
+ } else {
+ style.overflow = 'hidden';
+ }
为避免滚动行为回归,请确认历史样式约定(content-wrapper 是否负责滚动)。可在仓库中检索:
🏁 Script executed:
#!/bin/bash
rg -nP "content-wrapper" -g "**/*.{less,css}"
rg -nP "overflow\s*:\s*(auto|hidden|visible)" -g "**/*.{less,css}"
Length of output: 837
调整 overflow 以兼顾滚动与拖拽
基于仓库中 assets/index.less
对 .content-wrapper
的历史约定(第 57、147 行为 overflow: auto
),非可调整状态下应保留滚动行为;可调整状态下则需设置为 visible
,避免拖拽句柄被裁剪。
请修改如下:
- if (resizable) {
- // Allow dragger to be visible outside wrapper bounds
- style.overflow = 'hidden';
- } else {
- style.overflow = 'hidden';
- }
+ if (resizable) {
+ // 可调整时允许拖拽柄超出容器可见
+ style.overflow = 'visible';
+ } else {
+ // 非可调整时保持默认滚动行为
+ style.overflow = 'auto';
+ }
请在调整后手动验证滚动条仍可正常展示和交互。
📝 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.
const dynamicWrapperStyle = React.useMemo(() => { | |
const style: React.CSSProperties = { ...wrapperStyle }; | |
if (resizable) { | |
// Allow dragger to be visible outside wrapper bounds | |
style.overflow = 'hidden'; | |
} else { | |
style.overflow = 'hidden'; | |
} | |
return style; | |
}, [wrapperStyle, resizable]); | |
const dynamicWrapperStyle = React.useMemo(() => { | |
const style: React.CSSProperties = { ...wrapperStyle }; | |
if (resizable) { | |
// 可调整时允许拖拽柄超出容器可见 | |
style.overflow = 'visible'; | |
} else { | |
// 非可调整时保持默认滚动行为 | |
style.overflow = 'auto'; | |
} | |
return style; | |
}, [wrapperStyle, resizable]); |
🤖 Prompt for AI Agents
In src/DrawerPopup.tsx around lines 391 to 403, the computed dynamicWrapperStyle
currently sets overflow to 'hidden' in both branches which prevents scroll
behavior when not resizable and clips the drag handle when resizable; update the
logic so that when resizable is true set style.overflow = 'visible' (so the
dragger can render outside wrapper bounds) and when resizable is false set
style.overflow = 'auto' (to preserve scrolling), keep the React.useMemo and its
dependencies unchanged, and after applying the change manually verify the
scrollbar still appears and is interactive.
🤔 This is a ...
🔗 Related Issues
close ant-design/ant-design#54597
💡 Background and Solution
add resizable feature to Drawer component
add ResizableLine component to control the size of drawer
📝 Change Log
Summary by CodeRabbit