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 (