Skip to content

fix: optimize ESC key handling in nested portal scenarios#494

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

Merged
zombieJ merged 5 commits into from
Dec 26, 2025
Merged

fix: optimize ESC key handling in nested portal scenarios #494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bda5e1b
add reproduce demo
aojunhao123 Dec 24, 2025
b4c7b99
chore: adjust
aojunhao123 Dec 25, 2025
d093098
fix: optimize ESC key handling in nested portal scenarios
aojunhao123 Dec 26, 2025
7eccdce
chore: adjust test
aojunhao123 Dec 26, 2025
283014a
update
aojunhao123 Dec 26, 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
3 changes: 2 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,4 +44,5 @@ es/
.dumi/tmp
.dumi/tmp-production

bun.lockb
bun.lockb
.vscode
1 change: 1 addition & 0 deletions assets/preview.less
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,6 +5,7 @@
overflow: hidden;
user-select: none;
inset: 0;
z-index: 1055;

&-mask {
position: absolute;
Expand Down
8 changes: 8 additions & 0 deletions docs/demo/nested.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
---
title: nested
nav:
title: Demo
path: /demo
---

<code src="../examples/nested.tsx"></code>
71 changes: 71 additions & 0 deletions docs/examples/nested.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
import React, { useState } from 'react';
import Dialog from '@rc-component/dialog';
import Image from '@rc-component/image';
import '@rc-component/dialog/assets/index.css';
import '../../assets/index.less';

const App: React.FC = () => {
const [show, setShow] = useState(false);
return (
<>
<button
onClick={() => {
setShow(true);
}}
>
showModal
</button>
<Dialog
visible={show}
afterOpenChange={open => {
setShow(open);
}}
onClose={() => {
setShow(false);
}}
footer={
<>
<button
onClick={() => {
setShow(false);
}}
>
Cancel
</button>
<button
onClick={() => {
setShow(false);
}}
>
OK
</button>
</>
}
>
<Image
width={200}
alt="svg image"
src="https://gw.alipayobjects.com/zos/rmsportal/KDpgvguMpGfqaHPjicRK.svg"
/>
<Image.PreviewGroup
preview={{
onChange: (current, prev) =>
console.log(`current index: ${current}, prev index: ${prev}`),
}}
>
<Image
width={200}
alt="svg image"
src="https://gw.alipayobjects.com/zos/rmsportal/KDpgvguMpGfqaHPjicRK.svg"
/>
<Image
width={200}
src="https://gw.alipayobjects.com/zos/antfincdn/aPkFc8Sj7n/method-draw-image.svg"
/>
</Image.PreviewGroup>
</Dialog>
</>
);
Comment on lines +8 to +68

@gemini-code-assistgemini-code-assistBotDec 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The event handlers in this component can be simplified and deduplicated for better readability and maintainability. You can define separate functions for showing and hiding the modal and reuse them. Also, afterOpenChange can directly receive the setShow function.

 const [show, setShow] = useState(false);
const showModal = () => setShow(true);
const hideModal = () => setShow(false);
return (
<>
<button
onClick={showModal}
>
showModal
</button>
<Dialog
visible={show}
afterOpenChange={setShow}
onClose={hideModal}
footer={
<>
<button
onClick={hideModal}
>
Cancel
</button>
<button
onClick={hideModal}
>
OK
</button>
</>
}
>
<Image
width={200}
alt="svg image"
src="https://gw.alipayobjects.com/zos/rmsportal/KDpgvguMpGfqaHPjicRK.svg"
/>
<Image.PreviewGroup
preview={{
onChange: (current, prev) =>
console.log(`current index: ${current}, prev index: ${prev}`),
}}
>
<Image
width={200}
alt="svg image"
src="https://gw.alipayobjects.com/zos/rmsportal/KDpgvguMpGfqaHPjicRK.svg"
/>
<Image
width={200}
src="https://gw.alipayobjects.com/zos/antfincdn/aPkFc8Sj7n/method-draw-image.svg"
/>
</Image.PreviewGroup>
</Dialog>
</>
);

};

export default App;
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -41,14 +41,15 @@
},
"dependencies": {
"@rc-component/motion": "^1.0.0",
"@rc-component/portal": "^2.0.0",
"@rc-component/portal": "^2.1.2",
"@rc-component/util": "^1.3.0",
"clsx": "^2.1.1"
},
"devDependencies": {
"@ant-design/icons": "^5.0.1",
"@rc-component/father-plugin": "^2.0.2",
"@rc-component/np": "^1.0.0",
"@rc-component/dialog": "^1.7.0",
"@testing-library/jest-dom": "^6.4.0",
"@testing-library/react": "^15.0.6",
"@types/jest": "^30.0.0",
Expand Down
18 changes: 13 additions & 5 deletions src/Preview/index.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -332,10 +332,6 @@ const Preview: React.FC<PreviewProps> = props => {
if (open) {
const { keyCode } = event;

if (keyCode === KeyCode.ESC) {
onClose?.();
}

if (showLeftOrRightSwitches) {
if (keyCode === KeyCode.LEFT) {
onActive(-1);
Expand DownExpand Up@@ -380,6 +376,12 @@ const Preview: React.FC<PreviewProps> = props => {
}
}, [open]);

const onEsc: PortalProps['onEsc'] = ({ top }) => {
if (top) {
onClose?.();
}
};

// ========================== Render ==========================
const bodyStyle: React.CSSProperties = {
...styles.body,
Expand All@@ -389,7 +391,13 @@ const Preview: React.FC<PreviewProps> = props => {
}

return (
<Portal open={portalRender} getContainer={getContainer} autoLock={lockScroll}>
<Portal
open={portalRender && open}

@aojunhao123aojunhao123Dec 26, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

portalRender 我 blame了一下，发现是 #392 中为了解决ssr渲染的问题而引入的，但是由于portal侧会根据open状态去做入栈出栈操作，但是Image这里的portalRender在layoutEffect中置为true之后就恒为true，这就导致了Esc按下后，preview消失，但portal存在，无法出栈，再次按下Esc时Modal也不会关闭。因此我这边加上了open做栈进出，portalRender保留，但只作为ssr gate。应该不会break到原有逻辑
cc @zombieJ

autoDestroy={false}

@aojunhao123aojunhao123Dec 26, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

这里是为了确保行为与原逻辑一致

getContainer={getContainer}
autoLock={lockScroll}
onEsc={onEsc}
>
<CSSMotion
motionName={motionName}
visible={portalRender && open}
Expand Down
22 changes: 21 additions & 1 deletion tests/preview.test.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,7 +7,8 @@ import ZoomInOutlined from '@ant-design/icons/ZoomInOutlined';
import ZoomOutOutlined from '@ant-design/icons/ZoomOutOutlined';
import { spyElementPrototypes } from '@rc-component/util/lib/test/domHook';
import { act, createEvent, fireEvent, render } from '@testing-library/react';
import React from 'react';
import React, { useState } from 'react';
import Dialog from '@rc-component/dialog';

jest.mock('../src/Preview', () => {
const MockPreview = (props: any) => {
Expand DownExpand Up@@ -1052,6 +1053,25 @@ describe('Preview', () => {
expect(afterOpenChange).toHaveBeenCalledTimes(2);
});

it('Esc closes preview then modal', () => {
const onClose = jest.fn();

const { baseElement, getByRole } = render(
<Dialog visible onClose={onClose}>
<Image src="https://zos.alipayobjects.com/rmsportal/jkjgkEfvpUPVyRjUImniVslZfWPnJuuZ.png" />
</Dialog>,
);

fireEvent.click(getByRole('img'));
expect(baseElement.querySelector('.rc-image-preview')).toBeTruthy();

fireEvent.keyDown(window, { key: 'Escape' });
expect(baseElement.querySelector('.rc-image-preview')).toBeFalsy();

fireEvent.keyDown(window, { key: 'Escape' });
expect(onClose).toHaveBeenCalled();
});

it('not modify preview image size', () => {
render(
<Image
Expand Down
3 changes: 2 additions & 1 deletion tsconfig.json
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,7 @@
"@/*": ["src/*"],
"@@/*": [".dumi/tmp/*"],
"@rc-component/image": ["src/index.ts"]
}
},
"types": ["@testing-library/jest-dom", "jest", "node"]
}
}