[number field] Prevent scrubbing on horizontal wheel events

[sai6855]

Wheel scrub treated any event with no vertical delta as an increment, so horizontal trackpad swipes and tilt wheels silently stepped and committed the value while blocking the page scroll. Ignore horizontal-dominant gestures, and use the horizontal delta when shift is held.

[sai6855]

@claude review

[github-actions]

PR review

The one-line guard fixes the obvious case (a pure horizontal wheel no longer increments the field) but keys off event.deltaY === 0, which is both too broad and too narrow: browsers deliver shift + wheel as deltaX with deltaY === 0, so the documented largeStep-on-wheel interaction becomes a no-op, and real trackpad horizontal swipes rarely produce exactly deltaY === 0, so the reported bug can still fire. One finding is merge-blocking.

Bugs (2)

1. 🔴 Shift + wheel stepping by largeStep stops working in browsers that swap the wheel axis

Location: packages/react/src/number-field/root/NumberFieldRoot.tsx:355

if (
  // Allow pinch-zooming.
  event.ctrlKey ||
  activeElement(ownerDocument(inputRef.current)) !== inputRef.current ||
  event.deltaY === 0
) {
  return;
}

When the shift key is held during a mouse wheel turn, Chromium (and macOS at the OS level) swaps the axis and dispatches the event as deltaX: ±n, deltaY: 0, shiftKey: true. With the new guard, that event is now discarded before getStepAmount(event) is ever called, so shift + wheel does nothing at all on the field — despite largeStep being publicly documented as "the large step value of the input element when incrementing while the shift key is held", and despite NumberFieldRoot.test.tsx:1470 ("uses largeStep when shift is held during wheel") still passing, because it fires a synthetic { deltaY: -1, shiftKey: true } that no browser actually produces.

The new test does not scrub on a horizontal wheel event while shift is held cements this: it asserts that exactly the event shape Chromium emits for shift + wheel must be ignored.

To be fair, the pre-PR behavior on those browsers was also wrong (deltaY === 0 made event.deltaY > 0 ? -1 : 1 always resolve to +1, so shift + wheel only ever incremented). But going from "steps by largeStep, direction always up" to "silently does nothing, and the page scrolls sideways instead" is still a user-visible change to a documented interaction, and it locks the broken state in with a test.

Failure scenario: In Chrome, focus a <NumberField allowWheelScrub largeStep={10} /> and hold shift while turning the wheel. Today the value moves by 10; after this PR nothing happens.

Fix: Resolve the stepping axis instead of hard-requiring deltaY, e.g.

// Shift + wheel is delivered on the horizontal axis by some browsers.
const delta = event.deltaY !== 0 || !event.shiftKey ? event.deltaY : event.deltaX;
if (delta === 0) {
  return;
}
// ...
direction: delta > 0 ? -1 : 1,

and update the new shift test to assert that { deltaY: 0, deltaX: -100, shiftKey: true } steps by largeStep rather than being ignored. Please confirm the axis-swap behavior in a real Chrome/Safari build before merging — I could not run a browser here, and this is the part of the finding that depends on browser behavior rather than on the diff.

2. 🟠 deltaY === 0 misses most real horizontal trackpad swipes

Location: packages/react/src/number-field/root/NumberFieldRoot.tsx:355

Precision touchpads emit both axes on essentially every frame of a gesture; a "horizontal" two-finger swipe typically produces something like { deltaX: 42, deltaY: -0.5 }, not deltaY: 0. Only a synthetic event (or a discrete horizontal tilt wheel) hits exactly zero. So the scenario in the PR description — "prevent changing input value when user swipes horizontally" — still reproduces on a trackpad, and it reproduces in the worst way: the direction is decided by event.deltaY > 0 ? -1 : 1 on that sub-pixel noise, so the value jitters up and down at random during a purely sideways swipe, and the swipe is also preventDefaulted so the page no longer scrolls.

Failure scenario: On a MacBook trackpad, focus a <NumberField allowWheelScrub /> inside a horizontally scrollable container and swipe left. The container refuses to scroll and the number flickers between values.

Fix: Test the dominant axis rather than exact zero — Math.abs(event.deltaX) > Math.abs(event.deltaY) → ignore — combined with the shift handling from finding 1 (when shift is held, the horizontal axis is the intended vertical one, so exempt it from the dominant-axis check). Add a test for the realistic case, e.g. { deltaX: 100, deltaY: -0.5 }, since none of the four new tests cover a non-zero deltaY.

