-
-
Notifications
You must be signed in to change notification settings - Fork 462
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
fix: The Enter key lock cannot be released correctly when the custom input calls blur(). #1112
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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/BaseSelect/index.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
变更概览演练这个拉取请求主要修改了 变更
可能相关的 PR
建议的审阅者
诗歌
Finishing Touches
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 (
|
@@ -566,6 +566,11 @@ const BaseSelect = React.forwardRef<BaseSelectRef, BaseSelectProps>((props, ref) | |||
}); | |||
}; | |||
|
|||
const onSelectorBlur = () => { | |||
// Unlock the Enter key after the selector blur; otherwise, the Enter key needs to be pressed twice to trigger the correct effect. | |||
keyLockRef.current = false; |
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.
有这个了,上面看起来也不用换位置?
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.
需要的,因为onSelect事件是在onKeyDown事件里触发的,然后用户在onSelect里面执行blur,就会触发这个新增的onSelectorBlur,然后继续回到onKeyDown的事件里面,如果不调整顺序,最终keyLockRef.current还是会被设置为true
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1112 +/- ##
=======================================
Coverage 98.25% 98.25%
=======================================
Files 39 39
Lines 1486 1488 +2
Branches 448 421 -27
=======================================
+ Hits 1460 1462 +2
Misses 26 26 ☔ View full report in Codecov by Sentry. |
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
🧹 Nitpick comments (2)
docs/examples/combobox.tsx (1)
6-8
: 建议添加 null 检查在调用 blur() 时,建议添加 null 检查以提高代码的健壮性。
const onSelect = () => { - ref.current!.blur(); + ref.current?.blur(); };tests/Select.test.tsx (1)
2393-2394
: 建议添加非空断言由于我们确定 input 元素存在,建议在获取 input 元素后添加非空断言。
- inputElem = container.querySelector('input'); + inputElem = container.querySelector('input')!;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/examples/combobox.tsx
(1 hunks)tests/Select.test.tsx
(4 hunks)tests/utils/common.ts
(1 hunks)
🔇 Additional comments (3)
docs/examples/combobox.tsx (1)
4-20
: 代码实现清晰简洁!组件实现了一个很好的示例,展示了如何正确处理自定义输入组件的 Enter 键锁定问题。使用 useRef 来管理输入引用是一个很好的做法。
tests/utils/common.ts (1)
121-127
: 实用函数实现恰当!delay 函数的实现很好地封装了异步等待功能,便于测试异步行为。使用 act 确保了 React 状态更新的正确性。
tests/Select.test.tsx (1)
2373-2418
: 测试用例设计全面!测试用例很好地覆盖了 Enter 键锁定问题的各个方面:
- 验证了选择后下拉框的关闭
- 验证了输入值的更新
- 确保可以重新打开下拉框
- 验证了多次选择的正确性
…input calls blur().
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 comments (1)
src/BaseSelect/index.tsx (1)
建议完善 Enter 键锁定机制的单元测试
现有的测试文件中仅包含了自定义输入组件失焦时释放 Enter 键锁定的测试用例。建议在
tests/Select.test.tsx
中补充以下测试场景:
- Enter 键按下时的锁定机制
- 防止重复触发的验证
- 普通失焦时的锁定释放行为
可以参考
tests/shared/keyDownTest.tsx
中的键盘事件测试工具来实现这些测试用例。🔗 Analysis chain
Line range hint
540-823
: 建议添加单元测试建议为新增的 Enter 键锁定和释放机制添加相应的单元测试,以验证在以下场景下的行为:
- Enter 键按下时的锁定
- blur 事件触发时的锁定释放
- 连续按 Enter 键的情况
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查是否存在相关的测试用例 rg -l "test.*keyLock|test.*Enter.*key" --type tsLength of output: 50
Script:
#!/bin/bash # Search for test files in BaseSelect directory fd "test.*\.(ts|tsx)$" src/BaseSelect # Search for keyboard event related tests rg -l "keyboard|keyDown|keyUp|onKey" --type ts ".*test.*" # Search for specific test patterns rg -A 5 "describe.*keyboard|test.*key" --type tsLength of output: 199
Script:
#!/bin/bash # Find all test files fd "\.test\.(ts|tsx)$" # Search for keyboard event tests with corrected pattern rg -l "keyboard|keyDown|keyUp|onKey" --type ts # Look for specific keyboard handling tests rg -A 5 "describe\(.*keyboard|test\(.*key" --type tsLength of output: 976
Script:
#!/bin/bash # Check keyboard event test implementations rg -A 10 "keyDown|keyUp|onKey" tests/shared/keyDownTest.tsx tests/BaseSelect.test.tsx # Look for Enter key related tests rg -A 5 "Enter" tests/shared/keyDownTest.tsx tests/BaseSelect.test.tsx tests/Select.test.tsxLength of output: 1758
🧹 Nitpick comments (1)
src/Selector/MultipleSelector.tsx (1)
75-75
: 代码变更正确,建议小幅优化!blur 事件处理的实现与 SingleSelector 保持一致,很好地集成到了现有的焦点管理中。
建议考虑将
onBlur
事件与组件内的focused
状态管理结合:<Input ... - onBlur={onInputBlur} + onBlur={(e) => { + setFocused(false); + onInputBlur?.(e); + }} ... />这样可以确保组件的内部状态与外部事件处理保持同步。
Also applies to: 235-235
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/BaseSelect/index.tsx
(3 hunks)src/Selector/Input.tsx
(4 hunks)src/Selector/MultipleSelector.tsx
(2 hunks)src/Selector/SingleSelector.tsx
(2 hunks)src/Selector/index.tsx
(4 hunks)tests/Select.test.tsx
(2 hunks)tests/utils/common.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Selector/index.tsx
- tests/Select.test.tsx
🔇 Additional comments (8)
src/Selector/SingleSelector.tsx (1)
39-39
: 代码变更看起来不错!新增的
onInputBlur
属性和其传递到 Input 组件的实现方式正确。这些更改与修复 Enter 键锁定问题的目标相符。Also applies to: 104-104
src/Selector/Input.tsx (1)
28-28
: 实现完善且符合最佳实践!blur 事件的处理链完整且符合 React 的事件处理模式:
- 正确定义了
onBlur
类型- 保持了原始 input 元素的 blur 处理
- 实现了自定义的 blur 处理
这个实现将有效解决 Enter 键锁定的问题。
Also applies to: 56-56, 71-71, 140-145
tests/utils/common.ts (3)
121-126
: 代码注释清晰明确!JSDoc 注释很好地解释了函数的用途和参数。
127-139
: 验证该实现是否满足 PR 目标这个新的
waitFakeTimer
函数很好地支持了 PR 的主要目标:
- 通过控制计时器可以准确测试按键锁定和释放的时序
- 使用
act
确保状态更新被正确处理- 支持测试
blur()
调用后的异步行为运行以下脚本验证相关测试用例:
✅ Verification successful
waitFakeTimer 实现完全符合测试需求
该实现完美支持了测试用例中的异步行为验证:
- 正确处理了 Select 组件的打开/关闭状态
- 确保了按键事件和 blur 调用的正确时序
- 通过
act
包装保证了状态更新的可靠性🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 搜索与 Enter 键和 blur 相关的测试 rg "release.*Enter.*key.*lock.*blur" tests/Length of output: 147
Script:
#!/bin/bash # Get the test implementation with context rg -A 20 "release Enter key lock after customize input calls blur" tests/Length of output: 1207
Script:
#!/bin/bash # Get more context around the test case rg -B 5 -A 30 "release Enter key lock after customize input calls blur" tests/Length of output: 1968
127-139
: 建议优化等待时间的默认值函数实现逻辑正确,但有以下建议:
- 默认等待时间(1000ms)和迭代次数(20次)可能过高,这会导致测试运行时间较长。
- 考虑根据实际用例调整这些默认值,比如使用较小的初始值。
建议如下修改:
-export async function waitFakeTimer(advanceTime = 1000, times = 20) { +export async function waitFakeTimer(advanceTime = 100, times = 10) {另外,建议添加单元测试来验证这个新函数的行为:
- 测试正数 advanceTime 的情况
- 测试零或负数 advanceTime 的情况
- 验证迭代次数是否符合预期
运行以下脚本检查是否已有相关测试:
src/BaseSelect/index.tsx (3)
540-543
: 优化了 Enter 键锁定的时机在下拉菜单打开时,通过在触发 onSelect 事件之前设置 keyLockRef,解决了由于 blur 导致的键盘锁定问题。
571-574
: 添加了 blur 时的键盘锁定释放机制通过在 selector blur 时重置 keyLockRef,确保下次按 Enter 键时能够正确触发事件,避免需要按两次的问题。
823-823
: 集成了 blur 事件处理将 onSelectorBlur 处理函数正确地传递给了 Selector 组件,完成了键盘锁定释放机制的闭环。
🤔 这个变动的性质是?
🔗 相关 Issue
fix: ant-design#52028
💡 需求背景和解决方案
问题:
AutoComplete组件使用自定义Input,在onSelect事件中执行Input的blur(),会导致下次展开备选列表时无法用Enter选中
原因:
按下Enter键 --> 触发onSelect --> 在onSelect里面执行blur() --> 在onKeyDown里面把keyLockRef.current设置为true --> 正常是应该在onKeyUp里面释放锁的,但是因为执行了blur(),导致keyup事件不会触发,所以keyLockRef为true
于是在下次点击enter的时候,由于keyLockRef为true,就不会触发onSelect事件,反而可以正常触发keyup了,在onKeyUp里面释放了keyLockRef,也就是问题描述里的第二次开始需要点击两次Enter键才可以正常用
解决方案:
优化keyLockRef设置为true的时机,在触发onSelect之前设置为true,并给Selector添加一个onBlur的事件处理函数,在onBlur时主动释放keyLockRef
📝 更新日志
The Enter key lock cannot be released correctly when the custom input calls blur().
Summary by CodeRabbit
发布说明
新功能
测试
改进
这些更改旨在提升组件的用户交互体验和稳定性。