From d66f41345e0502132523434b5075052c2ebd63b9 Mon Sep 17 00:00:00 2001 From: Joe Date: Tue, 7 Jul 2026 12:13:49 -0700 Subject: [PATCH] Fire off number updates eagerly react-aria waits until a blur event to parse inputs and emit numbers. This is nice except that validation is stuck waiting for a blur event (so, e.g., all your form contents are valid, but the submit button is disabled, because we're waiting for you to blur your input). While it'd be nice for react-aria to support some kind of "eager mode", we can pretty easily write one ourselves, by forcing a `commit` whenever we get suspicious. --- app/ui/lib/NumberInput.spec.tsx | 115 ++++++++++++++++++++++++++++++++ app/ui/lib/NumberInput.tsx | 16 ++++- 2 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 app/ui/lib/NumberInput.spec.tsx diff --git a/app/ui/lib/NumberInput.spec.tsx b/app/ui/lib/NumberInput.spec.tsx new file mode 100644 index 000000000..ab26ba430 --- /dev/null +++ b/app/ui/lib/NumberInput.spec.tsx @@ -0,0 +1,115 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ +import { fireEvent, render } from '@testing-library/react' +import { useState } from 'react' +import * as R from 'remeda' +import { describe, expect, it, vi } from 'vitest' + +import { NumberInput } from './NumberInput' + +type Props = React.ComponentProps + +function Controlled({ onChange, ...props }: Props) { + const [value, setValue] = useState(props.value ?? NaN) + return ( + { + setValue(v) + onChange?.(v) + }} + /> + ) +} + +const getInput = (container: HTMLElement) => container.querySelector('input')! + +describe('NumberInput', () => { + it('fires onChange per keystroke with the parsed number', () => { + const onChange = vi.fn() + const { container } = render() + const input = getInput(container) + + fireEvent.change(input, { target: { value: '1' } }) + expect(onChange).toHaveBeenLastCalledWith(1) + + fireEvent.change(input, { target: { value: '12' } }) + expect(onChange).toHaveBeenLastCalledWith(12) + }) + + it('fires onChange with NaN when the input is cleared', () => { + const onChange = vi.fn() + const { container } = render() + const input = getInput(container) + + fireEvent.change(input, { target: { value: '' } }) + expect(onChange).toHaveBeenLastCalledWith(NaN) + }) + + it('clamps typed values above maxValue', () => { + const onChange = vi.fn() + const { container } = render( + + ) + const input = getInput(container) + + fireEvent.change(input, { target: { value: '150' } }) + expect(onChange).toHaveBeenLastCalledWith(100) + expect(input.value).toBe('100') + }) + + it('clamps typed values below minValue', () => { + const onChange = vi.fn() + const { container } = render() + const input = getInput(container) + + fireEvent.change(input, { target: { value: '0' } }) + expect(onChange).toHaveBeenLastCalledWith(1) + expect(input.value).toBe('1') + }) + + it('only simplifies numbers on blur', () => { + const onChange = vi.fn() + const { container } = render() + const input = getInput(container) + + fireEvent.change(input, { target: { value: '0' } }) + expect(onChange).toHaveBeenCalledTimes(1) + expect(onChange).toHaveBeenLastCalledWith(0) + expect(input.value).toBe('0') + + R.times(6, (precision) => { + const value = `1.${'0'.repeat(precision)}` // 1., 1.0, etc. + fireEvent.change(input, { target: { value } }) + expect(onChange).toHaveBeenCalledTimes(1) + expect(input.value).toBe(value) + }) + + fireEvent.blur(input) + expect(onChange).toHaveBeenCalledTimes(2) + expect(onChange).toHaveBeenLastCalledWith(1) + expect(input.value).toBe('1') + }) + + it('still controls the displayed value when onChange causes no re-render', () => { + const onChange = vi.fn() + const { container } = render() + const input = getInput(container) + + fireEvent.change(input, { target: { value: '1099' } }) + expect(onChange).toHaveBeenLastCalledWith(1023) + expect(input.value).toBe('1023') + + fireEvent.change(input, { target: { value: '10239' } }) + expect(onChange).toHaveBeenLastCalledWith(1023) + expect(input.value).toBe('1023') + }) +}) diff --git a/app/ui/lib/NumberInput.tsx b/app/ui/lib/NumberInput.tsx index 3792654b7..be326c475 100644 --- a/app/ui/lib/NumberInput.tsx +++ b/app/ui/lib/NumberInput.tsx @@ -6,7 +6,7 @@ * Copyright Oxide Computer Company */ import cn from 'classnames' -import { useRef, type Ref } from 'react' +import { useEffect, useRef, type Ref } from 'react' import { useButton, useLocale, @@ -31,6 +31,20 @@ export function NumberInput(props: NumberInputProps) { const { groupProps, inputProps, incrementButtonProps, decrementButtonProps } = useNumberField(props, state, inputRef) + // react-aria only fires props.onChange on commit (blur / Enter / stepper), + // but we want form state to update as soon as it would produce a different + // field value. Committing whenever state.inputValue changes to an + // unambiguous number lets react-aria keep controlling parsing, clamping + // etc., but forces it to be more eager. + // + // Context: https://github.com/adobe/react-spectrum/issues/7984 + useEffect(() => { + if (String(Number(state.inputValue)) === state.inputValue || state.inputValue === '') { + state.commit() + } + // eslint-disable-next-line exhaustive-deps + }, [state.inputValue]) + return (