-
Notifications
You must be signed in to change notification settings - Fork 275
refactor(overlay): 替换csstransition #3227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feat_v3.x
Are you sure you want to change the base?
Conversation
""" Walkthrough本次更改将 Overlay 组件的动画方式由原本基于 CSS 过渡(使用 Changes
Sequence Diagram(s)sequenceDiagram
participant Parent
participant Overlay
participant ReactSpring
Parent->>Overlay: 设置 visible 状态
Overlay->>ReactSpring: 调用 useSpring 动画 opacity
ReactSpring-->>Overlay: 动画完成回调 (onRest)
Overlay->>Parent: 调用 afterShow/afterClose 回调
Suggested reviewers
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/packages/overlay/overlay.tsx (1)
65-75
: 使用Spring动画替代CSS过渡使用
useSpring
实现的动画效果比CSS过渡更加灵活,但目前只实现了透明度渐变。如果原先的CSS过渡包含其他变换(如位移或缩放),这些效果会丢失。建议:
- 检查原有CSS过渡是否包含了其他动画效果
- 如有必要,将这些效果也添加到Spring配置中
另外,当前实现使用了固定的
duration
作为动画时长,这与Spring的物理模型设计理念不太一致。Spring动画通常使用tension
和friction
参数获得更自然的效果。const springProps = useSpring({ opacity: innerVisible ? 1 : 0, - config: { duration }, + config: { + tension: 280, + friction: 20, + // 仍然支持duration作为备选 + duration: props.useDuration ? duration : undefined + }, onRest: () => { if (innerVisible) { afterShow() } else { afterClose() } }, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/overlay/overlay.tsx
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (4)
src/packages/overlay/overlay.tsx (4)
53-53
: 类名逻辑变更原先的实现可能是通过
CSSTransition
动态添加类名,现在改为始终包含nut-overlay-slide
类。请确认这个变更不会对现有样式产生负面影响,特别是在不同状态下(显示/隐藏)的样式表现。
78-88
: 渲染逻辑变更使用
animated.div
替代CSSTransition
是个好的改进,但需要确保以下几点:
- 原有的CSS类在不同动画阶段的应用逻辑是否需要保留
- 确认动画完成前的点击事件处理是否一致
- 验证在不同设备和浏览器上的动画性能表现
当前实现只有在
innerVisible
为true时才渲染组件,这与原有的CSSTransition
行为可能不同。请确认这不会导致用户体验不一致。
82-82
: 样式合并方式的更改将
springProps
直接合并到样式对象中是正确的做法,但需要注意useSpring
返回的对象中可能包含不应传递给DOM的属性。
8-8
:✅ Verification successful
新增了 React Spring 依赖,用于动画效果实现。
从CSS过渡动画切换到基于Spring的动画实现是一个不错的改进,可以提供更平滑的动画效果。但请确认这个新依赖已经被添加到项目的package.json中,并考虑其对包体积的影响。
请运行以下命令验证依赖是否已正确添加:
🏁 Script executed:
#!/bin/bash grep -r "@react-spring/web" package.jsonLength of output: 75
确认 @react-spring/web 已添加
- 已在 package.json 中找到
"@react-spring/web": "~9.6.1"
- 请继续关注该依赖对打包体积的影响,必要时可使用打包分析工具进行评估
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/packages/overlay/overlay.taro.tsx (1)
54-58
:⚠️ Potential issue事件类型不匹配导致 TS2322
handleClick
参数类型为ITouchEvent
,但在animated.div
/View
上通过onClick
透传时,类型期望分别为MouseEvent
或BaseEventOrig
,CI 已报错。-const handleClick = (e: ITouchEvent) => { +// 兼容 H5 & 小程序事件类型 +const handleClick = (e: any) => {或为
onClick
显式断言:onClick={handleClick as unknown as React.MouseEventHandler<HTMLDivElement>}如最终保留
View
,可改为(e: any)
并在实现内部做 run-time 判断。🧰 Tools
🪛 Biome (1.9.4)
[error] 56-56: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🧹 Nitpick comments (2)
src/packages/overlay/overlay.taro.tsx (2)
60-71
:useSpring
配置与生命周期回调耦合过紧
config: { duration }
会覆盖默认的缓动函数,导致线性动画;若想保持与旧版一致的缓动效果,可使用config: { duration, easing: easings.easeInOutCubic }
(或保持默认张弛动画,去掉duration
)。onRest
中依据innerVisible
判定回调存在竞态:快速点击显示→隐藏可能导致afterShow/afterClose
顺序颠倒。可采用useEffect
监听innerVisible
并在spring.start(() => …)
的回调里分支判断,降低竞态概率。
48-48
: 类名中保留-slide
修饰符可能产生歧义现已改用弹簧渐隐动画,但仍附带
nut-overlay-slide
类;若样式文件中存在与滑动相关的 transform,将产生不必要的样式冲突。考虑改为nut-overlay-fade
或去掉该修饰符。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/overlay/overlay.taro.tsx
(4 hunks)
🧰 Additional context used
🪛 GitHub Check: build
src/packages/overlay/overlay.taro.tsx
[failure] 79-79:
Type '(e: ITouchEvent) => void' is not assignable to type 'MouseEventHandler'.
🪛 GitHub Actions: CI
src/packages/overlay/overlay.taro.tsx
[error] 79-79: TypeScript error TS2322: Type '(e: ITouchEvent) => void' is not assignable to type 'MouseEventHandler'.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
暂时还不能替换~ 无法支持其他端 |
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit