feat: support boolean for resizable #538
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @cactuser-Lu, 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 enhancement to the Highlights
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
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 与 DrawerPopup 的 resizable 属性从仅支持配置对象扩展为支持布尔值或配置对象。DrawerPopup 内部新增规范化逻辑以统一处理回调与渲染可拖拽句柄的条件，无新增控制流程分支，仅替换访问方式与判定变量。 Changes
Sequence Diagram(s)sequenceDiagram
actor User as User
participant Handle as DraggableHandle
participant DrawerPopup as DrawerPopup
participant Hook as useDrag
participant Callbacks as resizeConfig(onResize/Start/End)
Note over DrawerPopup: 规范化<br/>isResizable = !!resizable<br/>resizeConfig = object或{}
User->>DrawerPopup: 打开抽屉
alt isResizable 为真
DrawerPopup->>Handle: 渲染拖拽句柄
User->>Handle: 拖拽开始
Handle->>Hook: onResizeStart
Hook->>Callbacks: onResizeStart?()
User->>Handle: 拖拽中(size)
Handle->>DrawerPopup: onInternalResize(size)
DrawerPopup->>Callbacks: onResize?(size)
User->>Handle: 拖拽结束
Handle->>Hook: onResizeEnd
Hook->>Callbacks: onResizeEnd?()
else 不可调整大小
Note over DrawerPopup: 不渲染句柄，忽略回调
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #538 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 6 6
Lines 216 218 +2
Branches 79 80 +1
=========================================
+ Hits 216 218 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/Drawer.tsx (1)
43-50: 公共 API 变更 OK，但建议去重并抽出共享类型以避免未来漂移
DrawerProps已继承DrawerPopupProps，这里再次声明resizable容易产生重复与类型偏差。建议：
- 方案 A：直接移除
DrawerProps中的重复声明，沿用继承的定义；- 方案 B：在
inter.ts抽出ResizeConfig，两处统一引用，避免多处粘贴。示例（方案 B）：
- /** Resizable configuration - boolean to enable/disable or object with optional callbacks */ - resizable?: - | boolean - | { - onResize?: (size: number) => void; - onResizeStart?: () => void; - onResizeEnd?: () => void; - }; + /** Resizable configuration - boolean to enable/disable or object with optional callbacks */ + resizable?: boolean | ResizeConfig;并在
inter.ts新增：export type ResizeConfig = { onResize?: (size: number) => void; onResizeStart?: () => void; onResizeEnd?: () => void; };与
DrawerPopup.tsx同步改为resizable?: boolean | ResizeConfig;。src/DrawerPopup.tsx (3)
88-95: API 扩展方向正确，但建议复用共享类型，避免重复定义类型从配置对象扩展为
boolean | { ... }没问题。为降低重复与后续维护成本，建议抽出ResizeConfig（放在inter.ts），此处与Drawer.tsx统一为：- resizable?: - | boolean - | { - onResize?: (size: number) => void; - onResizeStart?: () => void; - onResizeEnd?: () => void; - }; + resizable?: boolean | ResizeConfig;
341-343: 可选：用useEvent包装起止回调，避免闭包陈旧与引用抖动
useDrag若在挂载时订阅回调，直接传入 props 可能造成闭包陈旧或频繁变更。建议（可选）：- onResizeStart: resizeConfig.onResizeStart, - onResizeEnd: resizeConfig.onResizeEnd, + onResizeStart: resizeConfig.onResizeStart ? useEvent(resizeConfig.onResizeStart) : undefined, + onResizeEnd: resizeConfig.onResizeEnd ? useEvent(resizeConfig.onResizeEnd) : undefined,
402-402: 验证拖拽把手的可访问性语义（A11y）请确认
dragElementProps是否包含：
role="separator"与正确的aria-orientation；- 可键盘操作（箭头键/Shift+箭头）与相应
aria-valuenow/max/min（如适用）；- 合理的命中区域与可见焦点样式。
若未包含，建议在
useDrag内补齐语义或此处加属性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Drawer.tsx(1 hunks)src/DrawerPopup.tsx(4 hunks)
🔇 Additional comments (1)
src/DrawerPopup.tsx (1)
329-329: 回调改为走归一化配置，方向正确
onInternalResize内置状态更新并透传到resizeConfig.onResize，与布尔/对象两种形态兼容，符合预期。建议补充一个用例覆盖 “resizable = true 时内部尺寸仍能更新（无回调）”。
| const isResizable = !!resizable; | ||
| const resizeConfig = (typeof resizable === 'object' && resizable) || {}; | ||
|
|
There was a problem hiding this comment.
运行时判断简洁，但 TS 类型不安全（严格模式下会报错）
resizeConfig 目前推断为 {}，后续访问 resizeConfig.onResize 可能触发 “Property 'onResize' does not exist on type '{}'” 的编译错误。建议显式标注类型并 useMemo 避免每次 render 生成新对象：
- const isResizable = !!resizable;
- const resizeConfig = (typeof resizable === 'object' && resizable) || {};
+ const isResizable = Boolean(resizable);
+ type ResizeCfg = Exclude<NonNullable<DrawerPopupProps['resizable']>, boolean>;
+ const resizeConfig: Partial<ResizeCfg> = React.useMemo(
+ () => (typeof resizable === 'object' && resizable) ? resizable : {},
+ [resizable],
+ );如已在 inter.ts 抽出 ResizeConfig，则将 ResizeCfg 替换为 ResizeConfig 更清晰。
📝 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 isResizable = !!resizable; | |
| const resizeConfig = (typeof resizable === 'object' && resizable) || {}; | |
| const isResizable = Boolean(resizable); | |
| type ResizeCfg = Exclude<NonNullable<DrawerPopupProps['resizable']>, boolean>; | |
| const resizeConfig: Partial<ResizeCfg> = React.useMemo( | |
| () => (typeof resizable === 'object' && resizable) ? resizable : {}, | |
| [resizable], | |
| ); |
🤖 Prompt for AI Agents
In src/DrawerPopup.tsx around lines 324-326, the runtime checks set resizeConfig
to {} which TypeScript infers as an empty object and can cause "Property
'onResize' does not exist on type '{}'" errors in strict mode; change the
declaration to an explicitly typed constant (use the ResizeConfig type exported
from inter.ts or rename ResizeCfg → ResizeConfig) and wrap the computed value in
useMemo so you return either the provided object or a typed empty object without
recreating it each render (e.g., const resizeConfig: ResizeConfig = useMemo(()
=> (typeof resizable === 'object' && resizable) || {}, [resizable])).
增加resizable的布尔类型
🤔 This is a ...
🔗 Related Issues
💡 Background and Solution
Summary by CodeRabbit