Skip to content

chore: adjust keyboard event handling logic #541

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 4 commits into from
Dec 24, 2025
Merged

chore: adjust keyboard event handling logic #541

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
efe2d0a
chore: adjust keyboard event handling logic
aojunhao123 Dec 23, 2025
5d2516d
improve type
aojunhao123 Dec 23, 2025
defc94a
chore: adjust
aojunhao123 Dec 24, 2025
6260af3
chore: adjust
aojunhao123 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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,4 @@ pnpm-lock.yaml
.dumi/tmp-production

bun.lockb
.vscode
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
},
"dependencies": {
"@rc-component/motion": "^1.1.3",
"@rc-component/portal": "^2.0.0",
"@rc-component/portal": "^2.1.0",

@coderabbitai coderabbitai Bot Dec 23, 2025

Copy link
Copy Markdown

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 @rc-component/portal package information on npm
curl -s https://registry.npmjs.org/@rc-component/portal | jq '.versions | keys[]' | grep -E "2\.[01]\." | sort -V

Repository: react-component/dialog

Length of output: 89

🌐 Web query:

@rc-component/portal 2.1.0 onEsc property TypeScript

💡 Result:

There is no onEsc prop in @rc-component/portal v2.1.0. The TypeScript definitions for that version list getContainer, children, open, autoDestroy, autoLock and debug (see Portal.d.ts), and the README's API does not include onEsc. [1][2]

Sources:

  • Portal.d.ts (package v2.1.0 type defs). [1]
  • README / API for @rc-component/portal (package files). [2]

🌐 Web query:

@rc-component/portal changelog breaking changes 2.0.0 to 2.1.0

💡 Result:

I couldn't find a 2.1.0 release for @rc-component/portal — the published package versions are 2.0.0 / 2.0.1 (no 2.1.0). Therefore there are no documented breaking changes from 2.0.0 → 2.1.0 because 2.1.0 does not appear to exist. [1][2]

Sources:

  • npm package page showing v2.0.0 (and v2.0.1 in metadata). [1]
  • jsDelivr package listing (no v2.1.0). [2]

🌐 Web query:

@rc-component/portal 2.1.0 security vulnerabilities

💡 Result:

Short answer: I found no reported security vulnerabilities for @rc-component/portal — and npm lists the latest published version as 2.0.0 (no 2.1.0 release), so there are no advisories for a v2.1.0 release. [1][2][3]

Sources:

  • npm package page (shows latest version 2.0.0). [1]
  • Snyk vulnerability database (no known vulnerabilities for @rc-component/portal versions). [2]
  • Socket/package scan summary (package metadata / last update). [3]

If you want, I can:

  • check your codebase for instances of this package,
  • run a dependency scan (Snyk/npm audit style) against your lockfile, or
  • watch for future advisories and send alerts.

@rc-component/portal 版本 2.1.0 不存在，onEsc 属性也不受支持。

依赖版本更新到 ^2.1.0 无法实现——npm 注册表中仅存在 2.0.0 和 2.0.1 版本。此外，@rc-component/portal 的 TypeScript 定义不包含 onEsc 属性；支持的属性仅有：getContainer、children、open、autoDestroy、autoLock 和 debug。需要修正：

  1. 版本号改为有效的发布版本（2.0.0 或 2.0.1）
  2. 移除或重构代码中对 PortalProps['onEsc'] 的使用
🤖 Prompt for AI Agents 
In package.json around line 54, the dependency "@rc-component/portal": "^2.1.0"
is invalid and the library does not expose an onEsc prop; change the version to
a published release (use "2.0.1" or "2.0.0") in package.json and then remove or
refactor any code that references PortalProps['onEsc']—replace it with a
supported approach (e.g., attach a keydown/escape handler to the portal
container or handle escape at the parent component and pass a supported
callback) so there are no references to a non-existent prop.

"@rc-component/util": "^1.5.0",
"clsx": "^2.1.1"
},
Expand Down
13 changes: 0 additions & 13 deletions src/Dialog/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { clsx } from 'clsx';
import contains from '@rc-component/util/lib/Dom/contains';
import useId from '@rc-component/util/lib/hooks/useId';
import KeyCode from '@rc-component/util/lib/KeyCode';
import pickAttrs from '@rc-component/util/lib/pickAttrs';
import * as React from 'react';
import { useEffect, useRef } from 'react';
Expand All @@ -16,10 +15,7 @@ const Dialog: React.FC<IDialogPropTypes> = (props) => {
prefixCls = 'rc-dialog',
zIndex,
visible = false,
keyboard = true,
focusTriggerAfterClose = true,
// scrollLocker,
// Wrapper
wrapStyle,
wrapClassName,
wrapProps,
Expand Down Expand Up @@ -142,14 +138,6 @@ const Dialog: React.FC<IDialogPropTypes> = (props) => {
};
}