Simplifications (1)

1. 🟡 Three of the four new tests exercise the same branch with the same setup

Location: packages/react/src/number-field/root/NumberFieldRoot.test.tsx:1412

does not scrub on a horizontal wheel event, does not scrub on a wheel event with no delta, and does not block scrolling for a wheel event it ignores all render the same <NumberField defaultValue={5} allowWheelScrub />, focus it, and fire a wheel event that hits the exact same early return. The third one fires a byte-identical event to the first ({ deltaY: 0, deltaX: 100 }) and only adds the dispatchEvent return-value assertion.

Failure scenario: ~45 lines of test maintain a one-line guard; a future change to the guard forces edits in three near-duplicate places, and the extra cases give no additional branch coverage.

Fix: Fold them into one test that fires the horizontal wheel, asserts fireEvent.wheel(...) === true (not prevented), asserts the value is unchanged, and asserts onValueChange/onValueCommitted were not called. Spend the freed budget on the uncovered diagonal case from bug 2.

Docs (1)

1. 🟡 New guard clause has no comment explaining its intent

Location: packages/react/src/number-field/root/NumberFieldRoot.tsx:355

Every other term in this condition is self-describing or commented (// Allow pinch-zooming. above event.ctrlKey). event.deltaY === 0 reads as a trivial no-op check; the actual intent — "this is a horizontal-only wheel, let the page scroll instead of scrubbing" — is not recoverable from the code, and it is exactly the line most likely to be revisited when the shift-key case from bug 1 comes up.

Failure scenario: A future contributor reading the guard cannot tell whether the zero check is a horizontal-scroll exclusion or a micro-optimization, and removes or broadens it.

Fix: Add // Ignore horizontal wheel/trackpad gestures so the page can scroll. directly above the new term.

Verdict

Request changes - the deltaY === 0 guard disables the documented shift + wheel largeStep interaction on browsers that swap the wheel axis, while still letting real trackpad horizontal swipes through.

---

🤖 Review generated with Claude Code · Opus 5 (High) · medium review depth · 16 turns · 2m46s · $0.83 · run

[pkg-pr-new]

• base-ui-tanstack-start

• vite-css-base-ui-example

  pnpm add https://pkg.pr.new/mui/base-ui/@base-ui/react@5463
  

  pnpm add https://pkg.pr.new/mui/base-ui/@base-ui/utils@5463
  

commit: e93a07f

[code-infra-dashboard]

Bundle size

Bundle	Parsed size	Gzip size
@base-ui/react	🔺+103B(+0.02%)	🔺+35B(+0.02%)

Details of bundle changes

Performance

Total duration: 1,053.07 ms -38.15 ms^(-3.5%) | Renders: 76 ⁽⁺⁰⁾ | Paint: 1,667.07 ms -41.45 ms^(-2.4%)

No significant changes — details

---

Check out the code infra dashboard for more information about this PR.

[netlify]

✅ Deploy Preview for base-ui ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	e93a07f
🔍 Latest deploy log	https://app.netlify.com/projects/base-ui/deploys/6a7abe22da0a59000839b802
😎 Deploy Preview	https://deploy-preview-5463--base-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[sai6855]

@claude review

[github-actions]

PR review

The core fix is sound: pure and noisy horizontal wheel gestures now fall through to the page instead of scrubbing, and the axis-dominance comparison is the right shape for touchpad noise. Nothing here is merge-blocking, but the Shift path applies the same noise reasoning inconsistently — the delta axis is picked by deltaY !== 0 rather than by dominance, so a Shift-held sideways swipe can flip direction between consecutive events of one gesture. One tightened expression fixes that and shortens the code.

Bugs (1)

1. 🟠 Shift-held horizontal swipe picks its direction from sub-pixel cross-axis noise

Location: packages/react/src/number-field/root/NumberFieldRoot.tsx:361

const delta = event.deltaY !== 0 || !event.shiftKey ? event.deltaY : event.deltaX;

const isHorizontalGesture =
  !event.shiftKey && Math.abs(event.deltaX) > Math.abs(event.deltaY);

The new comment on isHorizontalGesture correctly notes that "touchpads emit sub-pixel noise on the cross axis, so compare the axes rather than requiring an exact zero" — but delta on the line above still uses an exact-zero test (event.deltaY !== 0) to decide which axis carries the intent. Because Shift is exempt from isHorizontalGesture, a Shift-held sideways swipe is never filtered out, and within one gesture the axis feeding direction alternates: events with deltaY === 0 read deltaX, and events with any noise on deltaY read that noise instead. Since deltaX and the noise typically have opposite signs, the step direction flips between neighbouring events of the same swipe.

Failure scenario: <NumberField allowWheelScrub largeStep={10} defaultValue={0} /> focused, user holds Shift and swipes right on a precision touchpad. The wheel events arrive as {deltaX: 100, deltaY: 0} → −10, {deltaX: 100, deltaY: -0.5} → +10, {deltaX: 100, deltaY: 0} → −10. The value oscillates by largeStep instead of moving one way, and every event is preventDefaulted so the horizontal scroll the user asked for is also swallowed.

Fix: derive both the axis choice and the filter from the same dominance test, which is also less code:

// Some browsers deliver shift + wheel on the horizontal axis, so there the horizontal
// delta is the intended vertical one. Touchpads emit sub-pixel noise on the cross axis,
// so compare the axes rather than requiring an exact zero.
const isHorizontal = Math.abs(event.deltaX) > Math.abs(event.deltaY);
const delta = event.shiftKey && isHorizontal ? event.deltaX : event.deltaY;

// Ignore horizontal gestures so the page can scroll instead of scrubbing. Shift is exempt:
// its gesture is horizontal wherever the browser swaps the axis.
if (delta === 0 || (!event.shiftKey && isHorizontal)) {
  return;
}

This keeps all five wheel cases the PR adds passing ({deltaX: -100, deltaY: 0, shiftKey} → +largeStep; {deltaY: -1, shiftKey} → +largeStep; the horizontal and noisy-horizontal cases still bail), and makes the Shift-held sideways swipe step consistently in one direction.

Tests (1)

1. 🟡 New wheel tests never assert the scrubbing path is still canceled, and skip the Shift + noise case

Location: packages/react/src/number-field/root/NumberFieldRoot.test.tsx:1434

// `fireEvent` returns false when the event was canceled with `preventDefault`.
expect(fireEvent.wheel(input, { deltaY: 0, deltaX: 100 })).toBe(true);

The preventDefault contract is now asserted only in the negative direction. No test asserts expect(fireEvent.wheel(input, { deltaY: 1 })).toBe(false), so a future change that stops canceling the vertical wheel event — page scrolls away underneath the user while the value scrubs, the exact bug the native listener at line 341 exists to prevent — passes the suite. Separately, the Shift branch is only exercised with a clean deltaY: 0 axis swap, which is why the direction inconsistency in finding 1 goes unnoticed.

Failure scenario: Removing event.preventDefault() from handleWheel leaves every test in describe('prop: allowWheelScrub') green; so does {deltaX: 100, deltaY: -0.5, shiftKey: true} stepping the value the wrong way.

Fix: add expect(fireEvent.wheel(input, { deltaY: 1 })).toBe(false); to 'scrubs on a vertical wheel event that carries horizontal noise', and extend the axis-swap test with a noisy Shift event asserting the same direction as its clean counterpart.

Verdict

Approve after nits — the horizontal-gesture filter is correct for the non-Shift case; the Shift branch's exact-zero axis test should be aligned with the dominance test before merge, and it shortens the code.

---

🤖 Review generated with Claude Code · Opus 5 (High) · medium review depth · 18 turns · 3m41s · $1.01 · run

[Copilot]

Pull request overview

This PR fixes an issue in Base UI’s NumberField wheel-scrubbing behavior where horizontal-dominant wheel gestures (e.g., trackpad sideways swipes or tilt wheels) could unintentionally step and commit the value while also preventing page scroll. It updates the wheel handler to ignore horizontal gestures, while still supporting browser-specific axis swapping when Shift is held.

Changes:

• Ignore horizontal-dominant wheel gestures so they don’t scrub (and don’t call preventDefault), allowing the page to scroll normally.
• When Shift is held and the browser swaps wheel axes, treat the horizontal delta as the intended “vertical” delta for scrubbing direction.
• Add/extend tests covering horizontal gestures, cross-axis noise, and Shift + swapped-axis behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
packages/react/src/number-field/root/NumberFieldRoot.tsx	Adds axis-dominance detection and uses an appropriate delta for wheel scrubbing direction while ignoring horizontal gestures.
packages/react/src/number-field/root/NumberFieldRoot.test.tsx	Adds coverage to ensure horizontal wheel events don’t scrub (and don’t cancel scrolling) and verifies Shift + swapped-axis behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.