fix(Image): Image mask fade enter blur transition does not work #492
Conversation
|
@zenggpzqbx is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
Walkthrough将 Preview 的动画边界与 DOM 结构重组：根容器变为静态外层，遮罩与主体分别由两个 Changes
Sequence Diagram(s)sequenceDiagram
participant App as 使用方
participant Portal as Portal
participant Root as 外层 RootDiv
participant MotionMask as CSSMotion(mask)
participant MotionBody as CSSMotion(body)
participant Body as Body/Close/PrevNext/Footer
Note over App,Portal: 用户或外部切换 preview open 且 portalRender = true
App->>Portal: render preview (portalRender)
Portal->>Root: 挂载外层 root div（应用 zIndex/基础样式）
alt portalRender && open
Root->>MotionMask: mount 遮罩（可点击关闭）
MotionMask-->>Root: 遮罩动效/可交互
Root->>MotionBody: mount 主体子树（动效边界内）
MotionBody->>Body: 渲染 Body / Close / PrevNext / Footer
Body-->>MotionBody: Footer 接收 showProgress 及交互回调
else not open
Root-xMotionMask: 不渲染遮罩
Root-xMotionBody: 不渲染主体
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/Preview/index.tsx (5)
🔇 Additional comments (4)
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 @zenggpzqbx, 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! 这个拉取请求旨在修复 Ant Design 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. 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 旨在修复 Image 组件在预览模式下，遮罩层的淡入模糊过渡动画在非首次加载时失效的问题。
你提出的修改方案非常简洁且有效。通过将 Portal 组件的 open 属性从 portalRender 修改为 portalRender && open，确保了 Portal 的生命周期与预览的 open 状态正确绑定。这样，当预览关闭时，Portal 及其所有子组件（包括 CSSMotion）都会被正确卸载。当预览再次打开时，它们会被重新挂载，从而能够可靠地触发 CSSMotion 的入场动画。
这个解决方案是声明式的，并且比在回调函数中重置状态的替代方案更清晰，因为它将逻辑控制在组件的渲染方法内部，降低了组件间的耦合。
整体来看，这是一个高质量的修复，代码清晰，解决了根本问题。做得很好！
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #492 +/- ##
=======================================
Coverage 99.41% 99.41%
=======================================
Files 17 17
Lines 509 510 +1
Branches 152 153 +1
=======================================
+ Hits 506 507 +1
Misses 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (2)
src/Preview/index.tsx (2)
482-483: 格式问题：分号位置不规范
return语句的结束部分格式有些奇怪，分号单独占一行。建议调整为更常规的格式。🔎 建议修复
- ) - ; + );
412-415: 冗余的包装可以简化当只有一个类名时，
clsx(motionClassName)是多余的；{{ ...motionStyle }}的展开也创建了不必要的对象。🔎 建议简化
<div - className={clsx(motionClassName)} - style={{ ...motionStyle }} + className={motionClassName} + style={motionStyle} >
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Preview/index.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/Preview/index.tsx (5)
docs/examples/actionsRender.tsx (2)
imageRender(55-57)actionsRender(58-60)docs/examples/imageRender.tsx (1)
imageRender(6-27)src/Preview/CloseBtn.tsx (1)
CloseBtn(9-17)src/Preview/PrevNext.tsx (1)
PrevNext(13-44)src/Preview/Footer.tsx (1)
Footer(46-212)
🔇 Additional comments (2)
src/Preview/index.tsx (2)
402-443: 结构重构合理，符合修复目标将主体内容（body、关闭按钮、PrevNext）移入
CSSMotion内部是正确的做法。这样可以确保：
- 内容在关闭时触发离场动画
- 动画完成后组件被销毁，从而解决后续加载时模糊过渡失效的问题
visible={portalRender && open}的条件也能正确控制动画的触发时机。
446-475: Footer 组件属性传递正确
showProgress和showSwitch属性的传递与Footer.tsx中的使用方式一致，所有回调函数和样式属性也都正确传递。
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/Preview/index.tsx (1)
396-401: 遮罩层仍然缺少退出动画遮罩层通过
{open && ...}条件渲染，位于CSSMotion外部。当open变为false时，遮罩 DOM 元素会立即被销毁，无法执行淡出动画，而内容区域（Body/CloseBtn/PrevNext/Footer）通过CSSMotion会有平滑的退出过渡。这导致视觉不一致：内容淡出但遮罩瞬间消失。这与之前的评审意见中指出的问题相同。建议将遮罩层也包裹在
CSSMotion中，或为其创建单独的CSSMotion实例，确保遮罩和内容同步进行进入/退出动画。🔎 建议的修复方案
将遮罩移入 CSSMotion 渲染函数内部，使其与内容同步动画：
- {/* Mask */} - {open && (<div - className={clsx(`${prefixCls}-mask`, classNames.mask)} - style={styles.mask} - onClick={onClose} - /> - )} <CSSMotion motionName={motionName} visible={portalRender && open} motionAppear motionEnter motionLeave onVisibleChanged={onVisibleChanged} > {({ className: motionClassName, style: motionStyle }) => { return ( <div className={clsx(motionClassName)} style={{ ...motionStyle }} > + {/* Mask */} + <div + className={clsx(`${prefixCls}-mask`, classNames.mask)} + style={styles.mask} + onClick={onClose} + /> {/* Body */} <div className={clsx(`${prefixCls}-body`, classNames.body)} style={bodyStyle}>基于之前的评审意见。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Preview/index.tsxtests/controlled.test.tsxtests/previewGroup.test.tsx
✅ Files skipped from review due to trivial changes (1)
- tests/previewGroup.test.tsx
🔇 Additional comments (3)
src/Preview/index.tsx (3)
392-394: 根容器结构合理新的根 div 容器正确处理了基础样式、类名和 zIndex，为遮罩层和内容提供了清晰的承载边界。
402-409: CSSMotion 配置正确
CSSMotion组件的配置合理：
visible={portalRender && open}正确控制可见性motionAppear、motionEnter、motionLeave均已启用，支持完整的动画生命周期onVisibleChanged回调正确处理可见性变化
446-475: Footer 组件的 props 接口类型完整且正确已验证
FooterProps接口在src/Preview/Footer.tsx:26中的定义。该接口继承Actions接口，并包含所有传入的 props：
showProgress正确定义为boolean类型（第 28 行）- 所有操作处理器（
onFlipY、onFlipX、onRotateLeft等）通过Actions接口完整覆盖- 其他 props 如
image、transform、scale、minScale、maxScale等均已正确声明无类型安全问题。
| }); | ||
|
|
||
| expect(document.querySelector('.rc-image-preview')).toBeFalsy(); | ||
| expect(document.querySelector('.rc-image-preview')).toBeTruthy(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the Preview component's Portal and CSSMotion configuration
rg -n "Portal|CSSMotion" src/Preview/index.tsx -B 3 -A 3Repository: react-component/image
Length of output: 1595
🏁 Script executed:
# Check the test file context to understand what's being tested
head -50 tests/controlled.test.tsxRepository: react-component/image
Length of output: 1358
🏁 Script executed:
# Look at the specific test around line 34
sed -n '25,45p' tests/controlled.test.tsxRepository: react-component/image
Length of output: 611
🏁 Script executed:
# Check if there's cleanup logic for portalRender
rg -n "setPortalRender" src/Preview/index.tsx -B 2 -A 2Repository: react-component/image
Length of output: 313
🏁 Script executed:
# Check Portal documentation or types to understand its behavior
rg -n "autoDestroy|removeOnLeave|unmountOnExit" src/Preview/index.tsxRepository: react-component/image
Length of output: 47
🏁 Script executed:
# Look at the complete useLayoutEffect hook for Portal setup
sed -n '375,390p' src/Preview/index.tsxRepository: react-component/image
Length of output: 530
🏁 Script executed:
# Check if there's a cleanup effect that sets portalRender to false
sed -n '375,410p' src/Preview/index.tsxRepository: react-component/image
Length of output: 1281
🏁 Script executed:
# Check if there's a cleanup effect that should set portalRender to false
rg -n "useLayoutEffect|useEffect" src/Preview/index.tsx | head -20Repository: react-component/image
Length of output: 348
🏁 Script executed:
# Look for any other places where portalRender might be reset
rg -n "portalRender" src/Preview/index.tsxRepository: react-component/image
Length of output: 259
🏁 Script executed:
# Check if there are any comments or logic around the Portal behavior
sed -n '370,420p' src/Preview/index.tsxRepository: react-component/image
Length of output: 1702
🏁 Script executed:
# Check what onVisibleChanged does in the component
rg -n "onVisibleChanged" src/Preview/index.tsx -B 5 -A 5Repository: react-component/image
Length of output: 730
🏁 Script executed:
# Look for the onVisibleChanged implementation
sed -n '350,375p' src/Preview/index.tsxRepository: react-component/image
Length of output: 676
🏁 Script executed:
# Check if there's any logic in onVisibleChanged that should clean up portalRender
rg -n "const onVisibleChanged" src/Preview/index.tsx -A 15Repository: react-component/image
Length of output: 566
🏁 Script executed:
# Check the full context around the Portal and CSSMotion to understand complete flow
sed -n '375,450p' src/Preview/index.tsxRepository: react-component/image
Length of output: 2687
🏁 Script executed:
# Check if Portal component unmounts content when open={false} even if portalRender is true
rg -n "Portal" src/Preview/index.tsx -A 1 | head -20Repository: react-component/image
Length of output: 601
🏁 Script executed:
# Look at the test again to see what was the previous expectation
git log --oneline -20 tests/controlled.test.tsx 2>/dev/null || echo "Git not available, checking for version history"Repository: react-component/image
Length of output: 136
🏁 Script executed:
# Check if Portal has any mechanism to unmount when its open prop is false
rg -n "portal" src/ -i | grep -i "destroy\|unmount\|remove" | head -10Repository: react-component/image
Length of output: 47
🏁 Script executed:
# Look at the git diff or commit message to understand the intent
git show --stat 7150f88 2>/dev/null | head -30 || echo "Can't show commit"Repository: react-component/image
Length of output: 1505
🏁 Script executed:
# Check if there's any issue or comment about this design decision
rg -n "portalRender\|Portal.*open\|root.*always\|element.*mount" src/Preview/index.tsx -B 2 -A 2Repository: react-component/image
Length of output: 47
🏁 Script executed:
# Look at what the Portal component type expects
rg -n "type PortalProps\|interface PortalProps" -A 10Repository: react-component/image
Length of output: 47
🏁 Script executed:
# Check if there's a cleanup effect that should reset portalRender
sed -n '376,385p' src/Preview/index.tsxRepository: react-component/image
Length of output: 344
🏁 Script executed:
# Look at the entire useLayoutEffect hook to see if there's cleanup logic
ast-grep --pattern $'useLayoutEffect(() => {
$$$
}, [$$$])'Repository: react-component/image
Length of output: 285
🏁 Script executed:
# Check the actual behavior: does Portal unmount its content when open={false}?
rg -n "open.*false\|Portal.*behavior" . -i | head -10Repository: react-component/image
Length of output: 47
🏁 Script executed:
# Check if there's a second useEffect that should clean up portalRender
sed -n '375,500p' src/Preview/index.tsx | grep -A 20 "useLayoutEffect\|useEffect"Repository: react-component/image
Length of output: 784
🏁 Script executed:
# Check the complete useLayoutEffect and surrounding code more carefully
sed -n '362,385p' src/Preview/index.tsxRepository: react-component/image
Length of output: 655
🏁 Script executed:
# Check if Portal component from @rc-component/portal unmounts when open={false}
web_search @rc-component/portal open property behavior unmountRepository: react-component/image
Length of output: 115
🏁 Script executed:
# Look at the test again - what was the original expectation before this change?
git log --all --oneline tests/controlled.test.tsx 2>/dev/null | head -5Repository: react-component/image
Length of output: 310
🏁 Script executed:
# Check if there are other tests that verify the unmount behavior
rg -n "toBeFalsy\|toBeTruthy" tests/controlled.test.tsxRepository: react-component/image
Length of output: 47
🏁 Script executed:
# Check the git diff for this specific commit to see what changed
git show 7150f88 -- src/Preview/index.tsx | head -100Repository: react-component/image
Length of output: 3137
🏁 Script executed:
# Check if there's a second effect that should clean up portalRender
sed -n '375,385p' src/Preview/index.tsx | cat -ARepository: react-component/image
Length of output: 421
🏁 Script executed:
# Look at the entire file around Portal to see if there's cleanup logic we missed
sed -n '375,390p' src/Preview/index.tsxRepository: react-component/image
Length of output: 530
🏁 Script executed:
# Check if the previous commit (502ca38) has the Portal unmount logic
git show 502ca38:src/Preview/index.tsx 2>/dev/null | grep -A 10 "Portal\|portalRender" | head -30Repository: react-component/image
Length of output: 1132
🏁 Script executed:
# Check all useEffect/useLayoutEffect hooks in the Preview component
rg -n "useEffect|useLayoutEffect" src/Preview/index.tsx -A 8Repository: react-component/image
Length of output: 1926
🏁 Script executed:
# Verify the exact behavior - check if Portal.open controls unmounting
rg -n "Portal.*open\|open.*Portal" src/Preview/index.tsx -B 2 -A 2Repository: react-component/image
Length of output: 47
🏁 Script executed:
# Check if onVisibleChanged should be resetting portalRender
sed -n '368,410p' src/Preview/index.tsxRepository: react-component/image
Length of output: 1436
🏁 Script executed:
# Check the previous version to see what the old Portal behavior was
git show 502ca38:src/Preview/index.tsx | sed -n '200,250p'Repository: react-component/image
Length of output: 1179
🏁 Script executed:
# Look at the old test expectation before the change
git show 502ca38:tests/controlled.test.tsx | sed -n '30,40p'Repository: react-component/image
Length of output: 372
🌐 Web query:
@rc-component/portal open property behavior documentation
💡 Result:
Summary — @rc-component/portal "open" prop
- Type: boolean.
- Behavior: when open={true} the Portal mounts/renders its children into the portal container; when open={false} the portal content is not shown.
- Default: false.
- Related props: getContainer (customize target container; default document.body) and autoLock (locks page scroll when open).
- Example: Hello World.
Source: package README/API. [1]
补充缺失的清理逻辑以正确卸载 Portal
useLayoutEffect 中缺少当 open 变为 false 时重置 portalRender 的逻辑。当前代码只在 open 为 true 时设置 portalRender = true，但从未重置为 false，导致 Portal 及其内容永久保留在 DOM 中。这会在重复打开/关闭预览的场景中造成内存泄漏。
应在 useLayoutEffect 中添加 else 分支重置状态，或在 onVisibleChanged 回调中当 nextVisible === false 时调用 setPortalRender(false)。测试断言从 toBeFalsy() 改为 toBeTruthy() 反映的是当前的错误行为，而非预期行为。
🤖 Prompt for AI Agents
tests/controlled.test.tsx around line 34: the Portal is never unmounted because
useLayoutEffect only sets portalRender = true when open becomes true and never
resets it to false, causing the Portal to remain in the DOM; fix by adding the
missing cleanup path—either add an else branch in the useLayoutEffect to call
setPortalRender(false) when open is false, or ensure onVisibleChanged calls
setPortalRender(false) when nextVisible === false; after fixing the component
restore the test assertion to expect the portal to be absent (use toBeFalsy())
instead of toBeTruthy().
There was a problem hiding this comment.
你的这个问题在逻辑上没有错误，我实际测试过，若添加setPortalRender(false)，动画会有问题
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
1 participant
PR: ant-design/ant-design#56204
fix:ant-design/ant-design#55955
保持原有的动画写法不变，初次加载的时候动画是正常的，之后的每次加载都不正常。主要原因在下方这张图里
我想到了两种解决方案，这两种解决方案都在RcImage里面改动，改动后那个div就会在关闭的时候销毁掉，若是有更好的建议也可以帮我改进一下，谢谢大佬！
第一种方案
第二种方案
Summary by CodeRabbit
Bug 修复
新功能 / 改进
测试
✏️ Tip: You can customize this high-level summary in your review settings.