fix: not prevent when child #1174
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @zombieJ, 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 resolves a bug in 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
|
Walkthrough修改 SelectInput 的 onInternalMouseDown 事件处理逻辑，增强 preventDefault 的条件判断。现在仅在点击目标位于输入框外时才阻止默认行为，而对输入框内部（包括子元素如 textarea）的点击不再触发 preventDefault。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 用户
participant Input as 输入框<br/>(含自定义元素)
participant Handler as onInternalMouseDown
participant Event as 事件默认行为
rect rgb(220, 245, 220)
Note over User,Event: 修复前：点击 textarea 被阻止
User->>Input: 点击 textarea
Input->>Handler: 触发 mouseDown
Handler->>Event: preventDefault()<br/>(无条件)
Event-->>Input: 焦点获取被阻止 ❌
end
rect rgb(245, 220, 220)
Note over User,Event: 修复后：点击 textarea 正常工作
User->>Input: 点击 textarea
Input->>Handler: 触发 mouseDown
Handler->>Handler: 检查点击目标<br/>是否在输入框内?
alt 在输入框内
Handler-->>Event: 不调用 preventDefault ✓
else 在输入框外
Handler->>Event: preventDefault()
end
Event-->>Input: 焦点正常获取 ✓
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
![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 addresses a bug where preventDefault was being called on child elements within a custom select input, which blocked user interaction. The fix in src/SelectInput/index.tsx correctly checks if the event target is a descendant of the input element before preventing the default behavior. A corresponding test has been added in tests/components.test.tsx to validate this change. The implementation is solid, though I have one suggestion to enhance type safety.
| if (!disabled) { | ||
| if (event.target !== getDOM(inputRef.current)) { | ||
| const inputDOM = getDOM(inputRef.current); | ||
| if (inputDOM && event.target !== inputDOM && !inputDOM.contains(event.target as Node)) { |
There was a problem hiding this comment.
For better type safety and to avoid an explicit type cast, it's preferable to use a type guard like instanceof Node to verify that event.target is a DOM node before using DOM-specific methods such as contains.
| if (inputDOM && event.target !== inputDOM && !inputDOM.contains(event.target as Node)) { | |
| if (inputDOM && event.target instanceof Node && event.target !== inputDOM && !inputDOM.contains(event.target)) { |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1174 +/- ##
=======================================
Coverage 99.41% 99.41%
=======================================
Files 31 31
Lines 1195 1196 +1
Branches 425 423 -2
=======================================
+ Hits 1188 1189 +1
Misses 7 7 ☔ 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: 0
🧹 Nitpick comments (1)
tests/components.test.tsx (1)
26-47: 用例正确覆盖回归场景，可小幅增强健壮性这个用例很好地锁定了「自定义输入包裹 textarea 时，不应调用
preventDefault」的行为变化，事件模拟方式（重写原生event.preventDefault后fireEvent触发）也能可靠捕获onInternalMouseDown中的调用，逻辑清晰。可以考虑一个小优化：在创建事件前显式断言 textarea 存在，避免未来结构变动时代码在
createEvent处直接抛错，而不是给出更直观的断言失败信息，例如：- const textareaEle = container.querySelector('textarea'); + const textareaEle = container.querySelector('textarea'); + expect(textareaEle).not.toBeNull();整体上用例设计是对本次修复非常有价值的补充。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/SelectInput/index.tsx(1 hunks)tests/components.test.tsx(2 hunks)
🔇 Additional comments (2)
tests/components.test.tsx (1)
1-1: 测试库导入与新用例匹配良好这里补充导入
createEvent和fireEvent，与后面新增的用例需求完全对齐，没有多余依赖，看起来是合理且必要的调整。src/SelectInput/index.tsx (1)
172-177: 有针对性地缩小 preventDefault 作用范围，修复自定义子输入无法获得焦点的问题这里通过
inputDOM+contains判断，仅在点击目标既不是输入根节点、也不在其 DOM 子树内时才调用event.preventDefault()，从而允许像<div><textarea /></div>这类包裹结构内的实际输入元素获得浏览器默认的 focus 与光标行为，逻辑上与新增测试以及关联问题描述是一致的。需要留意的一点是：当
inputRef.current尚未挂载时，现在会跳过preventDefault（旧逻辑在这种瞬间也会阻止默认行为）。这个行为差异理论上只会出现在极短的初始阶段，通常影响很小，但建议在实际交互回归中顺带确认是否有意料之外的闪动或聚焦差异。
close ant-design/ant-design#55924
Summary by CodeRabbit
发布说明
Bug 修复
测试
✏️ Tip: You can customize this high-level summary in your review settings.