Skip to content

Commit 79e37b5

Browse files
fix(NumberInput): ensure controlled value sync for type=number (#20218)
* fix(NumberInput): ensure controlled value sync for type=number * chore: remove test stories * chore: remove unused code in stories --------- Co-authored-by: Taylor Jones <taylor.jones826@gmail.com> Co-authored-by: Taylor Jones <tay1orjones@users.noreply.github.com>
1 parent cfc41d0 commit 79e37b5

File tree

3 files changed

+37
-32
lines changed

3 files changed

+37
-32
lines changed

packages/react/src/components/NumberInput/NumberInput.stories.js

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -165,20 +165,6 @@ export const withAILabel = (args) => {
165165

166166
withAILabel.argTypes = { ...sharedArgTypes };
167167

168-
// TODO: for testing. remove before marking ready for review
169-
const handleOnChange = (event, state) => {
170-
console.log(`-------onChange--------`);
171-
console.log(event.target);
172-
console.log(state?.value);
173-
console.log(state?.direction);
174-
};
175-
176-
// TODO: for testing. remove before marking ready for review
177-
const handleOnBlur = (event) => {
178-
console.log(`--------onBlur--------`);
179-
console.log(event.target);
180-
};
181-
182168
export const WithTypeOfText = (args) => {
183169
const locale = useDocumentLang();
184170

@@ -193,8 +179,6 @@ export const WithTypeOfText = (args) => {
193179
helperText="Optional helper text."
194180
{...args}
195181
locale={locale}
196-
onChange={handleOnChange}
197-
onBlur={handleOnBlur}
198182
/>
199183
);
200184
};
@@ -217,7 +201,6 @@ WithTypeOfText.argTypes = {
217201
...sharedArgTypes,
218202
};
219203

220-
// TODO: for testing, remove before marking ready for review
221204
export const WithTypeOfTextControlled = (args) => {
222205
const locale = useDocumentLang();
223206
const [value, setValue] = useState(NaN);
@@ -236,14 +219,8 @@ export const WithTypeOfTextControlled = (args) => {
236219
locale={locale}
237220
value={value}
238221
onChange={(event, state) => {
239-
handleOnChange(event, state);
240-
console.log(`setting value to:`);
241-
console.log(state.value);
242222
setValue(state.value);
243223
}}
244-
onBlur={(event) => {
245-
handleOnBlur(event);
246-
}}
247224
/>
248225
<button
249226
type="button"

packages/react/src/components/NumberInput/NumberInput.tsx

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -445,15 +445,16 @@ const NumberInput = React.forwardRef<HTMLInputElement, NumberInputProps>(
445445
[`${prefix}--number__invalid--warning`]: normalizedProps.warn,
446446
});
447447

448-
if (
449-
controlledValue !== prevControlledValue &&
450-
!(isNaN(Number(controlledValue)) === isNaN(Number(prevControlledValue)))
451-
) {
452-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
453-
setValue(controlledValue!);
454-
setPrevControlledValue(controlledValue);
455-
}
456-
448+
useEffect(() => {
449+
if (type === 'number' && controlledValue !== undefined) {
450+
if (allowEmpty && controlledValue === '') {
451+
setValue('');
452+
} else {
453+
setValue(controlledValue);
454+
}
455+
setPrevControlledValue(controlledValue);
456+
}
457+
}, [controlledValue, type, allowEmpty]);
457458
let ariaDescribedBy: string | undefined = undefined;
458459
if (normalizedProps.invalid) {
459460
ariaDescribedBy = normalizedProps.invalidId;

packages/react/src/components/NumberInput/__tests__/NumberInput-test.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,33 @@ describe('NumberInput', () => {
202202
expect(onChange).toHaveBeenCalledTimes(2);
203203
});
204204

205+
it('should update when value prop changes externally with type="number"', async () => {
206+
const ControlledNumberInput = () => {
207+
const [value, setValue] = useState(5);
208+
return (
209+
<>
210+
<NumberInput
211+
type="number"
212+
label="NumberInput label"
213+
id="number-input"
214+
value={value}
215+
onChange={(e, state) => setValue(state.value)}
216+
translateWithId={translateWithId}
217+
/>
218+
<button onClick={() => setValue(10)}>set to 10</button>
219+
</>
220+
);
221+
};
222+
223+
render(<ControlledNumberInput />);
224+
225+
const input = screen.getByLabelText('NumberInput label');
226+
expect(input).toHaveValue(5);
227+
228+
await userEvent.click(screen.getByText('set to 10'));
229+
expect(input).toHaveValue(10);
230+
});
231+
205232
describe('steppers', () => {
206233
it('should call `onClick` when up or down arrows are clicked', async () => {
207234
const onClick = jest.fn();

0 commit comments

Comments
 (0)