fix(studio): clamp NumberField commits to their bounds - #5947

Thanks for this, and welcome. The bug is real: main's commit() hands the raw parsed number to onCommit, and the six new cases fail against it, so they are not vacuous. They also go in through the production path, a real keydown on the real input, which is what I wanted to see.

One question before I approve. commit() sets draft to the bounded value, but your description says Base UI owns the focused editing text until blur. If that holds, then after typing 150 in a max={100} field and pressing Enter the field still reads 150 while the caller already has 100, and the two only agree once you click away. The tests assert input.value after blur but never right after Enter, so they cannot tell the two behaviours apart. Could you add expect(input.value).toBe("100") immediately after the Enter fire? If it passes, that settles it and I am happy; if it fails, the bounded text needs to reach the input too.

Smaller note: with a misconfigured min > max your clamp returns max, while clampNumber in utils/studioHelpers returns min. components/ui deliberately imports nothing outside itself, so do not pull the helper in, but a short comment saying max wins would help.

Heads up that only the Graphite mergeability check has run at this head, so none of this has been exercised in CI yet.

Approving at bf345334. Both points from Miguel's review at 72898cc0 are resolved, and the fix holds up.

Miguel's two points:

• He asked for input.value to be checked right after Enter, not only after blur. The new cases now assert it right after the Enter keydown, with focus still on the input, and the code makes that true: commitFromText flushes the commit synchronously and writes the committed value back into the focused input.
• He asked for a comment saying max wins when min > max. It is there on the clamp line.

What I verified:

• Applied onto current main, all 69 tests in src/components/ui/ pass. With only NumberField.tsx reverted, all 6 new cases fail, including the "clamps back to the committed bound" case that checks no duplicate commit or tracking event fires.
• The clamp runs before the dedupe against committedRef, so Enter on an out-of-range value that lands on the current bound commits nothing, and the blur that follows stays a no-op.
• The write-back only happens for finite input, so unparseable text still takes the red invalid path untouched.

A small note, not blocking: the write-back uses String(committedRef.current), while Base UI formats the value itself on blur. For plain integers the two agree. If a field ever uses a locale or fraction format, the text right after Enter could briefly differ from what blur shows.

On the gate: Miguel's CHANGES_REQUESTED is still live at the older head, so this approval alone does not clear the PR until he re-reviews or it is dismissed.

Need: yes, out-of-range Enter commits reach callers today · Size: ok · Reuse: clampNumber exists in utils/studioHelpers, but components/ui deliberately imports nothing outside itself, so inlining is right · Simpler: already minimal

— Rames

Thanks for the quick turnaround, and for the Before and After captures. They made the change easy to check. The comment on conflicting bounds is exactly what I was after.

On the Enter display, I dug one level further, and the new write-back holds only until the field renders again. Base UI's input is controlled by its own text state, and after typing it stops syncing that state from value until blur. So input.value = String(committedRef.current) in commitFromText changes the DOM node but not Base UI's state, and the next render of the input puts the typed text back.

To reproduce it with the helpers in valueControls.test.tsx:

1. Type 150 into a max={100} field and press Enter. The field shows 100 (as your new test checks).
2. While the field is still focused, re-render it, for example a parent state bump, or a consumer whose onCommit updates its store in a microtask.
3. The field reads 150 again while the caller already has 100.

A consumer that calls setState straight from onCommit doesn't show it, because that render happens inside your flushSync, before the write.

Blur still settles on the right value, so nothing wrong reaches the caller. But this is the display mismatch from my earlier question, so I'd like it closed before merging.

A fix that worked when I tried it locally: push the text through Base UI's own change path instead of setting the DOM value directly, the same way the test file's type() helper does:

const text = String(committedRef.current);
if (input.value !== text) {
  Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")!.set!.call(input, text);
  input.dispatchEvent(new Event("input", { bubbles: true }));
}

With that change, all the existing NumberField cases still pass, including your no-duplicate-commit and tracking counts. A test that re-renders the field after Enter (a small wrapper with a state bump) then passes too, and it fails on the current head. Please add that test so this stays fixed.

CI is green at this head; the two red rows in the checks list are cancelled duplicate runs.

Verdict: REQUEST_CHANGES
Reasoning: The bounded value reaches the caller correctly, but the bounded text on screen reverts to the typed value on the next render while the field is focused, which is the mismatch the previous round asked to close.

Reviewed at 1d59e3013eeb72b6b95e91282620c53cf8c2acb8.

I checked the corrected Base UI lifecycle and the delayed-parent-update fixture. The native setter plus input event updates the controlled text state, not just the DOM value; clamp-before-dedupe and invalid handling remain intact. The focused suite passes and the independent source pass found no new blocker.

Exact-head valueControls suite passed 22 tests, including the focused Enter and delayed-parent-update case. Borrowed Base UI 1.7, Tailwind 4.3.3 and cn 0.2.4 were mapped in a scratch config, with one React singleton; no source edits. No real-browser visual acceptance.

Need: yes · Size: ok · Reuse: existing implementation and tests · Simpler: already bounded · Structure: fine
Risk: medium - controlled numeric input commit and rerender lifecycle.

Verdict: APPROVE

Reasoning: The actual Base UI rerender test now verifies the native setter and input-event fix.