fix(popover): 修复点击外部触发 ref.show() 时弹窗关闭问题#7073
Conversation
- 新增 showingRef 标记防止 show() 触发链上的 click-away - 添加测试用例验证修复效果
dosubot
Bot
added
size:S
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughPopover 的 ChangesPopover show() 修复
Estimated code review effort: 2 (Simple) | ~15 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ 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 introduces a showingRef to track when the Popover is being shown via ref.show(), preventing the click-away handler from immediately closing it. It also adds corresponding unit tests to verify this behavior. The review feedback suggests managing the setTimeout in show() with a ref and clearing it on unmount or consecutive calls to prevent potential memory leaks, race conditions, or Jest open handle warnings.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const showingRef = useRef(false) | ||
|
|
||
| useImperativeHandle( | ||
| ref, | ||
| () => ({ | ||
| show: () => setVisible(true), | ||
| show: () => { | ||
| showingRef.current = true | ||
| setVisible(true) | ||
| setTimeout(() => { | ||
| showingRef.current = false | ||
| }) | ||
| }, |
There was a problem hiding this comment.
在 show 方法中使用了 setTimeout 来异步重置 showingRef.current。然而，如果组件在定时器触发前被卸载，或者连续多次调用 show，可能会导致以下问题：\n1. 测试环境中的 Open Handles 警告：在测试用例中，如果测试在定时器执行前结束并卸载了组件，未清理的定时器会导致 Jest 报 "open handles" 警告，甚至可能影响后续测试。\n2. 竞态条件：连续多次调用 show 时，多个定时器交错执行可能会提前将 showingRef.current 重置为 false。\n\n建议使用 timeoutRef 保存定时器 ID，并在组件卸载时以及下一次调用 show 前进行清理。
const showingRef = useRef(false)\n const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null)\n\n useEffect(() => {\n return () => {\n if (timeoutRef.current) {\n clearTimeout(timeoutRef.current)\n }\n }\n }, [])\n\n useImperativeHandle(\n ref,\n () => ({\n show: () => {\n showingRef.current = true\n setVisible(true)\n if (timeoutRef.current) {\n clearTimeout(timeoutRef.current)\n }\n timeoutRef.current = setTimeout(() => {\n showingRef.current = false\n timeoutRef.current = null\n })\n },
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/popover/popover.tsx (1)
79-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win建议为该时序技巧补充注释，防止后续被误删。
show()中的setTimeout用于制造一个短暂窗口，让触发show()的同一次点击事件先冒泡完成（此时useClickAway才能读取到showingRef.current === true而跳过关闭）。这个依赖关系并不直观，建议加一行注释说明用途，避免未来有人误以为setTimeout是多余代码而将其移除，导致#6731问题回归。📝 建议补充注释
show: () => { + // 标记进入 show() 触发链，避免同一次点击事件冒泡到 document 时被 useClickAway 立即关闭 showingRef.current = true setVisible(true) setTimeout(() => { showingRef.current = false }) },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/popover/popover.tsx` around lines 79 - 95, The show/hide timing in popover.tsx is intentionally using setTimeout inside useImperativeHandle’s show method to keep showingRef.current true through the same click event so useClickAway won’t immediately close it. Add a short comment near show() and the timeout explaining this event-bubbling window and the dependency on showingRef.current, so future refactors don’t remove it from the Popover component and reintroduce the close-on-open issue.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/popover/popover.tsx`:
- Around line 79-95: The show/hide timing in popover.tsx is intentionally using
setTimeout inside useImperativeHandle’s show method to keep showingRef.current
true through the same click event so useClickAway won’t immediately close it.
Add a short comment near show() and the timeout explaining this event-bubbling
window and the dependency on showingRef.current, so future refactors don’t
remove it from the Popover component and reintroduce the close-on-open issue.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c0ff733-a5e5-4086-9cf5-9555784d9739
📒 Files selected for processing (2)
src/components/popover/popover.tsxsrc/components/popover/tests/popover.test.tsx
- 使用 ref 存储定时器引用 - 在组件卸载时清理定时器 - 防止内存泄漏
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7073 +/- ##
==========================================
+ Coverage 93.34% 93.39% +0.05%
==========================================
Files 337 337
Lines 7418 7430 +12
Branches 1860 1901 +41
==========================================
+ Hits 6924 6939 +15
+ Misses 486 459 -27
- Partials 8 32 +24 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| ref, | ||
| () => ({ | ||
| show: () => setVisible(true), | ||
| show: () => { |
There was a problem hiding this comment.
感觉可以封装个 hook，现在这么 inline 写看起来很脏。
2 participants
fixed #6731 修复 ref.show() 调用时弹窗会关闭
Summary by CodeRabbit
show()的短时间触发链路内更稳定，避免同一点击导致的意外立即关闭。PopoverRef.show()行为相关的测试用例，覆盖点击触发可见状态下的防抖关闭、外部点击关闭以及再次打开展示内容等场景。