function onWrapperKeyDown(e: React.KeyboardEvent<HTMLDivElement>) {
if (keyboard && e.keyCode === KeyCode.ESC) {
e.stopPropagation();
onInternalClose(e);
return;
}
}

// ========================= Effect =========================
useEffect(() => {
if (visible) {
Expand Down Expand Up @@ -201,7 +189,6 @@ const Dialog: React.FC<IDialogPropTypes> = (props) => {
className={modalClassNames?.mask}
/>
<div
onKeyDown={onWrapperKeyDown}
className={clsx(`${prefixCls}-wrap`, wrapClassName, modalClassNames?.wrapper)}
ref={wrapperRef}
onClick={onWrapperClick}
Expand Down
13 changes: 12 additions & 1 deletion src/DialogWrap.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Portal from '@rc-component/portal';
import Portal, { type PortalProps } from '@rc-component/portal';

@coderabbitai coderabbitai Bot Dec 24, 2025

Copy link
Copy Markdown

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:

#!/bin/bash
# 检查 node_modules 中是否存在 @rc-component/portal 的类型定义
echo "=== 检查 @rc-component/portal 包结构 ==="
if [ -d "node_modules/@rc-component/portal" ]; then
  ls -la node_modules/@rc-component/portal/ | head -20
  echo -e "\n=== 查找类型定义文件 ==="
  find node_modules/@rc-component/portal -name "*.d.ts" -o -name "package.json" | head -10
else
  echo "未找到 @rc-component/portal 包"
fi

Repository: react-component/dialog

Length of output: 128

🏁 Script executed:

#!/bin/bash
# 在 package.json 中检查 @rc-component/portal 的版本
echo "=== 检查项目中 @rc-component/portal 的版本 ==="
if [ -f "package.json" ]; then
  rg "@rc-component/portal" package.json
fi

# 如果存在 package-lock.json 或 yarn.lock，查看版本信息
if [ -f "package-lock.json" ]; then
  echo -e "\n=== Package Lock 中的版本信息 ==="
  rg -A5 "@rc-component/portal" package-lock.json | head -20
elif [ -f "yarn.lock" ]; then
  echo -e "\n=== Yarn Lock 中的版本信息 ==="
  rg -A5 "@rc-component/portal" yarn.lock | head -20
fi

Repository: react-component/dialog

Length of output: 142

🏁 Script executed:

#!/bin/bash
# 搜索 PortalProps 在 node_modules 中的定义
echo "=== 搜索 PortalProps 的类型定义 ==="
rg -n "PortalProps|onEsc" node_modules/@rc-component/portal -A5 2>/dev/null | head -50

Repository: react-component/dialog

Length of output: 94

🌐 Web query:

@rc-component/portal 2.1.0 PortalProps API documentation onEsc

💡 Result:

Short answer: @rc-component/portal (v2.x) PortalProps does not expose an onEsc prop. The documented props are getContainer, open and autoLock (no onEsc present). [1][2]

If you need ESC-to-close behavior, handle keydown in your overlay component (or wrap the portal with a stateful controller that listens for Escape and closes). [1][2]

Sources:

  • @rc-component/portal package README / API (props list). [1]
  • jsDelivr / package metadata for @rc-component/portal (v2.x). [2]

移除不支持的 onEsc 属性。

@rc-component/portal 2.1.0 的 PortalProps 不包含 onEsc 属性。在第 32 行使用 PortalProps['onEsc'] 和第 55 行传递 onEsc 给 Portal 组件都是无效的，该属性会被 Portal 忽略。Portal 仅支持 getContainer、open 和 autoLock 属性。需要在 DialogWrap 组件层面处理 ESC 键事件，而非依赖 Portal 的 onEsc。

import * as React from 'react';
import { RefContext } from './context';
import Dialog from './Dialog';
Expand All @@ -22,11 +22,21 @@ const DialogWrap: React.FC<IDialogPropTypes> = (props) => {
afterClose,
closable,
panelRef,
keyboard = true,
} = props;
const [animatedVisible, setAnimatedVisible] = React.useState<boolean>(visible);

const refContext = React.useMemo(() => ({ panel: panelRef }), [panelRef]);

const onEsc: PortalProps['onEsc'] = ({ top, event }) => {
if (top && keyboard) {
event.stopPropagation();
// @ts-ignore
props.onClose?.(event);

@gemini-code-assist gemini-code-assist Bot Dec 23, 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.

high

The use of // @ts-ignore here hides a significant type mismatch. The onEsc handler from @rc-component/portal provides a native KeyboardEvent, but the onClose prop is typed to expect a React.SyntheticEvent. This breaks the component's public API contract and can lead to unexpected behavior or runtime errors for consumers who might rely on SyntheticEvent-specific properties or behavior.

This should be addressed properly instead of being ignored. A proper fix would likely involve updating the onClose prop's type signature in IDialogPropTypes.ts to correctly reflect that it can receive different event types, for example: (e: React.SyntheticEvent | KeyboardEvent) => any;.

Suppressing this type error is risky and makes the component contract unreliable.

Comment thread
aojunhao123 marked this conversation as resolved.
Outdated
return;
}
};
Comment on lines +32 to +38

@coderabbitai coderabbitai Bot Dec 23, 2025

Copy link
Copy Markdown

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:

#!/bin/bash
# Check the actual version of @rc-component/portal in package.json
echo "=== Checking package.json for @rc-component/portal version ==="
cat package.json | grep -A 1 "@rc-component/portal"

Repository: react-component/dialog

Length of output: 202

🏁 Script executed:

#!/bin/bash
# Check if node_modules/@rc-component/portal exists and explore its structure
echo "=== Checking node_modules/@rc-component/portal structure ==="
if [ -d "node_modules/@rc-component/portal" ]; then
  echo "Directory exists. Contents:"
  ls -la node_modules/@rc-component/portal/ | head -20
  echo ""
  echo "=== Looking for type definitions ==="
  find node_modules/@rc-component/portal -name "*.d.ts" | head -10
else
  echo "node_modules/@rc-component/portal does not exist"
fi

Repository: react-component/dialog

Length of output: 175

🏁 Script executed:

#!/bin/bash
# Search for onEsc in Portal type definitions
echo "=== Searching for onEsc in @rc-component/portal ==="
if [ -d "node_modules/@rc-component/portal" ]; then
  rg -n "onEsc" node_modules/@rc-component/portal || echo "No onEsc found"
fi

Repository: react-component/dialog

Length of output: 117

🏁 Script executed:

#!/bin/bash
# Check the DialogWrap.tsx file, especially around line 54
echo "=== Checking DialogWrap.tsx around Portal usage ==="
if [ -f "src/DialogWrap.tsx" ]; then
  echo "File exists. Checking lines 50-60:"
  sed -n '50,60p' src/DialogWrap.tsx | cat -n
  echo ""
  echo "=== Full file to understand context ==="
  wc -l src/DialogWrap.tsx
fi

Repository: react-component/dialog

Length of output: 596

🏁 Script executed:

#!/bin/bash
# Check IDialogPropTypes.ts mentioned in the review
echo "=== Looking for IDialogPropTypes.ts ==="
find . -name "IDialogPropTypes.ts" -o -name "IDialogPropTypes*" 2>/dev/null

Repository: react-component/dialog

Length of output: 131

🏁 Script executed:

#!/bin/bash
# Read the full DialogWrap.tsx file to understand the complete context
echo "=== Reading src/DialogWrap.tsx ==="
if [ -f "src/DialogWrap.tsx" ]; then
  cat -n src/DialogWrap.tsx
fi

Repository: react-component/dialog

Length of output: 2896

🏁 Script executed:

#!/bin/bash
# Check IDialogPropTypes.ts
echo "=== Reading IDialogPropTypes.ts ==="
cat -n src/IDialogPropTypes.ts

Repository: react-component/dialog

Length of output: 2411

🏁 Script executed:

#!/bin/bash
# Check if there are any TypeScript errors or issues in the repo
echo "=== Looking for tsconfig and checking for build errors ==="
if [ -f "tsconfig.json" ]; then
  echo "tsconfig.json found"
fi

# Check for any files that might reference Portal or onEsc issues
echo ""
echo "=== Searching for Portal-related issues in comments ==="
rg -n "Portal|onEsc" src/ --type ts --type tsx -i | head -30

Repository: react-component/dialog

Length of output: 246

🌐 Web query:

@rc-component/portal 2.1.0 onEsc property API documentation

💡 Result:

I couldn't find an onEsc prop in @rc-component/portal's API. The published docs/type list for the portal component only show getContainer, open, and autoLock (no onEsc). [1][2]

Sources:

  • @rc-component/portal package README / API (props: getContainer, open, autoLock). [1]
  • Package listing / registry metadata for @rc-component/portal (no onEsc in prop list). [2]

@rc-component/portal ^2.1.0 不支持 onEsc 属性。

根据 web 查询，@rc-component/portal 的发布 API 仅包含 getContainer、open 和 autoLock 属性，不包含 onEsc。代码在第 31-37 行定义的 onEsc 处理程序和第 54 行传递给 Portal 的 onEsc 属性将被忽略。

  • PortalProps['onEsc'] 类型将为 undefined
  • ESC 键处理功能在运行时无法生效
  • Portal 组件不会识别 onEsc 属性

需要使用 Portal 支持的机制来处理 ESC 键，或升级 @rc-component/portal 至支持 onEsc 的版本。

🤖 Prompt for AI Agents 
In src/DialogWrap.tsx around lines 31-37 (and where onEsc is passed on line 54),
the Portal package version used does not support an onEsc prop so the handler is
ignored and PortalProps['onEsc'] is undefined; remove the onEsc prop usage and
instead implement ESC handling using a supported mechanism: add a useEffect
that, when the dialog is mounted/open and keyboard is enabled, attaches a
keydown listener on document that checks for Escape, verifies the dialog is top
(match current internal top-state or z-index/top flag), calls
event.stopPropagation() and props.onClose(event) if appropriate, and cleans up
the listener on unmount/close; alternatively, if you prefer keeping onEsc,
upgrade @rc-component/portal to a version that exposes onEsc and then update
types accordingly.


React.useEffect(() => {
if (visible) {
setAnimatedVisible(true);
Expand All @@ -42,6 +52,7 @@ const DialogWrap: React.FC<IDialogPropTypes> = (props) => {
<RefContext.Provider value={refContext}>
<Portal
open={visible || forceRender || animatedVisible}
onEsc={onEsc}
autoDestroy={false}
getContainer={getContainer}
autoLock={visible || animatedVisible}
Expand Down
0 src/IDialogPropTypes.tsx → src/IDialogPropTypes.ts
View file
Open in desktop
File renamed without changes.
5 changes: 1 addition & 4 deletions tests/index.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/* eslint-disable react/no-render-return-value, max-classes-per-file, func-names, no-console */
import { fireEvent, render, act } from '@testing-library/react';
import { Provider } from '@rc-component/motion';
import KeyCode from '@rc-component/util/lib/KeyCode';
import React, { cloneElement, useEffect } from 'react';
import type { DialogProps } from '../src';
import Dialog from '../src';
Expand Down Expand Up @@ -164,10 +163,8 @@ describe('dialog', () => {
it('esc to close', () => {
const onClose = jest.fn();
render(<Dialog onClose={onClose} visible />);
jest.runAllTimers();

fireEvent.keyDown(document.querySelector('.rc-dialog'), { keyCode: KeyCode.ESC });
jest.runAllTimers();
fireEvent.keyDown(window, { key: 'Escape' });
expect(onClose).toHaveBeenCalled();
});
Comment on lines 163 to 169

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

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

The test only covers the default keyboard behavior (keyboard=true). Consider adding a test case for keyboard=false to ensure the Escape key doesn't close the dialog when this prop is disabled, similar to how the "mask to close" test (lines 171-186) tests both maskClosable=true and maskClosable=false scenarios.

Copilot uses AI. Check for mistakes.

Expand Down
3 changes: 2 additions & 1 deletion tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"@rc-component/dialog": [
"src/index.ts"
]
}
},
"types": ["node", "jest", "@testing-library/jest-dom"],
},
"include": [
".dumirc.ts",
Expand Down
Loading