-
-
Notifications
You must be signed in to change notification settings - Fork 462
fix: fix the edge mis trigger #1092
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改主要涉及三个文件的修改。 Changes
Assessment against linked issues
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/utils/keyUtil.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
🧹 Outside diff range and nitpick comments (3)
src/utils/keyUtil.ts (1)
5-39
: 代码逻辑正确,建议增加防御性检查修复方案通过增加
currentKeyCode
的真值检查来解决 Edge 浏览器的自动完成触发问题,实现简单有效。建议考虑增加类型检查以提高代码的健壮性。建议增加以下类型检查:
export function isValidateOpenKey(currentKeyCode: number): boolean { return ( // Undefined for Edge bug: // https://github.com/ant-design/ant-design/issues/51292 - currentKeyCode && + typeof currentKeyCode === 'number' && currentKeyCode > 0 && // Other keys ![tests/Accessibility.test.tsx (1)
71-104
: 测试用例覆盖了基本场景,建议补充边界情况当前测试用例验证了 undefined 和有效按键的情况,这很好。但建议增加更多边界测试场景以确保修复的完整性。
建议增加以下测试场景:
- 测试 keyCode 为 0 的情况
- 测试 keyCode 为负数的情况
- 测试 keyCode 为非数字类型的情况
示例代码:
it('edge bug - additional edge cases', () => { const { container } = render( <Select mode="combobox" options={[{ value: '123' }]} defaultValue="123" /> ); // Test keyCode = 0 keyDown(container.querySelector('input')!, 0); act(() => { jest.runAllTimers(); }); expectOpen(container, false); // Test negative keyCode keyDown(container.querySelector('input')!, -1); act(() => { jest.runAllTimers(); }); expectOpen(container, false); });tests/utils/common.ts (1)
104-106
: 事件模拟实现正确,建议添加注释说明通过 Object.defineProperties 添加 which 属性的实现符合 DOM 事件规范,但建议添加注释说明这个属性的作用和必要性。
建议添加如下注释:
const event = createEvent.keyDown(element, { keyCode }); + // 添加 which 属性以完整模拟键盘事件 + // 某些浏览器依赖 which 属性来确定按键值 Object.defineProperties(event, { which: { get: () => keyCode }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/utils/keyUtil.ts
(1 hunks)tests/Accessibility.test.tsx
(2 hunks)tests/utils/common.ts
(1 hunks)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1092 +/- ##
==========================================
+ Coverage 98.24% 98.31% +0.07%
==========================================
Files 39 39
Lines 1483 1487 +4
Branches 419 443 +24
==========================================
+ Hits 1457 1462 +5
+ Misses 26 25 -1 ☔ View full report in Codecov by Sentry. |
Edge 自动完成的时候会莫名其妙触发一个
keyDown
事件到不相关的 input 上。目前只看出来最小复原结构为:解法为对触发打开的判断加一个键位判断,为空的话就认为是无效键跳过开启。
fix ant-design/ant-design#51292
Summary by CodeRabbit
Bug Fixes
Tests
Select
组件添加了新的测试用例,以验证无效和有效键的处理。act
函数,以确保状态更新正确处理。Chores
keyDown
函数,以增强事件对象的功能。