fix: improve wheel step precision#733
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 (1)
WalkthroughInputNumber 增加了 wheel 事件的增量规范化与跨事件累积机制：按 deltaMode 换算增量，使用 refs 累积高精度 delta 并在达到阈值时触发一次步进。同步更新并新增 wheel 相关测试覆盖多种 deltaMode、精度与方向切换场景。 Changes滚轮增量累积和阈值触发
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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
tests/wheel.test.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in 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 |
![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 a wheel delta accumulation mechanism to the InputNumber component to support high-precision scrolling and different wheel delta modes (pixel, line, and page). It tracks the accumulated delta and triggers a step change once a threshold is reached. The review feedback suggests a valuable improvement: instead of resetting the accumulated wheel delta to zero when a step is triggered, subtracting the step distance would preserve any remaining fractional delta, leading to a smoother scrolling experience for high-precision wheels.
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.
| if (Math.abs(wheelDeltaRef.current) >= WHEEL_STEP_DISTANCE) { | ||
| // moving mouse wheel rises wheel event with deltaY < 0 | ||
| // scroll value grows from top to bottom, as screen Y coordinate | ||
| onInternalStep(wheelDeltaRef.current < 0, 'wheel'); | ||
| wheelDeltaRef.current = 0; | ||
| } |
There was a problem hiding this comment.
Instead of resetting wheelDeltaRef.current to 0 when a step is triggered, consider subtracting WHEEL_STEP_DISTANCE (preserving the sign). This ensures that any fractional delta beyond the threshold (e.g., from high-precision wheels or fast scrolls) is preserved for subsequent events rather than being discarded, making the scrolling experience much smoother and more precise.
| if (Math.abs(wheelDeltaRef.current) >= WHEEL_STEP_DISTANCE) { | |
| // moving mouse wheel rises wheel event with deltaY < 0 | |
| // scroll value grows from top to bottom, as screen Y coordinate | |
| onInternalStep(wheelDeltaRef.current < 0, 'wheel'); | |
| wheelDeltaRef.current = 0; | |
| } | |
| if (Math.abs(wheelDeltaRef.current) >= WHEEL_STEP_DISTANCE) { | |
| // moving mouse wheel rises wheel event with deltaY < 0 | |
| // scroll value grows from top to bottom, as screen Y coordinate | |
| onInternalStep(wheelDeltaRef.current < 0, 'wheel'); | |
| wheelDeltaRef.current -= Math.sign(wheelDeltaRef.current) * WHEEL_STEP_DISTANCE; | |
| } |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #733 +/- ##
==========================================
+ Coverage 95.31% 95.78% +0.46%
==========================================
Files 6 6
Lines 299 332 +33
Branches 83 92 +9
==========================================
+ Hits 285 318 +33
Misses 14 14 ☔ View full report in Codecov by Harness. 🚀 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/wheel.test.tsx (1)
107-115: 💤 Low value建议补充 page mode 和边界场景的测试覆盖。
当前测试覆盖了
deltaMode: 1（line mode），但缺少以下场景：
deltaMode: 2（page mode）的换算验证- 滚轮方向变化时累积值重置的行为
- 超过 200ms 间隔后累积值重置的行为
📝 建议新增的测试用例
it('supports page mode wheel delta', () => { const onChange = jest.fn(); const { container } = render(<InputNumber onChange={onChange} changeOnWheel />); fireEvent.focus(container.firstChild); // deltaMode: 2, deltaY: -1 → -1 * 800 = -800, |-800| >= 100 fireEvent.wheel(container.querySelector('input'), { deltaMode: 2, deltaY: -1 }); expect(onChange).toHaveBeenCalledWith(1); }); it('resets accumulated delta on direction change', () => { const onChange = jest.fn(); const { container } = render(<InputNumber onChange={onChange} changeOnWheel />); const input = container.querySelector('input'); fireEvent.focus(container.firstChild); // Accumulate 90 in negative direction for (let i = 0; i < 18; i += 1) { fireEvent.wheel(input, { deltaY: -5 }); } expect(onChange).not.toHaveBeenCalled(); // Change direction - should reset accumulator fireEvent.wheel(input, { deltaY: 5 }); expect(onChange).not.toHaveBeenCalled(); // Now need full 100 in positive direction for (let i = 0; i < 19; i += 1) { fireEvent.wheel(input, { deltaY: 5 }); } expect(onChange).toHaveBeenCalledWith(-1); });🤖 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/wheel.test.tsx` around lines 107 - 115, Add tests to cover page-mode conversion and accumulator reset behavior for the InputNumber changeOnWheel handling: add a test that fires a wheel event with deltaMode: 2 (page mode) and a small deltaY (e.g., -1) to assert the value change after page-to-pixel scaling is applied (reference: InputNumber, changeOnWheel, fireEvent.wheel, deltaMode/deltaY); add a test that accumulates wheel deltas in one direction then fires a wheel in the opposite direction to assert the accumulator resets on direction change (reference: accumulated delta logic used by changeOnWheel and the input element); and add a test that waits longer than 200ms between wheel events to assert the accumulator resets after the timeout (reference: the 200ms accumulator timeout in the wheel handling). Ensure each test focuses on the onChange calls and uses container.querySelector('input') and fireEvent.wheel to simulate events.
🤖 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/wheel.test.tsx`:
- Around line 107-115: Add tests to cover page-mode conversion and accumulator
reset behavior for the InputNumber changeOnWheel handling: add a test that fires
a wheel event with deltaMode: 2 (page mode) and a small deltaY (e.g., -1) to
assert the value change after page-to-pixel scaling is applied (reference:
InputNumber, changeOnWheel, fireEvent.wheel, deltaMode/deltaY); add a test that
accumulates wheel deltas in one direction then fires a wheel in the opposite
direction to assert the accumulator resets on direction change (reference:
accumulated delta logic used by changeOnWheel and the input element); and add a
test that waits longer than 200ms between wheel events to assert the accumulator
resets after the timeout (reference: the 200ms accumulator timeout in the wheel
handling). Ensure each test focuses on the onChange calls and uses
container.querySelector('input') and fireEvent.wheel to simulate events.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d51d730-665f-4d3f-85e5-6cc4fb7c80ea
📒 Files selected for processing (2)
src/InputNumber.tsxtests/wheel.test.tsx
1 participant
Summary
Why
When
changeOnWheelis enabled, each wheel event currently triggers one step. Low precision mouse wheels can emit many events for a small physical scroll, causing InputNumber values to jump too quickly.Related to ant-design/ant-design#58328.
Test
npm test -- tests/wheel.test.tsx --runInBandnpm test -- --runInBandnpm run lintSummary by CodeRabbit
功能改进
测试