fix: avoid submitting unconfirmed range values on blur#966
fix: avoid submitting unconfirmed range values on blur#966QDyanbing wants to merge 4 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Walkthrough在 RangePicker 中加入键盘/鼠标切换跟踪与相关焦点/鼠标处理，调整在 needConfirm（如 showTime）模式下的失焦与字段切换提交逻辑；新增测试覆盖这些场景以防止未确认时的部分提交或误触发 onChange。 Changes
Sequence Diagram(s)（未生成序列图 — 变更集中在组件内部事件与状态流，交互主体有限） Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #966 +/- ##
==========================================
- Coverage 98.81% 98.78% -0.03%
==========================================
Files 65 65
Lines 2695 2714 +19
Branches 724 754 +30
==========================================
+ Hits 2663 2681 +18
- Misses 29 30 +1
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
2 participants
Summary
needConfirmis enabled!needConfirmflowsshowTime + allowEmptywithout pressingOKTesting
Fixes ant-design/ant-design#57728
Summary by CodeRabbit
发布说明
New Features
Bug Fixes
Tests