Keep multiple select open on clear#1225
Conversation
|
@QDyanbing is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
Walkthrough修改 SelectInput 组件的鼠标处理逻辑,在 multiple 模式下点击清除按钮时保持弹出层打开。调整条件判断从 ChangesMultiple 模式清除行为修复
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
There was a problem hiding this comment.
Code Review
This pull request modifies the SelectInput component to ensure that the dropdown popup remains open when the clear icon is clicked in multiple-select mode, updating the condition to only close the popup for single-select mode. Additionally, a unit test has been added to verify this behavior. There are no review comments, and I have no further feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1225 +/- ##
=======================================
Coverage 99.44% 99.44%
=======================================
Files 31 31
Lines 1266 1266
Branches 442 462 +20
=======================================
Hits 1259 1259
Misses 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/Multiple.test.tsx (1)
111-130: ⚡ Quick win建议增强测试完整性:验证清除操作是否实际清空了值。
当前测试正确验证了弹出层保持打开状态,但未验证清除操作是否真的将
defaultValue={['1', '2']}清空。建议添加对清除后选中值的断言,以确保清除功能本身正常工作,而不仅仅是弹出层状态。💡 建议的测试增强
it('keeps popup open when clearing values', () => { + const onChange = jest.fn(); const onPopupVisibleChange = jest.fn(); const { container } = render( <Select mode="multiple" defaultOpen defaultValue={['1', '2']} allowClear + onChange={onChange} onPopupVisibleChange={onPopupVisibleChange} > <Option value="1">One</Option> <Option value="2">Two</Option> </Select>, ); fireEvent.mouseDown(container.querySelector('.rc-select-clear')); expectOpen(container, true); expect(onPopupVisibleChange).not.toHaveBeenCalledWith(false); + expect(onChange).toHaveBeenCalledWith([], expect.anything()); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Multiple.test.tsx` around lines 111 - 130, The test "keeps popup open when clearing values" currently only asserts the popup stays open and onPopupVisibleChange isn't called with false; update it to also verify the clear actually removed the selections: after calling fireEvent.mouseDown(container.querySelector('.rc-select-clear')), add an assertion that selected items are gone (e.g. query for selection tags like '.rc-select-selection-item' returns length 0 or the text "One"/"Two" is not present) or that the Select's value/input is empty; keep the existing expectOpen and onPopupVisibleChange assertions intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/Multiple.test.tsx`:
- Around line 111-130: The test "keeps popup open when clearing values"
currently only asserts the popup stays open and onPopupVisibleChange isn't
called with false; update it to also verify the clear actually removed the
selections: after calling
fireEvent.mouseDown(container.querySelector('.rc-select-clear')), add an
assertion that selected items are gone (e.g. query for selection tags like
'.rc-select-selection-item' returns length 0 or the text "One"/"Two" is not
present) or that the Select's value/input is empty; keep the existing expectOpen
and onPopupVisibleChange assertions intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5ee5d547-4211-480c-9b08-2500c8ddc2f4
📒 Files selected for processing (2)
src/SelectInput/index.tsxtests/Multiple.test.tsx
变更内容
defaultOpen + allowClear清空后弹层保持展开的场景。问题原因
clear icon 会标记
_select_lazy,此前内部逻辑在弹层已展开时会统一关闭弹层;这对单选仍然合理,但会打断多选用户清空后继续选择的连续操作。影响
multiple/tags清空后保持当前展开状态。验证
npm test -- tests/Multiple.test.tsx tests/Select.test.tsx --runInBand --silentnpm run tsc关联 ant-design/ant-design#58336
Summary by CodeRabbit
Bug Fixes
Tests