fix: Continue timing when mouse moves away#383
fix: Continue timing when mouse moves away#383luozz1994 wants to merge 2 commits intoreact-component:masterfrom
Conversation
|
Someone is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
Walkthrough添加全局鼠标位置追踪并在堆栈模式下基于元素边界计算鼠标下的通知，从而更新悬停状态；同时为堆栈通知添加两个测试用例，验证悬停与计时器交互行为。 Changes
Sequence Diagram(s)（无须生成序列图） Estimated code review effort🎯 3 (中等复杂度) | ⏱️ ~20 分钟 Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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, 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! 此拉取请求旨在解决一个用户体验问题：在堆叠通知模式下，当用户关闭一个鼠标悬停的通知时，即使鼠标已移开新出现的顶部通知区域，该通知的自动关闭倒计时仍会意外暂停。通过在通知列表结构发生变化时，利用全局鼠标位置信息重新计算并更新通知的悬停状态，确保了倒计时逻辑能够正确地恢复，从而提升了通知组件的响应性和用户交互的流畅性。 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. Changelog
Activity
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
|
![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
这个 Pull Request 旨在修复一个在堆叠通知中关闭一个通知后，其余通知的计时器没有正确恢复的问题。修复方案通过跟踪全局鼠标位置，并在通知列表变化时重新计算鼠标悬停的通知，这是合理的。实现上使用了 useEffect 和 requestAnimationFrame，这是处理 DOM 更新后逻辑的良好实践。同时，PR 也添加了相应的单元测试来覆盖修复的场景，这非常好。
我发现了一个小问题，在 src/NoticeList.tsx 中，useEffect hook 内部使用了可能过期的状态值，这可能会导致潜在的 bug。我已经在代码中提出了具体的修改建议。除此之外，整体修改看起来很不错。
![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/NoticeList.tsx (1)
87-119: 建议使用函数式更新模式避免潜在的闭包陈旧问题。第 113 行的条件判断使用了闭包中的
hoverKeys，但依赖数组[keys, stack]不包含hoverKeys。这可能导致hoverKeys值陈旧，在某些边界情况下跳过必要的状态更新。建议使用函数式更新，同时进行数组比较以避免不必要的重渲染：
♻️ 建议的优化
// Only update if there's a change to avoid unnecessary re-renders - if (newHoverKeys.length > 0 || hoverKeys.length > 0) { - setHoverKeys(newHoverKeys); - } + setHoverKeys((prev) => { + // Skip update if both are empty + if (newHoverKeys.length === 0 && prev.length === 0) { + return prev; + } + // Skip update if arrays are identical + if ( + newHoverKeys.length === prev.length && + newHoverKeys.every((key, i) => key === prev[i]) + ) { + return prev; + } + return newHoverKeys; + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/NoticeList.tsx` around lines 87 - 119, The effect that computes hover keys uses the closed-over hoverKeys variable but does not list it in the dependency array (useEffect(...) with dependencies [keys, stack]), which can cause stale reads and skipped updates; change the setHoverKeys call inside the requestAnimationFrame callback to use the functional updater form (setHoverKeys(prev => { compare prev to newHoverKeys with an array equality check and return prev if identical, otherwise return newHoverKeys })) and ensure you reference mousePositionRef, dictRef and keys as you already do so the state update is correct and avoids unnecessary re-renders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/NoticeList.tsx`:
- Around line 87-119: The effect that computes hover keys uses the closed-over
hoverKeys variable but does not list it in the dependency array (useEffect(...)
with dependencies [keys, stack]), which can cause stale reads and skipped
updates; change the setHoverKeys call inside the requestAnimationFrame callback
to use the functional updater form (setHoverKeys(prev => { compare prev to
newHoverKeys with an array equality check and return prev if identical,
otherwise return newHoverKeys })) and ensure you reference mousePositionRef,
dictRef and keys as you already do so the state update is correct and avoids
unnecessary re-renders.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/NoticeList.tsx`:
- Around line 96-109: The hit-testing currently collects all elements whose
bounding rect contains the mouse, which causes multiple overlapping notices to
be marked hovered and leaves stale keys; change the logic to only keep the
topmost hit: use document.elementFromPoint(mousePos.x, mousePos.y) (or otherwise
pick the last/highest z-order element) and map that single element back to your
dictRef entries (keys in dictRef.current) to push only that key into
newHoverKeys so only the topmost notice becomes hovered (adjust any
mapping/attribute lookup as needed and remove the multi-push behavior in the
block using getBoundingClientRect).
- Around line 78-83: The cleanup currently only runs when stack &&
hoverKeys.length > 1, which misses the case where a single hover key becomes
invalid; update the logic in the same block that calls setHoverKeys so it always
filters invalid keys (remove the hoverKeys.length > 1 guard or change it to run
unconditionally when stack is truthy), using the existing setHoverKeys((prev) =>
{ const filtered = prev.filter((key) => keys.some(({ key: dataKey }) => key ===
dataKey)); return filtered.length === prev.length ? prev : filtered; }) pattern
to ensure a removed single key clears and stops hovering/timers; keep the same
filtered comparison to avoid unnecessary re-renders.
1 participant
问题描述
notification 关闭首个通知后，下方通知上移，鼠标移开时倒计时不自动继续
复现步骤
关联Antd-Design Issue
ant-design/ant-design#57096
修复方案
当通知列表发生变化时，检查鼠标是否仍在通知区域内
Summary by CodeRabbit
发布说明
新功能
测试