chore: adjust keyboard event handling logic #558
Conversation
|
@aojunhao123 is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
概述本次变更重构了抽屉组件的键盘事件处理机制。将 ESC 键关闭功能从 DrawerPopup 内部处理转移到 Portal 层级，同时扩展了 onClose 回调的事件类型支持，并相应更新了相关测试用例以验证新的事件流。 变更清单
代码审查工作量评估🎯 2 (简单) | ⏱️ ~12 分钟 建议审查者
诗
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @aojunhao123, 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 refactors the keyboard event handling for the drawer component, specifically for closing the drawer with the ESC key. The change centralizes the ESC key logic within the main 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. 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
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #558 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 6 6
Lines 218 217 -1
Branches 80 79 -1
=========================================
- Hits 218 217 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
This pull request refactors the keyboard event handling for closing the drawer with the Escape key. The logic has been moved from DrawerPopup to the main Drawer component, utilizing the onEsc prop from the Portal component. This is a good improvement, as it centralizes the logic and makes the behavior more robust (e.g., it's no longer dependent on the drawer panel having focus). The tests have been updated accordingly to reflect this change. I've found one minor typing inconsistency that should be addressed to ensure type safety across components.
| prefixCls?: string; | ||
| open?: boolean; | ||
| onClose?: (e: React.MouseEvent | React.KeyboardEvent) => void; | ||
| onClose?: (e: React.MouseEvent | React.KeyboardEvent | KeyboardEvent) => void; |
There was a problem hiding this comment.
You've correctly updated the onClose type here to include KeyboardEvent. However, the onClose prop in the DrawerPopupProps interface (defined in src/DrawerPopup.tsx) was not updated to reflect this change. This creates a type inconsistency between the Drawer and DrawerPopup components, which can lead to type errors under stricter TypeScript configurations (like strictFunctionTypes). To ensure type safety, DrawerPopupProps['onClose'] should also be updated to accept a native KeyboardEvent.
![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 (3)
src/Drawer.tsx (1)
143-162: 建议清理未使用的属性传递。
keyboard属性被传递给DrawerPopup（行 149），但根据src/DrawerPopup.tsx的更改，该组件不再使用此属性（ESC 处理已移至 Portal 层）。如果
DrawerPopup确实不再需要keyboard属性，建议：
- 从传递给
DrawerPopup的 props 中移除keyboard- 或在
DrawerPopup.tsx中从接口定义中移除该属性这有助于保持代码清晰并避免混淆。
src/DrawerPopup.tsx (2)
41-41: 考虑从接口中移除未使用的 keyboard 属性。
keyboard属性仍在DrawerPopupProps接口中定义，但组件内部不再使用它（ESC 处理已移至Drawer.tsx）。建议：
- 如果不需要保持向后兼容性，从
DrawerPopupProps接口中移除keyboard属性- 如果需要保留，添加注释说明该属性由父组件使用
这有助于明确接口的意图并避免混淆。
157-176: LGTM！正确移除了 ESC 键处理逻辑。
onPanelKeyDown现在只处理 TAB 键导航，ESC 键处理已移至Drawer.tsx的onEsc处理器。这使得职责分离更清晰。可选建议：由于现在只有一个键码需要处理，可以考虑将 switch 语句简化为 if 语句，但保持 switch 结构也便于将来扩展。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Drawer.tsxsrc/DrawerPopup.tsxtests/index.spec.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/Drawer.tsx (3)
src/DrawerPopup.tsx (1)
DrawerPopupProps(33-94)src/DrawerPanel.tsx (2)
DrawerPanelEvents(11-18)DrawerPanelAccessibility(20-23)src/context.ts (1)
RefContext(15-15)
src/DrawerPopup.tsx (1)
src/DrawerPanel.tsx (2)
DrawerPanelEvents(11-18)DrawerPanelAccessibility(20-23)
🔇 Additional comments (3)
tests/index.spec.tsx (2)
300-307: LGTM！测试更新与新的 ESC 处理机制对齐。测试从元素级别的 ESC 事件改为窗口级别的 Escape 键事件，这与 Portal 组件的
onEsc处理器行为一致。
309-318: LGTM！禁用 ESC 的测试正确更新。测试验证了当
keyboard={false}时，窗口级别的 Escape 键事件不会触发onClose，符合预期行为。src/Drawer.tsx (1)
24-24: LGTM！正确扩展了 onClose 的类型签名。添加
KeyboardEvent（原生事件类型）是必要的，因为新的onEsc处理器会传递 Portal 提供的原生键盘事件。
| const onEsc: PortalProps['onEsc'] = ({ top, event }) => { | ||
| if (top && keyboard) { | ||
| event.stopPropagation(); | ||
| onClose?.(event); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 查找 src/Drawer.tsx 文件并查看目标行
fd -type f "Drawer.tsx" -path "*/src/*" | head -5Repository: react-component/drawer
Length of output: 236
🏁 Script executed:
# 检查 @rc-component/portal 包是否存在及其结构
ls -la node_modules/@rc-component/portal 2>/dev/null | head -20Repository: react-component/drawer
Length of output: 48
🏁 Script executed:
# 正确的查找命令 - 查找 Drawer.tsx
fd "Drawer.tsx" src/Repository: react-component/drawer
Length of output: 80
🏁 Script executed:
# 查看项目结构
ls -la src/ | head -20Repository: react-component/drawer
Length of output: 675
🏁 Script executed:
# 查找所有 TypeScript 文件中 onEsc 的引用
rg "onEsc" --type ts --type tsxRepository: react-component/drawer
Length of output: 93
🏁 Script executed:
# 查看 Drawer.tsx 的内容，特别关注行 164-169
cat -n src/Drawer.tsx | sed -n '150,180p'Repository: react-component/drawer
Length of output: 1000
🏁 Script executed:
# 搜索 onEsc 的使用（不指定文件类型）
rg "onEsc" -A 5 -B 2Repository: react-component/drawer
Length of output: 686
🏁 Script executed:
# 查看 Drawer.tsx 的导入部分
head -40 src/Drawer.tsxRepository: react-component/drawer
Length of output: 1501
🏁 Script executed:
# 搜索 PortalProps 的定义或使用
rg "PortalProps" -B 2 -A 5Repository: react-component/drawer
Length of output: 1158
🏁 Script executed:
# 查看 package.json 了解依赖信息
cat package.json | grep -A 2 -B 2 "portal\|Portal"Repository: react-component/drawer
Length of output: 218
🏁 Script executed:
# 查找项目中所有的 d.ts 或 TypeScript 定义文件
find . -name "*.d.ts" -o -name "*.ts" | grep -i portal | head -10Repository: react-component/drawer
Length of output: 48
🏁 Script executed:
# 查看是否有 node_modules，并检查 @rc-component/portal 的类型文件
find . -path "*/node_modules/@rc-component/portal*" -name "*.d.ts" 2>/dev/null | head -5Repository: react-component/drawer
Length of output: 48
🌐 Web query:
@rc-component/portal onEsc API documentation
💡 Result:
Short answer: @rc-component/portal does not expose an onEsc prop. Its documented props are getContainer, open, and autoLock; it provides no built‑in Escape-key handler — handle Escape yourself (e.g., add a keydown listener in your portal content or use a higher-level component like rc-dialog that supports onClose). [1][2]
Sources:
- @rc-component/portal npm README (props list: getContainer, open, autoLock). [1]
- Package overview / API listing (no onEsc in API). [2]
🌐 Web query:
@rc-component/portal version 2.0 onEsc escape key handler support
💡 Result:
Short answer: No — @rc-component/portal v2.x does not include a built-in onEsc/escape-key prop. The published API shows only props like getContainer, open and autoLock (no onEsc/close-on-esc). See the package README/API. [1][2]
If you need ESC-to-close behavior, add a keydown listener in your component that wraps Portal (or use a higher-level rc component that provides built-in close-on-esc). [1]
Sources:
- @rc-component/portal README / API (props list: getContainer, open, autoLock). [1]
- jsDelivr/npm listing for @rc-component/portal (v2.x metadata). [2]
🌐 Web query:
rc-component/portal GitHub releases 2024 onEsc feature
💡 Result:
Do you mean the GitHub repo react-component/portal (or the npm package @rc-component/portal)? Do you want to know whether a 2024 release added an onEsc (escape-key) prop/feature?
🏁 Script executed:
# 查找是否有其他 Portal 使用的例子
rg "Portal" src/ -B 2 -A 5 | head -50Repository: react-component/drawer
Length of output: 2096
🏁 Script executed:
# 查看项目的 tsconfig 或相关配置，检查是否有类型覆盖
cat tsconfig.json 2>/dev/null || echo "No tsconfig.json"Repository: react-component/drawer
Length of output: 450
Portal 不支持 onEsc 属性，此实现将无法工作。
根据官方文档，@rc-component/portal v2.x 仅暴露以下属性：getContainer、open 和 autoLock，不包含 onEsc 属性。当前代码将 onEsc 处理器传给 Portal（行 178），但该属性会被忽略，导致 ESC 键无法关闭 Drawer。
建议采用以下替代方案之一：
- 在 DrawerPopup 或 DrawerPanel 组件内部添加 keydown 事件监听器
- 检查 @rc-component/portal 是否计划添加此功能
- 使用其他支持 escape-key 处理的高级组件
2 participants
Summary by CodeRabbit
发布说明
✏️ Tip: You can customize this high-level summary in your review settings.