fix: avoid submitting unconfirmed range values on blur#966
fix: avoid submitting unconfirmed range values on blur#966QDyanbing wants to merge 5 commits intoreact-component:masterfrom
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在 RangePicker 中加入键盘/鼠标切换跟踪与相关焦点/鼠标处理，调整在 needConfirm（如 showTime）模式下的失焦与字段切换提交逻辑；新增测试覆盖这些场景以防止未确认时的部分提交或误触发 onChange。 Changes
Sequence Diagram(s)（未生成序列图 — 变更集中在组件内部事件与状态流，交互主体有限） Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![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
This pull request introduces formatting updates to the range picker interfaces and adds a conditional check to prevent setting the last operation to 'input' when confirmation is required. It also includes a new test case to ensure unconfirmed values are not submitted on blur when 'allowEmpty' is enabled. Feedback suggests that the current fix may not be robust enough to handle cases where a user types into the input, recommending that the '!needConfirm' check be moved to the 'useLayoutEffect' responsible for field leave logic.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #966 +/- ##
=======================================
Coverage 98.81% 98.82%
=======================================
Files 65 65
Lines 2695 2714 +19
Branches 724 754 +30
=======================================
+ Hits 2663 2682 +19
Misses 29 29
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/range.spec.tsx (1)
898-919: 建议增加暂存值已产生的前置断言。当前测试只验证最终未提交；如果
selectCell(11)未来没有真正触发暂存选择，测试仍可能因为onChange未调用、输入为空而误通过。建议断言onCalendarChange已收到未确认的 start 值，再执行 blur。🧪 建议增强回归测试
it('should not submit unconfirmed values on blur when allowEmpty lets fields switch', () => { const onChange = jest.fn(); - const { container } = render(<DayRangePicker showTime allowEmpty onChange={onChange} />); + const onCalendarChange = jest.fn(); + const { container } = render( + <DayRangePicker + showTime + allowEmpty + onChange={onChange} + onCalendarChange={onCalendarChange} + />, + ); openPicker(container, 0); selectCell(11); + expect(onCalendarChange).toHaveBeenCalledWith( + expect.anything(), + ['1990-09-11 00:00:00', ''], + { range: 'start' }, + ); openPicker(container, 1); openPicker(container, 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/range.spec.tsx` around lines 898 - 919, Add a pre-assert that the picker emitted the transient selection by creating a jest.fn() for onCalendarChange and passing it into the DayRangePicker, call selectCell(11), then assert onCalendarChange was invoked with an interim value containing the expected unconfirmed start (e.g. first element non-empty/expected date) before proceeding to the blur and final expect; reference the onCalendarChange prop, selectCell, and DayRangePicker to locate where to add the spy and assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/range.spec.tsx`:
- Around line 898-919: Add a pre-assert that the picker emitted the transient
selection by creating a jest.fn() for onCalendarChange and passing it into the
DayRangePicker, call selectCell(11), then assert onCalendarChange was invoked
with an interim value containing the expected unconfirmed start (e.g. first
element non-empty/expected date) before proceeding to the blur and final expect;
reference the onCalendarChange prop, selectCell, and DayRangePicker to locate
where to add the spy and assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f57b3f14-9664-4bb3-99d2-e6100496c0e0
📒 Files selected for processing (2)
src/PickerInput/RangePicker.tsxtests/range.spec.tsx
QDyanbing
force-pushed
the
codex/fix-range-picker-unconfirmed-blur
branch
from
5853032 to
2e520bb
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/range.spec.tsx (2)
922-926: 多次jest.runAllTimers()可用waitFakeTimer替代此处（以及随后 5 个新增用例的第 948-952、984-988、1024-1028、1068-1072、1109-1113 行）均使用
for循环调用jest.runAllTimers()5 次来等待多轮异步更新。由于文件顶部已从./util/commonUtil导入waitFakeTimer（line 33），可直接使用它以减少样板代码并更清晰地表达“等待所有副作用链稳定”的意图。♻️ 建议的精简写法
- for (let i = 0; i < 5; i += 1) { - act(() => { - jest.runAllTimers(); - }); - } + await waitFakeTimer();注意使用
waitFakeTimer时需将用例签名改为async。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/range.spec.tsx` around lines 922 - 926, 将多次调用 jest.runAllTimers() 的循环替换为已导入的 waitFakeTimer 工具以减少样板并清晰表达“等待所有副作用稳定”的意图：在每个相关测试（当前使用 for 循环并在 act 中调用 jest.runAllTimers() 的用例）将测试签名改为 async，并用 await waitFakeTimer() 替换那段循环（移除 act + 多次 jest.runAllTimers() 调用）；保留现有 act(...) 包裹的必要交互但不要再在循环中重复 runAllTimers。确保所有出现该模式的用例（当前有六处）都做相同替换并导入/使用已存在的 waitFakeTimer 符号。
1030-1034: 断言过弱：建议显式断言onChange的实际调用入参当前写法“未以
['…09-11…', '…09-12…']被调用 + 曾被调用过”无法锁定回归：若未来 bug 导致onChange以其它错误组合（例如['…09-11…', '…09-13…']）被触发，这两个断言依然都会通过。从下面matchValues(container, '1990-09-11 00:00:00', '')可知预期实际调用是['1990-09-11 00:00:00', '']，建议直接断言该结果以提高用例对回归的敏感度。此处（line 1030-1034）以及下一个用例 line 1074-1078 都存在同样问题。
♻️ 建议的显式断言
- expect(onChange).not.toHaveBeenCalledWith(expect.anything(), [ - '1990-09-11 00:00:00', - '1990-09-12 00:00:00', - ]); - expect(onChange).toHaveBeenCalled(); + expect(onChange).toHaveBeenCalledWith(expect.anything(), [ + '1990-09-11 00:00:00', + '', + ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/range.spec.tsx` around lines 1030 - 1034, The test currently uses weak assertions for onChange (checking it was called and not called with a specific wrong array), which can miss regressions; update the assertions in the two failing specs that interact via matchValues to assert the exact expected call arguments for the onChange mock (e.g., use jest's toHaveBeenCalledWith or toHaveBeenNthCalledWith against the exact array ['1990-09-11 00:00:00', ''] as indicated by matchValues(container, '1990-09-11 00:00:00', '')), and apply the same explicit assertion pattern to the other similar case in this file so the tests verify the precise onChange parameters rather than only negative/loose checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/range.spec.tsx`:
- Around line 922-926: 将多次调用 jest.runAllTimers() 的循环替换为已导入的 waitFakeTimer
工具以减少样板并清晰表达“等待所有副作用稳定”的意图：在每个相关测试（当前使用 for 循环并在 act 中调用 jest.runAllTimers()
的用例）将测试签名改为 async，并用 await waitFakeTimer() 替换那段循环（移除 act + 多次
jest.runAllTimers() 调用）；保留现有 act(...) 包裹的必要交互但不要再在循环中重复
runAllTimers。确保所有出现该模式的用例（当前有六处）都做相同替换并导入/使用已存在的 waitFakeTimer 符号。
- Around line 1030-1034: The test currently uses weak assertions for onChange
(checking it was called and not called with a specific wrong array), which can
miss regressions; update the assertions in the two failing specs that interact
via matchValues to assert the exact expected call arguments for the onChange
mock (e.g., use jest's toHaveBeenCalledWith or toHaveBeenNthCalledWith against
the exact array ['1990-09-11 00:00:00', ''] as indicated by
matchValues(container, '1990-09-11 00:00:00', '')), and apply the same explicit
assertion pattern to the other similar case in this file so the tests verify the
precise onChange parameters rather than only negative/loose checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ad09d6d1-8493-4c54-8a4e-46eb39bbb052
📒 Files selected for processing (1)
tests/range.spec.tsx
2 participants
Summary
needConfirmis enabled!needConfirmflowsshowTime + allowEmptywithout pressingOKTesting
Fixes ant-design/ant-design#57728
Summary by CodeRabbit
发布说明
New Features
Bug Fixes
Tests