feat: add safe hover polygon handling#625
Open
li-jia-nan wants to merge 3 commits into
Open
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #625 +/- ##
==========================================
+ Coverage 97.28% 97.54% +0.26%
==========================================
Files 17 18 +1
Lines 956 1100 +144
Branches 268 313 +45
==========================================
+ Hits 930 1073 +143
- Misses 26 27 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
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 (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
Walkthrough新增“安全悬停区域”功能：通过几何多边形判定鼠标在目标与弹层之间的安全走廊，Trigger 在离开时可注册文档级监听以延迟或阻止弹层关闭，并配套单元/集成测试与示例文档。 ChangesSafe Hover Area Detection Feature
Sequence DiagramsequenceDiagram
participant Trigger as Trigger Component
participant Document as Document
participant SafeHover as isPointInSafeHoverArea
Trigger->>Trigger: onMouseLeave (leavePoint)
Trigger->>SafeHover: isPointInSafeHoverArea(leavePoint, targetRect, popupRect)
alt 在安全区域
Trigger->>Document: register mousemove/pointermove monitor
Document->>SafeHover: check pointer vs polygons
alt 仍在安全区域
Document->>Trigger: defer close until mouseLeaveDelay
else 离开安全区域
Document->>Trigger: clearSafeHover() then close popup
end
else 不在安全区域
Trigger->>Trigger: 直接关闭 popup
end
Trigger->>Trigger: onPopupMouseEnter -> clearSafeHover()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
docs/examples/safe-hover.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/util/safeHover.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. typings.d.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. 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}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a safe hover area feature to prevent popups from closing prematurely when the cursor moves between the target element and the popup. It adds a new utility file src/safeHover.ts to calculate the safe polygon area, integrates event listeners in src/index.tsx to monitor mouse movements, and includes corresponding unit tests. Feedback on the changes highlights a critical bug where the popup closes prematurely upon re-entering the target or popup because the mousemove listener is not stopped, and suggests simplifying nested ternaries in the safe hover calculations to improve readability.
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/index.tsx (2)
527-533: 💤 Low value两个分支都调用
triggerOpen(false, mouseLeaveDelay)的设计值得关注。当前逻辑：
- 在安全区域内：持续重置关闭计时器，保持弹层显示
- 离开安全区域：停止监控并重新触发延迟关闭
这个模式可以正常工作，但离开安全区域时重置计时器意味着用户需要额外等待
mouseLeaveDelay才能关闭弹层。如果期望离开安全区域后立即（或更快）关闭，可考虑使用更短的延迟或直接触发关闭。🤖 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/index.tsx` around lines 527 - 533, The code calls triggerOpen(false, mouseLeaveDelay) in both branches, causing an unnecessary full mouseLeaveDelay when leaving the safe area; update the else branch that uses stopSafeHoverMonitor() to call triggerOpen with a shorter delay or immediate close (e.g., triggerOpen(false, 0) or a new constant like immediateCloseDelay) instead of mouseLeaveDelay so the popup closes faster when nextInSafeArea is false; locate the logic around nextInSafeArea, stopSafeHoverMonitor, triggerOpen, and mouseLeaveDelay in src/index.tsx and change only the else branch call to use the faster/zero delay.
494-501: 💤 Low value当
mouseLeaveDelay为 0 时，安全悬停监控将被静默禁用。由于条件
!mouseLeaveDelay包含mouseLeaveDelay === 0的情况，用户设置mouseLeaveDelay={0}时 safe hover 功能不会生效。如果这是预期行为，建议添加注释说明；否则应改为显式的undefined检查。🤖 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/index.tsx` around lines 494 - 501, The guard incorrectly treats mouseLeaveDelay === 0 as falsy and disables safe hover; in the conditional inside the function that checks currentTargetEle, currentPopupEle, openRef.current, and mouseLeaveDelay, replace the falsy check "!mouseLeaveDelay" with an explicit undefined check (e.g. "mouseLeaveDelay === undefined") so mouseLeaveDelay=0 remains valid (or if 0 should disable behavior, add a clarifying comment next to the condition); update the condition that references currentTargetEle/currentPopupEle/openRef.current accordingly.src/safeHover.ts (2)
145-165: ⚡ Quick win缺少对
target和popup元素的空值校验。
getRect内部调用element.getBoundingClientRect()，若target或popup为null/undefined会抛出运行时异常。虽然调用方应保证元素存在，但在此处添加防御性检查可提高鲁棒性。💡 建议的修复
export function isInSafeHoverArea(options: SafeHoverAreaOptions) { const { clientX, clientY, originX, originY, target, popup, align, buffer = 0.5, } = options; + if (!target || !popup) { + return false; + } + const clientPoint: Point = [clientX, clientY]; const refRect = getRect(target); const popupRect = getRect(popup);🤖 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/safeHover.ts` around lines 145 - 165, In isInSafeHoverArea, add defensive null/undefined checks for the DOM elements before calling getRect: verify that options.target and options.popup are non-null (or throw a clear error) and if either is missing return false early to avoid getRect calling element.getBoundingClientRect() on null; update the start of isInSafeHoverArea (before calling getRect on target/popup) to perform these checks and document the behavior so callers know missing elements result in a safe "not inside" result.
231-377: ⚖️ Poor tradeoff
getPolygon函数嵌套三元运算符过深，可读性和可维护性较差。当前每个
case分支内使用多层嵌套三元表达式计算多边形顶点，建议拆分为独立的辅助函数或使用if-else语句提升可读性。🤖 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/safeHover.ts` around lines 231 - 377, The getPolygon function uses deeply nested ternaries per side ('top','bottom','left','right'), hurting readability; refactor by extracting a helper per side (e.g., computeTopPolygon, computeBottomPolygon, computeLeftPolygon, computeRightPolygon) that accepts the same inputs (Point, buffer, popupRect and flags like isFloatingWider/isFloatingTaller/cursorLeaveFromRight/cursorLeaveFromBottom) and computes named intermediate values via if/else (e.g., anchorX, anchorY, edgeY/edgeX) instead of nested ?:, then have getPolygon simply switch on side and return the corresponding helper result—this keeps behavior identical but replaces nested ternaries with clear, testable logic located around those helper function names.
🤖 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/index.tsx`:
- Around line 527-533: The code calls triggerOpen(false, mouseLeaveDelay) in
both branches, causing an unnecessary full mouseLeaveDelay when leaving the safe
area; update the else branch that uses stopSafeHoverMonitor() to call
triggerOpen with a shorter delay or immediate close (e.g., triggerOpen(false, 0)
or a new constant like immediateCloseDelay) instead of mouseLeaveDelay so the
popup closes faster when nextInSafeArea is false; locate the logic around
nextInSafeArea, stopSafeHoverMonitor, triggerOpen, and mouseLeaveDelay in
src/index.tsx and change only the else branch call to use the faster/zero delay.
- Around line 494-501: The guard incorrectly treats mouseLeaveDelay === 0 as
falsy and disables safe hover; in the conditional inside the function that
checks currentTargetEle, currentPopupEle, openRef.current, and mouseLeaveDelay,
replace the falsy check "!mouseLeaveDelay" with an explicit undefined check
(e.g. "mouseLeaveDelay === undefined") so mouseLeaveDelay=0 remains valid (or if
0 should disable behavior, add a clarifying comment next to the condition);
update the condition that references
currentTargetEle/currentPopupEle/openRef.current accordingly.
In `@src/safeHover.ts`:
- Around line 145-165: In isInSafeHoverArea, add defensive null/undefined checks
for the DOM elements before calling getRect: verify that options.target and
options.popup are non-null (or throw a clear error) and if either is missing
return false early to avoid getRect calling element.getBoundingClientRect() on
null; update the start of isInSafeHoverArea (before calling getRect on
target/popup) to perform these checks and document the behavior so callers know
missing elements result in a safe "not inside" result.
- Around line 231-377: The getPolygon function uses deeply nested ternaries per
side ('top','bottom','left','right'), hurting readability; refactor by
extracting a helper per side (e.g., computeTopPolygon, computeBottomPolygon,
computeLeftPolygon, computeRightPolygon) that accepts the same inputs (Point,
buffer, popupRect and flags like
isFloatingWider/isFloatingTaller/cursorLeaveFromRight/cursorLeaveFromBottom) and
computes named intermediate values via if/else (e.g., anchorX, anchorY,
edgeY/edgeX) instead of nested ?:, then have getPolygon simply switch on side
and return the corresponding helper result—this keeps behavior identical but
replaces nested ternaries with clear, testable logic located around those helper
function names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 334bae19-24f8-49d4-8b58-a1207526ec5e
📒 Files selected for processing (4)
package.jsonsrc/index.tsxsrc/safeHover.tstests/basic.test.jsx
There was a problem hiding this comment.
Pull request overview
This PR adds “safe hover polygon” logic to keep hover-triggered popups open while the cursor moves from the trigger toward the popup, and includes a regression test for the new hover-close behavior.
Changes:
- Add a new
isInSafeHoverAreautility to compute whether the cursor remains within a safe hover region between trigger and popup. - Start/stop a document-level pointer tracking monitor on hover leave to refresh
mouseLeaveDelaywhile the cursor remains in the safe region. - Add a test validating that
mouseLeaveDelayis refreshed while moving through the safe hover area, and bump@rc-component/utilminimum version.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/basic.test.jsx | Adds a test to verify safe-hover keeps popup open while moving through the safe area. |
| src/safeHover.ts | Introduces safe hover polygon/rectangle computations (isInSafeHoverArea). |
| src/index.tsx | Hooks safe-hover monitoring into hover leave/enter lifecycle to refresh close delay. |
| package.json | Raises the minimum required version of @rc-component/util. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/index.tsx (1)
505-522:⚠️ Potential issue | 🟠 Major | ⚡ Quick win修复 safe-hover 原点坐标传参错误（
originY被误传为originX）这里把
originY传成了originX，会让isInSafeHoverArea的几何判断丢失垂直方向信息；同时这也是当前 CI 报错的直接原因（originY未使用）。🔧 建议修复
const originX = event.clientX; const originY = event.clientY; const onMouseMove = (mouseEvent: MouseEvent) => { @@ const inSafeHoverArea = isInSafeHoverArea({ clientX: mouseEvent.clientX, clientY: mouseEvent.clientY, originX: originX, - originY: originX, + originY: originY, target: nextTargetEle, popup: nextPopupEle, align: alignInfo, });🤖 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/index.tsx` around lines 505 - 522, The safe-hover origin Y coordinate is passed incorrectly in the onMouseMove handler: originY was passed as originX to isInSafeHoverArea, causing vertical checks to be lost; update the isInSafeHoverArea call inside onMouseMove to pass originY for the originY parameter (keep originX as-is for originX), referencing the variables originX and originY and the isInSafeHoverArea invocation to locate the change.Source: Pipeline failures
🤖 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.
Outside diff comments:
In `@src/index.tsx`:
- Around line 505-522: The safe-hover origin Y coordinate is passed incorrectly
in the onMouseMove handler: originY was passed as originX to isInSafeHoverArea,
causing vertical checks to be lost; update the isInSafeHoverArea call inside
onMouseMove to pass originY for the originY parameter (keep originX as-is for
originX), referencing the variables originX and originY and the
isInSafeHoverArea invocation to locate the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d023304d-57e0-4505-b4d0-eeea2c5d601c
📒 Files selected for processing (3)
src/index.tsxsrc/safeHover.tstests/safeHover.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/safeHover.ts
li-jia-nan
force-pushed
the
feat-safe-hover-polygon
branch
from
b5c7026 to
97703ff
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/util/safeHover.ts (1)
83-90: 💤 Low value变量命名可以更清晰。
verticalRect和horizontalRect的命名容易引起误解 —verticalRect实际用于获取水平方向的边界（left/right），而horizontalRect用于获取垂直方向的边界（top/bottom）。考虑重命名为narrowerWidthRect和narrowerHeightRect以更准确地表达其用途。♻️ 建议的重命名
- const verticalRect = + const narrowerWidthRect = popupRect.width > targetRect.width ? targetRect : popupRect; - const horizontalRect = + const narrowerHeightRect = popupRect.height > targetRect.height ? targetRect : popupRect; - const left = verticalRect.left - buffer; - const right = verticalRect.right + buffer; - const top = horizontalRect.top - buffer; - const bottom = horizontalRect.bottom + buffer; + const left = narrowerWidthRect.left - buffer; + const right = narrowerWidthRect.right + buffer; + const top = narrowerHeightRect.top - buffer; + const bottom = narrowerHeightRect.bottom + buffer;🤖 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/util/safeHover.ts` around lines 83 - 90, The variables verticalRect and horizontalRect are misleading because verticalRect is used to compute horizontal bounds (left/right) and horizontalRect is used for vertical bounds (top/bottom); rename verticalRect to narrowerWidthRect and horizontalRect to narrowerHeightRect and update all references (left, right, top, bottom) where popupRect, targetRect and buffer are used so the names clearly reflect "narrower width" and "narrower height" selection logic in the block that computes left = ... right = ... top = ... bottom = ....tests/basic.test.jsx (1)
139-148: 💤 Low value
mockRect辅助函数可以考虑在测试后恢复原始方法。虽然每个测试都创建新元素，当前实现不会导致测试间污染，但作为最佳实践，可以考虑保存并恢复原始的
getBoundingClientRect：🔄 可选的改进方案
function mockRect(element, rect) { + const original = element.getBoundingClientRect; element.getBoundingClientRect = jest.fn(() => ({ left: rect.left, top: rect.top, width: rect.width, height: rect.height, right: rect.left + rect.width, bottom: rect.top + rect.height, })); + return () => { + element.getBoundingClientRect = original; + }; }不过考虑到当前测试结构（每个测试使用独立的 DOM 元素且有
cleanup()），这只是一个可选的防御性改进。🤖 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 `@tests/basic.test.jsx` around lines 139 - 148, The helper mockRect overwrites element.getBoundingClientRect without restoring it; change mockRect (and tests that use it) to save the original method (e.g., const original = element.getBoundingClientRect) before replacing it and provide a restore step after the test (either return a restore function from mockRect or call the saved original in an afterEach/cleanup), ensuring the original getBoundingClientRect is reattached to the element; reference: mockRect function and tests that call it.tests/safeHover.test.ts (1)
7-21: 💤 Low value考虑为
rect辅助函数添加类型注解。虽然 TypeScript 可以推断返回类型，但为参数和返回值显式添加类型注解能提升可读性和类型安全性：
-function rect( - left: number, - top: number, - width: number, - height: number, -): SafeHoverRect { +function rect( + left: number, + top: number, + width: number, + height: number, +): SafeHoverRect {实际上当前代码已经有返回类型注解，这很好。不过可以考虑为
rect对象字面量添加类型断言以获得更早的类型检查。🤖 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 `@tests/safeHover.test.ts` around lines 7 - 21, The rect helper already has parameter and return type annotations but the returned object literal should be asserted as SafeHoverRect to enable earlier shape checking; update the return statement in the rect function (the object literal) to include a type assertion to SafeHoverRect (e.g., assert the literal as SafeHoverRect) so TypeScript validates the object properties against SafeHoverRect at the return site while keeping the existing function signature.
🤖 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/util/safeHover.ts`:
- Around line 83-90: The variables verticalRect and horizontalRect are
misleading because verticalRect is used to compute horizontal bounds
(left/right) and horizontalRect is used for vertical bounds (top/bottom); rename
verticalRect to narrowerWidthRect and horizontalRect to narrowerHeightRect and
update all references (left, right, top, bottom) where popupRect, targetRect and
buffer are used so the names clearly reflect "narrower width" and "narrower
height" selection logic in the block that computes left = ... right = ... top =
... bottom = ....
In `@tests/basic.test.jsx`:
- Around line 139-148: The helper mockRect overwrites
element.getBoundingClientRect without restoring it; change mockRect (and tests
that use it) to save the original method (e.g., const original =
element.getBoundingClientRect) before replacing it and provide a restore step
after the test (either return a restore function from mockRect or call the saved
original in an afterEach/cleanup), ensuring the original getBoundingClientRect
is reattached to the element; reference: mockRect function and tests that call
it.
In `@tests/safeHover.test.ts`:
- Around line 7-21: The rect helper already has parameter and return type
annotations but the returned object literal should be asserted as SafeHoverRect
to enable earlier shape checking; update the return statement in the rect
function (the object literal) to include a type assertion to SafeHoverRect
(e.g., assert the literal as SafeHoverRect) so TypeScript validates the object
properties against SafeHoverRect at the return site while keeping the existing
function signature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 224ec2fe-f413-4eb0-9a74-ffedb11d5980
📒 Files selected for processing (6)
docs/demos/safe-hover.mddocs/examples/safe-hover.tsxsrc/index.tsxsrc/util/safeHover.tstests/basic.test.jsxtests/safeHover.test.ts
✅ Files skipped from review due to trivial changes (1)
- docs/demos/safe-hover.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Kapture.2026-06-08.at.01.55.05.mp4
Summary by CodeRabbit
新功能
文档
测试
其他