feat: 为弹窗组件添加动画可见性控制#7045
Conversation
- 新增 use-spring-visibility 工具函数 - 重构 CenterPopup、Mask 和 Popup 组件的动画逻辑 - 优化弹窗关闭时的回调触发时机
dosubot
Bot
added
the
size:M
|
|
|
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)
📝 WalkthroughWalkthrough引入 变更可见性感知的弹窗生命周期管理
Sequence Diagram(s)sequenceDiagram
participant Document as 浏览器标签
participant Hook as useSpringVisibility
participant Component as 弹窗组件
participant Callback as afterClose 回调
rect rgba(220, 20, 60, 0.5)
Note over Document,Callback: 页面在后台时关闭弹窗
Component->>Component: visible=false (用户/代码触发关闭)
Component->>Component: Spring 动画开始（onRest 可能不触发）
Note over Document,Component: 页面隐藏，onRest 不一定触发
end
rect rgba(34, 139, 34, 0.5)
Note over Document,Callback: 用户切回到页面
Document->>Hook: visibilitychange (hidden→visible)
Hook->>Hook: 检查 active===true && visible===false && !closed
alt 条件满足
Hook->>Component: setActive(false)
Hook->>Callback: afterClose()
Hook->>Hook: closed = true
else 跳过
Hook->>Hook: 无操作
end
Callback->>Component: 卸载组件、清理 DOM
end
预估代码审查工作量🎯 3 (中等) | ⏱️ ~20 分钟 概览PR 引入了一个新的 诗
🚥 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 unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7045 +/- ##
==========================================
+ Coverage 92.97% 93.02% +0.04%
==========================================
Files 337 338 +1
Lines 7373 7411 +38
Branches 1841 1883 +42
==========================================
+ Hits 6855 6894 +39
+ Misses 510 509 -1
Partials 8 8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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 new useSpringVisibility hook to ensure that afterClose callbacks are reliably executed, even when page visibility changes during transitions. This hook is integrated into the CenterPopup, Mask, and Popup components, refining their animation onRest logic. Review feedback suggests optimizing the hook by memoizing the shouldCallAfterClose function with useCallback and using a useRef for the visible state to prevent redundant event listener re-attachments in the useEffect block.
| @@ -0,0 +1,52 @@ | |||
| import { useIsomorphicLayoutEffect } from 'ahooks' | |||
| import type { RefObject } from 'react' | |||
| import { useEffect, useRef } from 'react' | |||
There was a problem hiding this comment.
建议导入 useCallback 以便对 Hook 返回的函数进行记忆化处理，保持引用稳定。
| import { useEffect, useRef } from 'react' | |
| import { useCallback, useEffect, useRef } from 'react' |
| useEffect(() => { | ||
| const handler = () => { | ||
| if (document.visibilityState !== 'visible') return | ||
| if (unmountedRef.current) return | ||
| if (!visible && activeRef.current && !closedRef.current) { | ||
| closedRef.current = true | ||
| setActive(false) | ||
| afterCloseRef.current?.() | ||
| } | ||
| } | ||
| document.addEventListener('visibilitychange', handler) | ||
| return () => document.removeEventListener('visibilitychange', handler) | ||
| }, [visible, setActive, unmountedRef]) |
There was a problem hiding this comment.
建议使用 useRef 来追踪 visible 的最新值，并将 visible 从 useEffect 的依赖数组中移除。这样可以避免在每次 visible 状态改变时都重新绑定和解绑 visibilitychange 事件监听器。对于全局事件监听器，保持监听函数的稳定是更好的实践，尤其是在组件频繁开关的情况下。
const visibleRef = useRef(visible)
visibleRef.current = visible
useEffect(() => {
const handler = () => {
if (document.visibilityState !== 'visible') return
if (unmountedRef.current) return
if (!visibleRef.current && activeRef.current && !closedRef.current) {
closedRef.current = true
setActive(false)
afterCloseRef.current?.()
}
}
document.addEventListener('visibilitychange', handler)
return () => document.removeEventListener('visibilitychange', handler)
}, [setActive, unmountedRef])| function shouldCallAfterClose(): boolean { | ||
| if (closedRef.current) return false | ||
| closedRef.current = true | ||
| return true | ||
| } |
There was a problem hiding this comment.
建议使用 useCallback 包裹 shouldCallAfterClose 函数。虽然目前组件中的 onRest 也是每次渲染重新创建的，但保持 Hook 返回的函数引用稳定有助于避免在 useSpring 内部产生不必要的更新逻辑。
| function shouldCallAfterClose(): boolean { | |
| if (closedRef.current) return false | |
| closedRef.current = true | |
| return true | |
| } | |
| const shouldCallAfterClose = useCallback((): boolean => { | |
| if (closedRef.current) return false | |
| closedRef.current = true | |
| return true | |
| }, []) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/utils/use-spring-visibility.ts`:
- Around line 31-43: The visibilitychange handler closes over a possibly stale
visible value; update the hook to track the latest visible in a ref (e.g.
visibleRef) and read visibleRef.current inside the handler instead of the
closed-over visible, and then re-register useEffect with only stable refs in its
dependency list (keep unmountedRef/activeRef/closedRef/afterCloseRef if they are
refs) so the listener is not needlessly reattached; ensure handler uses
visibleRef.current and calls setActive/afterCloseRef.current() as before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1204b9cb-8eba-4311-b7da-b22dcd76af86
📒 Files selected for processing (5)
src/components/center-popup/center-popup.tsxsrc/components/mask/mask.tsxsrc/components/popup/popup.tsxsrc/utils/tests/use-spring-visibility.test.tssrc/utils/use-spring-visibility.ts
dosubot
Bot
added
size:L
|
From CC: 核心思路是引入 1. 文件位置与命名
2. 只兜底了 afterClose，没兜底 afterShowPR 描述写的是「修复页面不可见时动画暂停，并且切回来之后不继续执行」，听上去是双向的，但实现只处理了 3. hook 接口可以更简洁现在同时传 4. render 期间写 ref 的赋值afterCloseRef.current = afterClose
activeRef.current = active
visibleRef.current = visible这是常见模式，能 work，但严格来说在 concurrent 模式下并不完全安全（render 可能被丢弃）。仓库其它地方也有类似写法，不算新债，但建议加一句注释说明为什么不用 useEffect 同步。 5. 测试
6. 一个小行为变化值得确认原来 7. 视觉表现visibilitychange 触发时直接 整体方向 OK，可以合，但建议至少把 1（命名/位置）、3（接口）、5（补 component 级用例 + 删未用变量）处理掉再合。2 和 6 看作者取舍。 |
2 participants
close #6815 修复页面不可见时动画暂停，并且切回来之后不继续执行
Summary by CodeRabbit
发布说明
错误修复
测试