Skip to content
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() #1113

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

LeeSSHH
Copy link
Contributor

@LeeSSHH LeeSSHH commented Jan 13, 2025

🤔 这个变动的性质是?

  • 🐞 Bug 修复

🔗 相关 Issue

ref ant-design/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内部的input添加一个onBlur的事件处理函数,在onBlur时主动释放keyLockRef

📝 更新日志

语言 更新描述
🇺🇸 英文 fix: The Enter key lock cannot be released correctly when the custom input calls blur().(ant-design#52028)
🇨🇳 中文 fix: 在onSelect中触发自定义Input的blur()时,没有正确释放回车键的keyLockRef。(ant-design#52028)

The Enter key lock cannot be released correctly when the custom input calls blur().

Summary by CodeRabbit

Summary by CodeRabbit

  • 新功能

    • 为选择器组件添加了输入和选择器的模糊事件处理
    • 改进了回车键的交互控制机制
  • 测试

    • 新增了针对自定义输入元素和回车键锁定行为的测试用例
    • 添加了用于测试计时器的实用函数

Copy link

vercel bot commented Jan 13, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
select ❌ Failed (Inspect) Jan 14, 2025 6:51am

Copy link
Contributor

coderabbitai bot commented Jan 13, 2025

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/BaseSelect/index.tsx

Oops! Something went wrong! :(

ESLint: 8.57.1

Error: Cannot read config file: /.eslintrc.js
Error: Cannot find module '@umijs/fabric/dist/eslint'
Require stack:

  • /.eslintrc.js
  • /node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/lib/cli-engine/cli-engine.js
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/lib/eslint/eslint.js
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/lib/eslint/index.js
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/lib/cli.js
  • /node_modules/.pnpm/[email protected]/node_modules/eslint/bin/eslint.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
    at Module._load (node:internal/modules/cjs/loader:1074:27)
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
    at Module.require (node:internal/modules/cjs/loader:1339:12)
    at require (node:internal/modules/helpers:135:16)
    at Object. (/.eslintrc.js:1:14)
    at Module._compile (node:internal/modules/cjs/loader:1546:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
    at Module.load (node:internal/modules/cjs/loader:1317:32)

变更概述

演练

此次拉取请求主要修改了 BaseSelect 组件中的键盘交互处理,特别是对回车键的控制。新增了在按下回车键时锁定键盘事件的逻辑,并在选择器失去焦点时解锁。同时,在多个选择器相关文件中添加了输入模糊(blur)事件处理的支持,增强了组件的交互性和灵活性。

变更

文件 变更摘要
src/BaseSelect/index.tsx 添加回车键锁定机制和 onInputBlur 方法
src/Selector/Input.tsx 新增 onBlur 事件处理
src/Selector/MultipleSelector.tsx 添加 onInputBlur 属性
src/Selector/SingleSelector.tsx 添加 onInputBlur 属性
src/Selector/index.tsx 新增 onInputBluronSelectorBlur 接口属性
tests/Select.test.tsx 添加测试用例验证回车键锁定机制
tests/utils/common.ts 新增 waitFakeTimer 异步工具函数

可能相关的 PR

建议的审阅者

  • zombieJ

诗歌

🐰 键盘舞动,回车轻触
选择器闪烁,焦点流转
锁与解锁,智慧交织
代码如诗,兔子微笑
交互之美,悄然绽放 🌟


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (5)
tests/utils/common.ts (1)

121-139: 测试工具函数实现得很好!

waitFakeTimer 函数的实现考虑周到:

  • 使用 act 包装异步操作,符合 React 测试最佳实践
  • 提供了灵活的配置选项(advanceTime 和 times)
  • 支持两种计时器推进模式(advanceTimersByTime 和 runAllTimers)

建议添加一个示例用法的注释,以帮助其他开发者更好地理解如何使用这个函数。

 /**
  * Wait for a time delay. Will wait `advanceTime * times` ms.
  *
  * @param advanceTime Default 1000
  * @param times Default 20
+ * @example
+ * // Wait for 2 seconds with 100ms intervals
+ * await waitFakeTimer(100, 20);
+ *
+ * // Run all timers immediately 5 times
+ * await waitFakeTimer(0, 5);
  */
src/Selector/MultipleSelector.tsx (1)

75-75: 多选择器中的 blur 处理实现一致!

这些更改与 SingleSelector 保持一致,确保了组件行为的统一性。建议添加一个简单的注释说明这个 prop 的用途。

  onInputCompositionEnd,
+ // 处理输入框失焦事件,用于释放 Enter 键锁定
  onInputBlur,

Also applies to: 235-235

src/Selector/index.tsx (2)

47-47: 建议为新增的 onInputBlur 属性添加注释说明。

为了提高代码的可维护性,建议添加 JSDoc 注释来说明该回调的用途和触发时机。

+  /** 当输入框失去焦点时触发的回调函数 */
   onInputBlur: React.FocusEventHandler<HTMLInputElement | HTMLTextAreaElement>;

96-97: 建议为新增的 onSelectorBlur 属性添加注释说明。

为了提高代码的可维护性,建议添加 JSDoc 注释来说明该回调的用途和触发时机。

+  /** 当选择器失去焦点时触发的回调函数,用于释放 Enter 键锁定 */
   onSelectorBlur?: () => void;
tests/Select.test.tsx (1)

2371-2419: 测试用例设计完善。

该测试用例很好地覆盖了以下场景:

  1. 自定义输入框的失焦处理
  2. Enter 键锁定的释放机制
  3. 重新打开下拉框后的选择行为
  4. 值的正确性验证

建议补充以下测试场景:

  1. 键盘事件的防抖处理
  2. 在失焦过程中的异常处理
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d77993 and 5dd45e2.

📒 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)
🔇 Additional comments (6)
src/Selector/SingleSelector.tsx (1)

39-39: 新增的 onInputBlur 处理程序看起来不错!

这些更改正确地实现了模糊事件处理,有助于解决 Enter 键锁定问题。确保在失去焦点时释放键盘锁定是一个很好的做法。

Also applies to: 104-104

src/Selector/Input.tsx (1)

28-28: onBlur 事件处理实现正确!

blur 事件处理的实现遵循了组件中其他事件处理器的模式:

  • 正确处理原始输入元素的 onBlur
  • 保持了事件处理的一致性
  • 类型定义完善

这些更改是解决 Enter 键锁定问题的关键部分。

Also applies to: 56-56, 140-145

src/Selector/index.tsx (1)

276-276: 代码结构良好。

onInputBlur 添加到 sharedProps 中是正确的做法,这样可以确保 MultipleSelectorSingleSelector 都能正确处理失焦事件。

src/BaseSelect/index.tsx (3)

540-543: Enter 键锁定逻辑实现正确。

在触发事件之前设置锁定状态是正确的做法,这样可以防止重复触发 onChange 事件。


571-574: onSelectorBlur 函数实现简洁明了。

该函数的实现符合预期,通过在选择器失焦时释放 Enter 键锁定,解决了需要按两次 Enter 键才能触发正确效果的问题。


823-823: 属性传递正确。

onSelectorBlur 传递给 Selector 组件是正确的实现方式,确保了失焦事件可以正确触发并释放 Enter 键锁定。

Copy link

codecov bot commented Jan 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.26%. Comparing base (4d77993) to head (22f36c6).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1113      +/-   ##
==========================================
+ Coverage   98.25%   98.26%   +0.01%     
==========================================
  Files          39       39              
  Lines        1486     1498      +12     
  Branches      421      451      +30     
==========================================
+ Hits         1460     1472      +12     
  Misses         26       26              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/Selector/index.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (1)
tests/utils/common.ts (1)

121-126: 改进函数文档注释

建议在文档注释中添加以下内容:

  • 函数的具体用途和使用场景
  • 参数 times 的实际影响
  • 返回值说明
  • 使用示例
 /**
  * Wait for a time delay. Will wait `advanceTime * times` ms.
+ * 
+ * @description 用于在测试中模拟时间延迟,特别适用于需要等待异步操作完成的场景。
+ * 总等待时间为 advanceTime * times 毫秒。
  *
- * @param advanceTime Default 1000
- * @param times Default 20
+ * @param advanceTime - 每次迭代推进的时间(毫秒)。默认值:1000
+ * @param times - 迭代次数,用于控制总等待时间。默认值:20
+ * @returns Promise<void> - 当所有定时器执行完毕后解决
+ * 
+ * @example
+ * // 等待 2 秒,每次推进 100ms,共执行 20 次
+ * await waitFakeTimer(100, 20);
  */
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5dd45e2 and 9e20d1d.

📒 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 (5)
  • src/Selector/SingleSelector.tsx
  • src/Selector/MultipleSelector.tsx
  • src/Selector/index.tsx
  • tests/Select.test.tsx
  • src/Selector/Input.tsx
🔇 Additional comments (5)
tests/utils/common.ts (1)

127-139: 实现逻辑清晰,代码结构合理

函数实现符合以下要求:

  1. 使用 act 确保异步操作的安全性
  2. 提供灵活的定时器控制选项
  3. 使用 Promise 处理异步流程
  4. 代码简洁且易于理解
src/BaseSelect/index.tsx (4)

540-543: 优化了 Enter 键锁定的时机

将 Enter 键的锁定时机提前到触发列表 keydown 事件之前,这样可以更准确地防止重复触发 onChange 事件。


571-574: 新增输入框失焦时的锁定释放机制

通过在输入框失焦时释放 Enter 键锁定,解决了需要双击 Enter 键才能触发正确效果的问题。实现符合预期。


823-823: 完善了 Selector 组件的失焦处理

将 onInputBlur 处理函数正确地传递给了 Selector 组件,完成了完整的事件处理链。


Line range hint 540-823: 验证修复方案的完整性

整体实现方案完整地解决了自定义输入调用 blur() 时 Enter 键锁定无法正确释放的问题。建议添加相关测试用例以确保修复的稳定性。

考虑在组件文档中补充说明这个行为变更,特别是对于使用自定义输入的场景。

@LeeSSHH LeeSSHH requested a review from zombieJ January 14, 2025 06:43
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (1)
tests/Select.test.tsx (1)

2371-2419: 测试用例实现完整且合理!

该测试用例很好地覆盖了以下关键场景:

  • 自定义输入框的 blur 事件处理
  • Enter 键锁定和释放机制
  • 下拉框的打开/关闭状态
  • 输入值的正确性验证

建议考虑添加以下测试场景以提高覆盖率:

  • 在 blur 过程中取消选择的情况
  • 快速连续按 Enter 键的边缘情况
 it('release Enter key lock after customize input calls blur()', async () => {
   let inputElem: HTMLInputElement | null = null;
   const onBlur = jest.fn();
+  const onSelect = jest.fn();
   const Demo: React.FC = () => {
     const ref = React.useRef<HTMLInputElement>(null);
-    const onSelect = () => {
+    const handleSelect = (value: string) => {
+      onSelect(value);
       ref.current!.blur();
       fireEvent.blur(ref.current);
     };
     const getInputElement = () => {
       return <input ref={ref} onBlur={onBlur} />;
     };
     return (
       <Select
         options={[{ value: 'aa' }, { value: 'bb' }]}
-        onSelect={onSelect}
+        onSelect={handleSelect}
         mode="combobox"
         getInputElement={getInputElement}
       />
     );
   };
   
   // ... existing test code ...
+
+  // Test rapid Enter key presses
+  toggleOpen(container);
+  await waitFakeTimer();
+  
+  // Simulate rapid Enter key presses
+  keyDown(inputElem!, 13);
+  keyDown(inputElem!, 13);
+  
+  await waitFakeTimer();
+  expect(onSelect).toHaveBeenCalledTimes(1); // Should only trigger once
+  
+  // Test blur during selection
+  toggleOpen(container);
+  await waitFakeTimer();
+  
+  keyDown(inputElem!, 40);
+  fireEvent.blur(inputElem); // Blur before Enter
+  keyDown(inputElem!, 13);
+  
+  await waitFakeTimer();
+  expect(onBlur).toHaveBeenCalledTimes(3);
 });
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e20d1d and 22f36c6.

📒 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 (6)
  • src/Selector/SingleSelector.tsx
  • tests/utils/common.ts
  • src/Selector/MultipleSelector.tsx
  • src/Selector/Input.tsx
  • src/Selector/index.tsx
  • src/BaseSelect/index.tsx

@zombieJ zombieJ merged commit 3ff731d into react-component:master Jan 14, 2025
9 of 10 checks passed
afc163 pushed a commit that referenced this pull request Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants