Not close #1180
Not close #1180
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本PR调整 Select 的焦点/模糊与外部点击处理：新增 isInside 辅助、将触发器控制集中到 getSelectElements，用宏任务延迟处理 root-level blur，并在弹出层增加 onPopupBlur 支持；同时移除 SelectInput 的根 onBlur 绑定并微调宏任务与测试定时。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 用户
participant Popup as 弹出层（自定义）
participant BaseSelect as BaseSelect
participant Trigger as SelectTrigger
participant GlobalMouse as 全局鼠标事件
User->>Popup: 在自定义区域点击或聚焦输入
Popup->>Trigger: 焦点停留于 popup 内
GlobalMouse->>BaseSelect: mousedown 事件
BaseSelect->>BaseSelect: 收集 elements -> 调用 isInside(elements, target)
alt 目标在元素集合内
BaseSelect-->>GlobalMouse: isInside=true（跳过关闭）
Note right of BaseSelect: 弹窗保持打开
else 目标在外部
BaseSelect-->>GlobalMouse: isInside=false
BaseSelect->>Trigger: 触发 onPopupBlur -> 转为 onRootBlur
Trigger->>BaseSelect: onRootBlur（通过 macroTask 延迟）
BaseSelect->>Popup: 执行关闭逻辑
end
Estimated code review effort🎯 3 (中等) | ⏱️ ~20-25 分钟 需要重点检查：
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
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 @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! 此拉取请求旨在重构 Select 组件的失焦处理机制，将原本分散在子组件中的 blur 逻辑统一收敛到 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
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1180 +/- ##
==========================================
+ Coverage 99.41% 99.47% +0.05%
==========================================
Files 31 31
Lines 1202 1331 +129
Branches 407 468 +61
==========================================
+ Hits 1195 1324 +129
Misses 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/focus.test.tsx (1)
6-13: 优秀的测试基础设施改进！集中管理 fake timers 的设置和清理，避免了在每个测试中重复编写 timer 管理代码，提高了测试代码的可维护性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/BaseSelect/index.tsx(6 hunks)src/SelectInput/index.tsx(0 hunks)src/SelectTrigger.tsx(3 hunks)src/hooks/useOpen.ts(1 hunks)src/hooks/useSelectTriggerControl.ts(2 hunks)tests/focus.test.tsx(2 hunks)tests/setup.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/SelectInput/index.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/BaseSelect/index.tsx (2)
src/hooks/useSelectTriggerControl.ts (2)
useSelectTriggerControl(10-43)isInside(4-8)src/hooks/useOpen.ts (1)
macroTask(10-19)
🪛 GitHub Check: CodeQL
src/BaseSelect/index.tsx
[warning] 589-589: Superfluous trailing arguments
Superfluous argument passed to function onRootBlur.
⏰ 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). (2)
- GitHub Check: CodeQL
- GitHub Check: Analyze (javascript)
🔇 Additional comments (9)
src/SelectTrigger.tsx (1)
80-80: 代码实现正确！新增的
onPopupBlur属性已正确定义类型、解构并绑定到弹出层包装 div 的onBlur事件上。这使得 BaseSelect 能够统一处理弹出层的失焦逻辑，与 PR 目标一致。Also applies to: 108-108, 170-170
tests/focus.test.tsx (1)
70-120: 新增测试用例准确覆盖了 issue #56033！此测试验证了当
popupRender包含自定义输入框时，聚焦该输入框或在弹出层内部失焦不会关闭 Select，只有点击外部元素（如 body）才会触发关闭。这直接解决了 PR 目标中提到的问题：在 antd 6.0 中点击 dropdownRender 扩展区域会自动收起的 bug。src/hooks/useSelectTriggerControl.ts (1)
32-32: 简化了外部点击检测逻辑！使用新提取的
isInside辅助函数替代内联的包含性检查，提高了代码可读性。src/BaseSelect/index.tsx (4)
10-10: 正确引入了必要的工具函数！从相关 hooks 中导入
isInside和macroTask，为实现集中式失焦处理提供支持。Also applies to: 24-24
541-547: 集中式外部点击处理实现得当！新增的
getSelectElements辅助函数获取所有需要检查的元素（容器和弹出层），并通过useSelectTriggerControl统一处理外部点击关闭逻辑。这种方式将控制逻辑上移到 Select 层，与 PR 目标一致。
566-572: 失焦处理逻辑设计合理！
onRootBlur使用 macroTask 延迟检查焦点是否真正离开了 Select 组件（包括弹出层），只有在焦点移到外部元素时才关闭。这正是修复 issue #56033 的核心逻辑：点击dropdownRender中的自定义内容时，焦点仍在 Select 元素内部，因此不会关闭。注意：
onRootBlur的类型签名是React.FocusEventHandler<HTMLElement>，但实现中并未使用 event 参数。考虑将签名改为() => void或在需要时使用 event 参数。
778-778: 正确连接了弹出层失焦处理！将
onRootBlur传递给SelectTrigger的onPopupBlur属性，使得弹出层的失焦事件能够触发统一的根级失焦处理逻辑，完成了失焦逻辑向 Select 层的迁移。tests/setup.ts (1)
14-14: Timing adjustment in MessageChannel polyfill is consistent with test patterns and should be stable.The 10ms delay for
postMessagein the MessageChannel polyfill (tests/setup.ts:14) aligns with other test timeout patterns in the codebase (e.g., dynamicChildrenTest.tsx) and matches the macroTask scheduling mechanism used throughout the component (src/hooks/useOpen.ts). The change from 0ms to 10ms provides better stability for async test operations and should not cause timing-related failures. This adjustment is part of broader macroTask timing normalization in the PR and follows established testing patterns.src/hooks/useOpen.ts (1)
89-89: The concern about race conditions is theoretically inverted—increasing the delay from 2 to 3 cycles should reduce risk, not increase it.The code uses
MessageChannel(macroTasks) for deterministic timing, which is the correct approach for this scenario. The web search confirms that proper timing separation prevents React update/effect race conditions. A longer macroTask chain (3 cycles vs 2) provides a larger time window for the current toggle operation to complete before accepting new events, making interleaving less likely.Verification Limitation:
- No test suite exists in this repository to run the proposed verification script
- The timing change (1 additional async cycle) is minimal and directionally safer
- Without actual test failures, concrete race condition reports, or edge case demonstrations, this remains theoretical
Assessment:
The code change appears sound from both theoretical timing principles and React event loop ordering. If there are specific timing-dependent edge cases or test failures, those should be explicitly documented and reproduced to warrant further investigation.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/BaseSelect/index.tsx (1)
24-24: onRootBlur 使用 document.activeElement 存在潜在空值/类型隐患，建议配合 isInside 做更健壮的封装这里把
document.activeElement强制断言为HTMLElement，如果在极端情况下为null或非HTMLElement，会在isInside内部调用.contains时产生运行时风险；建议按之前机器人评论的思路，把isInside的第二个参数扩展为Element | null并在函数内部做空值判断，然后本处直接传入document.activeElement，既避免不安全断言，又保留对 SVG 等元素的支持。
src/hooks/useSelectTriggerControl.ts中示例改动：-export function isInside(elements: (HTMLElement | SVGElement | undefined)[], target: HTMLElement) { - return elements - .filter((element) => element) - .some((element) => element.contains(target) || element === target); -} +export function isInside( + elements: (HTMLElement | SVGElement | undefined)[], + target: Element | null, +) { + if (!target) { + return false; + } + + return elements + .filter((element) => element) + .some((element) => element.contains(target) || element === target); +}然后本文件中可简化为：
- const onRootBlur = () => { - macroTask(() => { - if (!isInside(getSelectElements(), document.activeElement as HTMLElement)) { - triggerOpen(false); - } - }); - }; + const onRootBlur = () => { + macroTask(() => { + if (!isInside(getSelectElements(), document.activeElement)) { + triggerOpen(false); + } + }); + };这样
onInternalBlur和onPopupBlur统一走onRootBlur的同时，也避免了潜在的空指针问题。Also applies to: 566-572, 589-589, 778-778
🧹 Nitpick comments (1)
src/BaseSelect/index.tsx (1)
10-10: 集中化全局点击控制（useSelectTriggerControl + getSelectElements）的方向是对的通过
getSelectElements把容器 DOM 和弹层 DOM 一起交给useSelectTriggerControl，可以避免 dropdownRender 区域被误判为“组件外”，整体策略和 issue 里的预期是一致的。可以顺带确认一下 TS 类型：如果getDOM(containerRef.current)在类型上可能为null或更宽的Element，可考虑在这里显式收窄为(HTMLElement | SVGElement | undefined)（例如加一层as或本地变量并过滤），以和useSelectTriggerControl的签名完全对齐，避免将来需要在别处做重复断言。- const getSelectElements = () => [ - getDOM(containerRef.current), - triggerRef.current?.getPopupElement(), - ]; + const getSelectElements = () => [ + getDOM(containerRef.current) as HTMLElement | undefined, + triggerRef.current?.getPopupElement(), + ];Also applies to: 540-548
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/BaseSelect/index.tsx(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/BaseSelect/index.tsx (2)
src/hooks/useSelectTriggerControl.ts (2)
useSelectTriggerControl(10-43)isInside(4-8)src/hooks/useOpen.ts (1)
macroTask(10-19)
* chore: extract func * fix: blur logic * test: add test case * chore: clean up
统一将 blur 逻辑上移，在 Select 层收口。
fix ant-design/ant-design#56033
Summary by CodeRabbit
发布说明
✏️ Tip: You can customize this high-level summary in your review settings.