feat(components): add onValueChange prop to InputNumberExperimental#3147
Merged
nad182 merged 3 commits intoMay 12, 2026
Merged
Conversation
- Some consumers need UI to reflect in-progress input (e.g. a live subtotal that updates as the user types a discount), which `onChange`'s commit-only timing deliberately suppresses - The new prop wires through to Base UI's per-keystroke `onValueChange`; `onChange` keeps its commit semantics
Deploying atlantis with
|
| Latest commit: |
0166f37
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e5efb73c.atlantis.pages.dev |
| Branch Preview URL: | https://job-145723-input-number-expe-q6ah.atlantis.pages.dev |
There was a problem hiding this comment.
Pull request overview
Adds an opt-in onValueChange callback to InputNumberExperimental so consumers can react to per-keystroke (in-progress) parsed numeric updates, while preserving the existing commit-only onChange behavior.
Changes:
- Added
onValueChange?: (newValue: number | undefined) => voidtoInputNumberExperimentalPropswith updated JSDoc guidance. - Wired
onValueChangethrough to Base UINumberField.Root.onValueChange(mappingnull -> undefined). - Added a new
onValueChange contracttest block covering per-keystroke typing, clearing, and ensuringonChangedoesn’t fire while typing when both callbacks are provided.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/components/src/primitives/InputNumberExperimental/types.ts | Adds the new onValueChange prop and updates callback documentation. |
| packages/components/src/primitives/InputNumberExperimental/InputNumberExperimental.tsx | Wires onValueChange to Base UI’s NumberField and adds a lint disable for function size. |
| packages/components/src/primitives/InputNumberExperimental/tests/InputNumberExperimental.test.tsx | Adds tests defining the onValueChange behavior and interaction with onChange. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+87
to
+89
| * Fires on every parsed change as the user types. Prefer `onChange` for | ||
| * side effects; reach for this only when UI must reflect in-progress input | ||
| * (e.g. a live-updating subtotal). |
Comment on lines
+438
to
+442
| describe("onValueChange contract", () => { | ||
| it("fires per keystroke with the parsed number while the user types", async () => { | ||
| const user = userEvent.setup(); | ||
| const onValueChange = jest.fn(); | ||
| render( |
Comment on lines
+438
to
+505
| describe("onValueChange contract", () => { | ||
| it("fires per keystroke with the parsed number while the user types", async () => { | ||
| const user = userEvent.setup(); | ||
| const onValueChange = jest.fn(); | ||
| render( | ||
| <InputNumberExperimental | ||
| onValueChange={onValueChange} | ||
| placeholder="Count" | ||
| />, | ||
| ); | ||
|
|
||
| await user.type(screen.getByRole("textbox", { name: "Count" }), "42"); | ||
|
|
||
| expect(onValueChange).toHaveBeenCalledTimes(2); | ||
| expect(onValueChange).toHaveBeenNthCalledWith(1, 4); | ||
| expect(onValueChange).toHaveBeenNthCalledWith(2, 42); | ||
| }); | ||
|
|
||
| it("emits undefined when the field is cleared", async () => { | ||
| const user = userEvent.setup(); | ||
| const onValueChange = jest.fn(); | ||
| render( | ||
| <InputNumberExperimental | ||
| onValueChange={onValueChange} | ||
| placeholder="Count" | ||
| value={5} | ||
| />, | ||
| ); | ||
|
|
||
| await user.clear(screen.getByRole("textbox", { name: "Count" })); | ||
|
|
||
| expect(onValueChange).toHaveBeenLastCalledWith(undefined); | ||
| }); | ||
|
|
||
| it("does not fire onChange while the user is typing even when onValueChange is set", async () => { | ||
| const user = userEvent.setup(); | ||
| const onChange = jest.fn(); | ||
| const onValueChange = jest.fn(); | ||
| render( | ||
| <InputNumberExperimental | ||
| onChange={onChange} | ||
| onValueChange={onValueChange} | ||
| placeholder="Count" | ||
| />, | ||
| ); | ||
|
|
||
| await user.type(screen.getByRole("textbox", { name: "Count" }), "42"); | ||
|
|
||
| expect(onValueChange).toHaveBeenCalled(); | ||
| expect(onChange).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("fires on stepper interactions with the new value", async () => { | ||
| const user = userEvent.setup(); | ||
| const onValueChange = jest.fn(); | ||
| render( | ||
| <InputNumberExperimental | ||
| onValueChange={onValueChange} | ||
| placeholder="Count" | ||
| value={3} | ||
| />, | ||
| ); | ||
|
|
||
| await user.click(screen.getByRole("button", { name: "Increase Count" })); | ||
|
|
||
| expect(onValueChange).toHaveBeenLastCalledWith(4); | ||
| }); | ||
| }); |
Comment on lines
+88
to
+89
| * Prefer `onChange` for side effects; reach for this only when UI must | ||
| * reflect in-progress input (e.g. a live-updating subtotal). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why Is This Changing?
InputNumberExperimental.onChangedeliberately commits on blur / Enter / stepper / arrow step to mirror V1 / V2InputNumbertiming. That keeps side effects like network requests, URL updates, and form-state writes from firing on every keystroke, which is the right default for the vast majority of consumers.It's wrong for one legitimate case: UI that needs to reflect in-progress input as the user types (e.g. a subtotal that updates live as a discount is typed). Today there's no first-party way to opt in, so consumers reach for raw
onKeyUpand parseevent.currentTarget.valuethemselves, losing the parsed-number / empty-input contract thatonChangeprovides.What Is Changing?
New opt-in
onValueChangeprop. Same(newValue: number | undefined) => voidsignature asonChange, fires per keystroke (also on stepper / paste). Wires straight through to Base UI'sNumberField.onValueChange.onChange's commit-only timing is unchanged.Also tightened the verbose
onChangeJSDoc and added a sibling JSDoc ononValueChangethat points consumers to the right callback for their use case.Consumer Impact
Validation
InputNumberExperimental.test.tsxunder a newonValueChange contractdescribe:undefinedwhen the field is clearedonChange's commit-only timing when both callbacks are wiredmax-statementswas already at the threshold for this function; added an inline disable matching the existing precedent inMenu.Legacy.tsxReviewer Notes
onValueChange) so the wiring is mechanical and the API matches what most React devs reading the source would expectonChange