fix: Allow space character in Select input#1210
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @zombieJ, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue in the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough修改 BaseSelect 的 Enter/Space 键处理：引入 isEditable（mode === 'combobox' || showSearch），仅在非可编辑场景下对 Enter/Space 调用 event.preventDefault()；新增针对 showSearch 下 Space 行为的测试用例集合。 Changes
Sequence Diagram(s)（省略 — 更改为简短逻辑调整与测试覆盖，不满足生成序列图的条件） Estimated code review effort🎯 2 (简单) | ⏱️ ~12 分钟 Possibly related PRs
Suggested reviewers
诗
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Fixed issue where pressing the space key in Select input would not add a space character to the input field. The root cause was that preventDefault() was always called when mode was not 'combobox', which prevented typing spaces even when the input was editable in showSearch mode. Changes: - Modified space key handling to only call preventDefault() when the input is not editable - Input is considered editable when mode is 'combobox' or showSearch is true - This allows users to type spaces in editable Select inputs while preventing default behavior (scroll/submit) in non-editable Selects This fix allows users to type spaces in search terms within Select components with showSearch enabled.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request effectively addresses the issue of allowing space characters in the Select input, which was previously prevented by preventDefault(). The changes correctly modify the key handling logic in BaseSelect to differentiate between various scenarios (dropdown open/closed, input empty/filled) and ensure the space key behaves as expected. The addition of new test cases (spaceInput.test.tsx) is a good practice and helps verify the fix. The code is well-structured and the logic appears sound for the intended fix.
tests/spaceInput.test.tsx
Outdated
| }); | ||
| input.dispatchEvent(keyDownEvent); | ||
|
|
||
| expect(input.value).toBe('hello'); |
There was a problem hiding this comment.
The test case should allow typing space when typing in the input currently asserts that input.value remains 'hello' after dispatching a keyDownEvent for space. However, for the space character to actually appear in the input field, the input.value should be updated to 'hello ' (with a space) and then an input event should be dispatched. The current test only verifies that preventDefault() was not called, but not that the space character was successfully added to the input.
| expect(input.value).toBe('hello'); | |
| expect(input.value).toBe('hello '); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1210 +/- ##
=======================================
Coverage 99.43% 99.43%
=======================================
Files 31 31
Lines 1234 1236 +2
Branches 423 446 +23
=======================================
+ Hits 1227 1229 +2
Misses 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zombieJ
force-pushed
the
fix/input-space-issue
branch
from
e28d53e to
f566f35
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/spaceInput.test.tsx`:
- Around line 6-26: The test currently bypasses React state by directly setting
input.value and dispatching native events, so rc-select's internal
mergedSearchValue/onSearch may not update and the test can pass for the wrong
reason; change the test "should allow typing space when typing in the input" to
use `@testing-library/react`'s fireEvent (e.g., fireEvent.change) to simulate
typing "hello" so React state updates, then simulate the space keydown using
fireEvent.keyDown (or new KeyboardEvent) while spying on
KeyboardEvent.prototype.preventDefault (or using jest.fn()) to assert that
preventDefault was NOT called; reference the test block name and the input
element selection to locate the code to modify.
🧹 Nitpick comments (1)
tests/spaceInput.test.tsx (1)
28-46: 测试二建议同时验证下拉框是否打开。当前仅验证了
preventDefault被调用，但 PR 目标是"空格键在输入为空时打开下拉框"。建议增加断言验证下拉框确实被打开了：建议补充断言
expect(preventDefaultSpy).toHaveBeenCalled(); + + // 验证下拉框已打开 + expect(container.querySelector('.rc-select-open')).toBeTruthy();
tests/spaceInput.test.tsx
Outdated
| it('should allow typing space when typing in the input', () => { | ||
| const { container } = render( | ||
| <Select showSearch options={[{ value: 'test', label: 'test' }]} />, | ||
| ); | ||
|
|
||
| const input = container.querySelector('input'); | ||
| input.focus(); | ||
| input.value = 'hello'; | ||
|
|
||
| const event = new Event('input', { bubbles: true }); | ||
| input.dispatchEvent(event); | ||
|
|
||
| const keyDownEvent = new KeyboardEvent('keydown', { | ||
| key: ' ', | ||
| code: 'Space', | ||
| bubbles: true, | ||
| }); | ||
| input.dispatchEvent(keyDownEvent); | ||
|
|
||
| expect(input.value).toBe('hello'); | ||
| }); |
There was a problem hiding this comment.
测试一可能因绕过 React 状态而无法真正验证空格输入行为。
直接设置 input.value = 'hello' 并 dispatch 原生 input 事件，可能不会触发 rc-select 的 onSearch 回调来更新 mergedSearchValue。如果 mergedSearchValue 仍为空字符串，那么 !(isSpaceKey && mergedSearchValue !== '') 条件为 true，空格仍会被 preventDefault——测试可能因为错误的原因而通过。
建议使用 @testing-library/react 的 fireEvent 来模拟用户输入，确保 React 状态正确更新：
建议的改进方案
- const input = container.querySelector('input');
- input.focus();
- input.value = 'hello';
-
- const event = new Event('input', { bubbles: true });
- input.dispatchEvent(event);
-
- const keyDownEvent = new KeyboardEvent('keydown', {
- key: ' ',
- code: 'Space',
- bubbles: true,
- });
- input.dispatchEvent(keyDownEvent);
-
- expect(input.value).toBe('hello');
+ import { fireEvent } from '@testing-library/react';
+ // ...
+ const input = container.querySelector('input');
+ fireEvent.focus(input);
+ fireEvent.change(input, { target: { value: 'hello' } });
+
+ const keyDownEvent = new KeyboardEvent('keydown', {
+ key: ' ',
+ code: 'Space',
+ bubbles: true,
+ });
+ const preventDefaultSpy = jest.spyOn(keyDownEvent, 'preventDefault');
+ input.dispatchEvent(keyDownEvent);
+
+ // 验证空格键未被阻止（允许输入空格）
+ expect(preventDefaultSpy).not.toHaveBeenCalled();此外，断言 input.value 不变并不能证明空格被"允许"输入。更有效的断言是验证 preventDefault 未被调用。
🤖 Prompt for AI Agents
In `@tests/spaceInput.test.tsx` around lines 6 - 26, The test currently bypasses
React state by directly setting input.value and dispatching native events, so
rc-select's internal mergedSearchValue/onSearch may not update and the test can
pass for the wrong reason; change the test "should allow typing space when
typing in the input" to use `@testing-library/react`'s fireEvent (e.g.,
fireEvent.change) to simulate typing "hello" so React state updates, then
simulate the space keydown using fireEvent.keyDown (or new KeyboardEvent) while
spying on KeyboardEvent.prototype.preventDefault (or using jest.fn()) to assert
that preventDefault was NOT called; reference the test block name and the input
element selection to locate the code to modify.
Added test coverage to verify space key behavior in different Select modes: 1. showSearch enabled: Space should NOT call preventDefault, allowing space to be typed 2. showSearch disabled: Space SHOULD call preventDefault, preventing page scroll 3. combobox mode: Space should NOT call preventDefault, allowing space to be typed These tests ensure the fix correctly handles space key input based on whether the input is editable.
zombieJ
force-pushed
the
fix/input-space-issue
branch
from
9d6a8f1 to
cfa1ff5
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/BaseSelect/index.tsx (1)
472-483:⚠️ Potential issue | 🔴 Critical
isEditable同时跳过了 Enter 键的preventDefault，可能导致表单意外提交。
isEditable条件同时影响 Enter 和 Space 两个键。在combobox或showSearch模式下，Enter 键不再调用preventDefault()，这意味着如果 Select 组件位于<form>中，按下 Enter 会触发表单提交——这是一个功能回退。建议将
preventDefault的跳过逻辑仅限于 Space 键：🐛 建议修复
if (isEnterKey || isSpaceKey) { - const isEditable = mode === 'combobox' || showSearch; - if (!isEditable) { + // Space key: allow typing space in editable selects (combobox or showSearch). + // Enter key: always preventDefault to avoid form submission. + const isEditableSpace = isSpaceKey && (mode === 'combobox' || showSearch); + if (!isEditableSpace) { event.preventDefault(); }
Separate isCombobox check and apply different preventDefault logic: - Space: prevent only when not editable - Enter: prevent only when not combobox Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
fix ant-design/ant-design#56988
Fixed issue where pressing the space key in Select input would not add a space character to the input field.
The space key was always calling preventDefault(), which prevented the default behavior of adding a space character to the input.
Changes:
This fix allows users to type spaces in search terms within Select components while maintaining the existing behavior for opening the dropdown.
Summary by CodeRabbit
发布说明
Bug Fixes
Tests