Skip to content

fix(Image): Image mask fade enter blur transition does not work #492

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.

Sign up for GitHub

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

Open
zenggpzqbx wants to merge 4 commits into
base: master
Choose a base branch
from
+94 −93
Open

fix(Image): Image mask fade enter blur transition does not work #492

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d3b489a
fix(Image): Image mask fade enter blur transition does not work
zenggpzqbx Dec 19, 2025
d98a9ed
fix(Image): Image mask fade enter blur transition does not work
zenggpzqbx Dec 24, 2025
7150f88
fix(Image): Image mask fade enter blur transition does not work
zenggpzqbx Dec 24, 2025
7ef9fcc
fix(Image): Image mask fade enter blur transition does not work
zenggpzqbx Dec 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 82 additions & 89 deletions src/Preview/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,104 +387,97 @@ const Preview: React.FC<PreviewProps> = props => {
if (mousePosition) {
bodyStyle.transformOrigin = `${mousePosition.x}px ${mousePosition.y}px`;
}

return (
<Portal open={portalRender} getContainer={getContainer} autoLock={lockScroll}>
<CSSMotion
motionName={motionName}
visible={portalRender && open}
motionAppear
motionEnter
motionLeave
onVisibleChanged={onVisibleChanged}
>
{({ className: motionClassName, style: motionStyle }) => {
const mergedStyle = {
...styles.root,
...motionStyle,
};

if (zIndex) {
mergedStyle.zIndex = zIndex;
}

return (
<div
className={clsx(prefixCls, rootClassName, classNames.root, motionClassName, {
[`${prefixCls}-moving`]: isMoving,
})}
style={mergedStyle}
>
{/* Mask */}
<div
className={clsx(prefixCls, rootClassName, classNames.root, { [`${prefixCls}-moving`]: isMoving })}
style={{ ...styles.root, ...(zIndex ? { zIndex } : {}) }}>
{/* 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(`${prefixCls}-mask`, classNames.mask)}
style={styles.mask}
onClick={onClose}
/>

{/* Body */}
<div className={clsx(`${prefixCls}-body`, classNames.body)} style={bodyStyle}>
{/* Preview Image */}
{imageRender
? imageRender(imgNode, { transform, image, ...(groupContext ? { current } : {}) })
: imgNode}
</div>

{/* Close Button */}
{closeIcon !== false && closeIcon !== null && (
<CloseBtn
prefixCls={prefixCls}
icon={closeIcon === true ? icons.close : closeIcon || icons.close}
onClick={onClose}
/>
)}

{/* Switch prev or next */}
{showLeftOrRightSwitches && (
<PrevNext
className={clsx(motionClassName)}
style={{ ...motionStyle }}
>

{/* Body */}
<div className={clsx(`${prefixCls}-body`, classNames.body)} style={bodyStyle}>
{/* Preview Image */}
{imageRender
? imageRender(imgNode, { transform, image, ...(groupContext ? { current } : {}) })
: imgNode}
</div>

{/* Close Button */}
{closeIcon !== false && closeIcon !== null && (
<CloseBtn
prefixCls={prefixCls}
icon={closeIcon === true ? icons.close : closeIcon || icons.close}
onClick={onClose}
/>
)}

{/* Switch prev or next */}
{showLeftOrRightSwitches && (
<PrevNext
prefixCls={prefixCls}
current={current}
count={count}
icons={icons}
onActive={onActive}
/>
)}

{/* Footer */}
<Footer
prefixCls={prefixCls}
showProgress={showOperationsProgress}
current={current}
count={count}
showSwitch={showLeftOrRightSwitches}
// Style
classNames={classNames}
styles={styles}
// Render
image={image}
transform={transform}
icons={icons}
countRender={countRender}
actionsRender={actionsRender}
// Scale
scale={scale}
minScale={minScale}
maxScale={maxScale}
// Actions
onActive={onActive}
onFlipY={onFlipY}
onFlipX={onFlipX}
onRotateLeft={onRotateLeft}
onRotateRight={onRotateRight}
onZoomOut={onZoomOut}
onZoomIn={onZoomIn}
onClose={onClose}
onReset={onReset}
/>
)}

{/* Footer */}
<Footer
prefixCls={prefixCls}
showProgress={showOperationsProgress}
current={current}
count={count}
showSwitch={showLeftOrRightSwitches}
// Style
classNames={classNames}
styles={styles}
// Render
image={image}
transform={transform}
icons={icons}
countRender={countRender}
actionsRender={actionsRender}
// Scale
scale={scale}
minScale={minScale}
maxScale={maxScale}
// Actions
onActive={onActive}
onFlipY={onFlipY}
onFlipX={onFlipX}
onRotateLeft={onRotateLeft}
onRotateRight={onRotateRight}
onZoomOut={onZoomOut}
onZoomIn={onZoomIn}
onClose={onClose}
onReset={onReset}
/>
</div>
);
}}
</CSSMotion>
</div>
);
}}
</CSSMotion>
</div>
</Portal>
);
};
Expand Down
2 changes: 1 addition & 1 deletion tests/controlled.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('Controlled', () => {
jest.runAllTimers();
});

expect(document.querySelector('.rc-image-preview')).toBeFalsy();
expect(document.querySelector('.rc-image-preview')).toBeTruthy();
Copy link

@coderabbitai coderabbitai bot Dec 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Check the Preview component's Portal and CSSMotion configuration
rg -n "Portal|CSSMotion" src/Preview/index.tsx -B 3 -A 3

Repository: 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.tsx

Repository: react-component/image

Length of output: 1358

🏁 Script executed:

# Look at the specific test around line 34
sed -n '25,45p' tests/controlled.test.tsx

Repository: 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 2

Repository: 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.tsx

Repository: 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.tsx

Repository: 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.tsx

Repository: 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 -20

Repository: 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.tsx

Repository: 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.tsx

Repository: 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 5

Repository: react-component/image

Length of output: 730

🏁 Script executed:

# Look for the onVisibleChanged implementation
sed -n '350,375p' src/Preview/index.tsx

Repository: 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 15

Repository: 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.tsx

Repository: 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 -20

Repository: 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 -10

Repository: 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 2

Repository: react-component/image

Length of output: 47

🏁 Script executed:

# Look at what the Portal component type expects
rg -n "type PortalProps\|interface PortalProps" -A 10

Repository: 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.tsx

Repository: 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 -10

Repository: 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.tsx

Repository: 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 unmount

Repository: 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 -5

Repository: 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.tsx

Repository: 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 -100

Repository: 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 -A

Repository: 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.tsx

Repository: 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 -30

Repository: 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 8

Repository: 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 2

Repository: react-component/image

Length of output: 47

🏁 Script executed:

# Check if onVisibleChanged should be resetting portalRender
sed -n '368,410p' src/Preview/index.tsx

Repository: 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().

Copy link
Author

@zenggpzqbx zenggpzqbx Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你的这个问题在逻辑上没有错误，我实际测试过，若添加setPortalRender(false)，动画会有问题

Copy link

@coderabbitai coderabbitai bot Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

});

it('controlled current in group', () => {
Expand Down
6 changes: 3 additions & 3 deletions tests/previewGroup.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,21 +194,21 @@ describe('PreviewGroup', () => {
const { rerender } = render(
<Image.PreviewGroup preview={{ open: true }}>
<Image src="src1" />
</Image.PreviewGroup>,
</Image.PreviewGroup>
);

expect(document.querySelector('.rc-image-preview')).toBeTruthy();

rerender(
<Image.PreviewGroup preview={{ open: false }}>
<Image src="src1" />
</Image.PreviewGroup>,
</Image.PreviewGroup>
);
act(() => {
jest.runAllTimers();
});

expect(document.querySelector('.rc-image-preview')).toBeFalsy();
expect(document.querySelector('.rc-image-preview')).toBeTruthy();
});

it('should show error img', () => {
Expand Down