fix(drawer): support closable placement configuration from ConfigProvider#55985
fix(drawer): support closable placement configuration from ConfigProvider#55985meet-student merged 14 commits intoant-design:masterfrom
Conversation
|
|
|
|
👁 Visual Regression Report for PR #55985 Failed ❌
Important There are 1 diffs found in this PR: 🔄
|
Summary of ChangesHello @meet-student, 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 crucial bug fix for the Drawer component, specifically addressing an oversight where 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
|
|
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. 📝 WalkthroughSummary by CodeRabbit发布说明
✏️ Tip: You can customize this high-level summary in your review settings. Walkthrough将 Drawer 的 closable 来源扩展为优先使用组件 prop，缺省时回退到 drawerContext 的 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 分钟
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (9)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
components/**/*.tsx📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
components/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,md}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,css}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js}📄 CodeRabbit inference engine (AGENTS.md)
Files:
components/*/*.md📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
components/**/*.md📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.md📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (3)📚 Learning: 2025-11-24T16:30:28.374ZApplied to files:
📚 Learning: 2025-11-24T16:31:15.831ZApplied to files:
📚 Learning: 2025-11-24T16:31:15.831ZApplied to files:
🧬 Code graph analysis (1)components/drawer/DrawerPanel.tsx (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
🔇 Additional comments (3)
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 |
15219f8 to
65f129b
Compare
There was a problem hiding this comment.
Code Review
This pull request correctly fixes an issue where the closable placement configuration from ConfigProvider was not being applied to the Drawer component. The changes in DrawerPanel.tsx properly merge the closable prop from both the component props and the context, with the component prop taking precedence. New tests have been added to verify this behavior.
My review includes a couple of suggestions for improvement:
- A defensive null check to prevent potential runtime errors.
- A small fix in the new test case to ensure proper cleanup, improving test suite stability.
65f129b to
37c056f
Compare
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: 遇见同学 <1875694521@qq.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: 遇见同学 <1875694521@qq.com>
More templates
commit: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/drawer/__tests__/Drawer.test.tsx (1)
580-636: ConfigProvider 下closableplacement 行为测试覆盖完整，但断言风格可略作统一优点：
- 分别验证了：
- 仅通过 ConfigProvider 配置
placement: 'end'时，关闭按钮带有.ant-drawer-close-end类（Line 585–597）。- 仅通过 ConfigProvider 配置
placement: 'start'时，没有-end类（Line 603–617）。- Drawer
closableprops 显式传入placement: 'start'时，覆盖掉 ConfigProvider 中的placement: 'end'（Line 623–635）。- 每个
render分支都获取了unmountX并在用例尾部调用，避免对其他用例的 DOM / 定时器产生影响，这一点很好。可以考虑的小优化（可选）：
- 将
expect(closeButtonX).toBeTruthy()与expect(...).toBeFalsy()改为toBeInTheDocument()/toBeNull()，与上文should support closable placement with start/end测试保持一致的 jest-dom 断言风格，会更直观一些，但不是功能性问题。整体来看，该测试很好地锁定了本次修复的核心行为。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/drawer/DrawerPanel.tsx(2 hunks)components/drawer/__tests__/Drawer.test.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always use TypeScript with strict type checking
Never useanytype - define precise types instead
Use interfaces (not type aliases) for object structures
Export all public interface types
Prefer union types over enums, useas constfor constants
Use early returns to improve readability
Support tree shaking
Follow import order: React → dependencies → antd components → custom components → types → styles
Prefer antd built-in components over external dependencies
Pass all ESLint and TypeScript checks
No console errors or warnings
**/*.{ts,tsx}: Use camelCase for property names
All components and functions must provide accurate type definitions
Avoid usinganytype, define types as precisely as possible
Use interface rather than type alias for defining object structures
All TypeScript should compile without errors or warnings
Use generics appropriately to enhance type flexibility
Use intersection types (&) to merge multiple types
Use literal union types to define limited option sets
Avoid using enum, prefer union types andas const
Rely on TypeScript's type inference as much as possible
Use type assertions (as) only when necessary
Complex data structures should be split into multiple interface definitions
Avoid outdated APIs and keep updated to new recommended usage
Files:
components/drawer/__tests__/Drawer.test.tsxcomponents/drawer/DrawerPanel.tsx
components/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
components/**/*.tsx: Component props interfaces should be namedComponentNameProps
Component ref types should useReact.ForwardRefRenderFunction
Use functional components with hooks exclusively (no class components)
Apply performance optimizations with React.memo, useMemo, useCallback appropriately
Support server-side rendering
Components must support ref forwarding with structure including nativeElement, focus, and other methods
Use PascalCase for component names
Use camelCase for props with specific patterns:default+PropNamefor defaults,forceRenderfor force rendering,openinstead ofvisiblefor panel state,show+PropNamefor display toggles,PropName+ablefor capabilities,dataSourcefor data source,disabledfor disabled state,extrafor additional content,iconfor icons,triggerfor triggers,classNamefor CSS classes
Useon+EventNamepattern for event handlers (e.g.,onClick,onChange)
Useon+SubComponentName+EventNamepattern for sub-component events
Use complete names, never abbreviations in prop naming
Optimize for minimal re-renders
UseuseLocalehook fromcomponents/locale/index.tsx
Support accessibility (WCAG 2.1 AA)
Files:
components/drawer/__tests__/Drawer.test.tsxcomponents/drawer/DrawerPanel.tsx
components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never hardcode colors, sizes, or spacing values
components/**/*.{ts,tsx}: Use TypeScript and React for component development
Use functional components and hooks, avoid class components
Use PascalCase for component names
Use complete names rather than abbreviations in component naming
Initialize properties should usedefault+PropNamenaming pattern
Panel opening should useopenprop instead ofvisible
Event handlers should followon+EventNamenaming pattern
Child component events should followon+SubComponentName+EventNamenaming pattern
Components should provide arefattribute withnativeElementandfocusmethods
Export all public interface types for user convenience
Component props should use interface definition namedComponentNameProps
Use CSS transitions for simple animations, rc-motion for complex animations
Respect user's prefers-reduced-motion setting for animations
Follow WCAG 2.1 AA level accessibility standards for styling
Achieve 100% test coverage
UseuseLocalehook fromcomponents/locale/index.tsxto get localization configuration
Use React.memo, useMemo, and useCallback appropriately for performance optimization
UseReact.ForwardRefRenderFunctionfor component ref types
Callback function types should have explicit parameter and return value definitions
Component state should have dedicated interfaces (e.g., ComponentNameState)
Components should display well on different screen sizes (responsive design)
Components should support right-to-left (RTL) reading direction
Support page zoom to 200% with normal layout
Avoid animations that cause flickering
Ensure code runs normally with no console errors
Avoid unnecessary re-renders
Support Tree Shaking for bundle optimization
Files:
components/drawer/__tests__/Drawer.test.tsxcomponents/drawer/DrawerPanel.tsx
**/__tests__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/__tests__/**/*.test.{ts,tsx}: Write comprehensive tests using Jest and React Testing Library
Include snapshot tests for UI components
**/__tests__/**/*.test.{ts,tsx}: Test files should be named index.test.tsx or xxx.test.tsx and placed in tests directory
Use snapshot testing for UI components
Files:
components/drawer/__tests__/Drawer.test.tsx
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation
Files:
components/drawer/__tests__/Drawer.test.tsxcomponents/drawer/DrawerPanel.tsx
**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain cross-browser compatibility
Files:
components/drawer/__tests__/Drawer.test.tsxcomponents/drawer/DrawerPanel.tsx
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js}: Use early returns to improve code readability
Pass all ESLint and TypeScript checks
Files:
components/drawer/__tests__/Drawer.test.tsxcomponents/drawer/DrawerPanel.tsx
**/__tests__/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Jest and React Testing Library for unit tests
Files:
components/drawer/__tests__/Drawer.test.tsx
🧠 Learnings (5)
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Write comprehensive tests using Jest and React Testing Library
Applied to files:
components/drawer/__tests__/Drawer.test.tsx
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Place tests in `__tests__` directory as `index.test.tsx` or `componentName.test.tsx`
Applied to files:
components/drawer/__tests__/Drawer.test.tsx
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Use snapshot testing for UI components
Applied to files:
components/drawer/__tests__/Drawer.test.tsx
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Include snapshot tests for UI components
Applied to files:
components/drawer/__tests__/Drawer.test.tsx
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/*.{ts,tsx} : Achieve 100% test coverage
Applied to files:
components/drawer/__tests__/Drawer.test.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: test lib/es module (es, 2/2)
- GitHub Check: test lib/es module (es, 1/2)
- GitHub Check: build
- GitHub Check: test-node
- GitHub Check: test-react-latest (dom, 2/2)
- GitHub Check: test-react-legacy (18, 2/2)
- GitHub Check: test-react-latest (dom, 1/2)
- GitHub Check: lint
- GitHub Check: test-react-legacy (18, 1/2)
- GitHub Check: build preview
- GitHub Check: visual-diff snapshot (2/2)
- GitHub Check: visual-diff snapshot (1/2)
- GitHub Check: size
- GitHub Check: build
🔇 Additional comments (2)
components/drawer/DrawerPanel.tsx (1)
84-88: 合并closable与上下文配置、计算 placement 的实现是合理且健壮的
- Line 99 使用
closable ?? contextClosable，保证组件props优先于 ConfigProvider 配置，且不会把false误当作“未配置”，符合预期优先级语义。- Line 100 单独处理
false，使closablePlacement为undefined时不再渲染关闭按钮，逻辑上与mergedClosable一致。- Line 103 通过
typeof mergedClosableVal === 'object' && mergedClosableVal避免了null情况下读取placement抛错的问题，空值安全性到位。- 组合下来，默认场景会落到
'start'，placement: 'end'时才增加-close-end类并把按钮渲染到尾部，行为与下方 header 中的左右渲染逻辑一致。整体实现清晰且类型安全，目前不建议额外修改。
Also applies to: 98-107
components/drawer/__tests__/Drawer.test.tsx (1)
506-528:it.each覆盖 mask blur 组合场景较全面，逻辑正确
- 使用
testCases枚举多种mask/contextMask/ 期望 blur / mask 是否打开的组合，避免重复样板代码。- 分支里先根据
openMask判断是否应存在 mask，再断言是否包含ant-drawer-mask-blur，能较好反映业务规则。该段测试目前看起来是正确且易维护的。
Bundle ReportChanges will increase total bundle size by 142 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: antd.min-array-pushAssets Changed:
|
|
@meet-student What is left to do for this PR to be merged? |
|
@zombieJ cc |
|
@meet-student As I see |
ok await. thinks |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/config-provider/index.en-US.md(1 hunks)components/config-provider/index.zh-CN.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
components/*/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
components/*/*.md: Provide both English and Chinese documentation
When documenting component APIs, use table structure with string defaults in backticks, boolean defaults as literal values, number defaults as literal values, no default value as-, descriptions starting with capital letter without ending period, properties sorted alphabetically
Files:
components/config-provider/index.zh-CN.mdcomponents/config-provider/index.en-US.md
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation
Files:
components/config-provider/index.zh-CN.mdcomponents/config-provider/index.en-US.md
components/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
components/**/*.md: Markdown documentation should have Chinese headings with manual English anchor IDs in format: ## 中文标题 {#english-anchor-id}
New properties should declare the version number they become available in documentation
Files:
components/config-provider/index.zh-CN.mdcomponents/config-provider/index.en-US.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: Anchor IDs must match regex^[a-zA-Z][\w-:\.]*$and not exceed 32 characters (except for demo IDs containing-demo-)
FAQ section headings should have anchor IDs prefixed withfaq-
Same topics in Chinese and English documentation should use identical anchor IDs
Provide both Chinese and English versions for documentation
Files:
components/config-provider/index.zh-CN.mdcomponents/config-provider/index.en-US.md
🧠 Learnings (4)
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Applies to components/**/demo/*.tsx : Demo code import order: React → dependencies → antd components → custom components → types → styles
Applied to files:
components/config-provider/index.zh-CN.mdcomponents/config-provider/index.en-US.md
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to components/**/*.tsx : Use camelCase for props with specific patterns: `default` + `PropName` for defaults, `forceRender` for force rendering, `open` instead of `visible` for panel state, `show` + `PropName` for display toggles, `PropName` + `able` for capabilities, `dataSource` for data source, `disabled` for disabled state, `extra` for additional content, `icon` for icons, `trigger` for triggers, `className` for CSS classes
Applied to files:
components/config-provider/index.zh-CN.mdcomponents/config-provider/index.en-US.md
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to components/**/*.{ts,tsx} : Never hardcode colors, sizes, or spacing values
Applied to files:
components/config-provider/index.zh-CN.md
📚 Learning: 2025-11-24T16:31:15.831Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:31:15.831Z
Learning: Support React 16 ~ 19 versions
Applied to files:
components/config-provider/index.en-US.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: test-react-latest-dist (dist-min, 1/2)
- GitHub Check: test-react-latest-dist (dist-min, 2/2)
- GitHub Check: test-react-latest-dist (dist, 2/2)
- GitHub Check: test-react-latest-dist (dist, 1/2)
- GitHub Check: test-react-latest (dom, 2/2)
- GitHub Check: test-react-latest (dom, 1/2)
- GitHub Check: test-react-legacy (18, 1/2)
- GitHub Check: test-react-legacy (18, 2/2)
- GitHub Check: visual-diff snapshot (2/2)
- GitHub Check: visual-diff snapshot (1/2)
🔇 Additional comments (1)
components/config-provider/index.zh-CN.md (1)
132-132: Removeclosableproperty from ConfigProvider drawer configuration; it is not supported.Based on Ant Design documentation, ConfigProvider does not support globally setting
Drawer.closable— theclosableprop must be set per Drawer component instance, not through ConfigProvider. The entry on line 132 includingclosable?: [DrawerProps\["closable"\]](/components/drawer-cn#api)should be removed entirely, as this property is not a valid ConfigProvider drawer configuration option.Likely an incorrect or invalid review comment.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #55985 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 804 804
Lines 14838 14843 +5
Branches 3919 3922 +3
=========================================
+ Hits 14838 14843 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
中文版模板 / Chinese template
🤔 This is a ...
🔗 Related Issues
💡 Background and Solution
📝 Change Log