Merge pull request #8033 from marmelab/fix-numberinput

Fix NumberInput state only changes on blur
marmelab · Aug 4, 2022 · 6327e43 · 6327e43
2 parents dea8aa5 + a953277
commit 6327e43
Show file tree

Hide file tree

Showing 4 changed files with 349 additions and 43 deletions.
diff --git a/packages/ra-ui-materialui/src/input/NumberInput.spec.tsx b/packages/ra-ui-materialui/src/input/NumberInput.spec.tsx
@@ -1,11 +1,11 @@
 import * as React from 'react';
 import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { useFormContext, useWatch } from 'react-hook-form';
 
 import { NumberInput } from './NumberInput';
 import { AdminContext } from '../AdminContext';
 import { SaveButton } from '../button';
 import { SimpleForm, Toolbar } from '../form';
-import { useFormContext, useWatch } from 'react-hook-form';
 
 describe('<NumberInput />', () => {
     const defaultProps = {
@@ -117,7 +117,10 @@ describe('<NumberInput />', () => {
             const formContext = useFormContext();
             return (
                 <code>
-                    {name}:{JSON.stringify(formContext.getFieldState(name))}
+                    {name}:
+                    {JSON.stringify(
+                        formContext.getFieldState(name, formContext.formState)
+                    )}
                 </code>
             );
         };
@@ -134,7 +137,7 @@ describe('<NumberInput />', () => {
                 'views:{"invalid":false,"isDirty":false,"isTouched":false}'
             );
         });
-        it('should return correct state when the field is touched', () => {
+        it('should return correct state when the field is dirty', () => {
             render(
                 <AdminContext>
                     <SimpleForm>
@@ -147,9 +150,26 @@ describe('<NumberInput />', () => {
                 'resources.posts.fields.views'
             ) as HTMLInputElement;
             fireEvent.change(input, { target: { value: '3' } });
+            screen.getByText(
+                'views:{"invalid":false,"isDirty":true,"isTouched":false}'
+            );
+        });
+        it('should return correct state when the field is touched', () => {
+            render(
+                <AdminContext>
+                    <SimpleForm>
+                        <NumberInput {...defaultProps} />
+                        <FieldState />
+                    </SimpleForm>
+                </AdminContext>
+            );
+            const input = screen.getByLabelText(
+                'resources.posts.fields.views'
+            ) as HTMLInputElement;
+            fireEvent.click(input);
             fireEvent.blur(input);
             screen.getByText(
-                'views:{"invalid":false,"isDirty":true,"isTouched":true}'
+                'views:{"invalid":false,"isDirty":false,"isTouched":true}'
             );
         });
         it('should return correct state when the field is invalid', async () => {
@@ -229,7 +249,6 @@ describe('<NumberInput />', () => {
             );
             const input = screen.getByLabelText('resources.posts.fields.views');
             fireEvent.change(input, { target: { value: '3' } });
-            fireEvent.blur(input);
             fireEvent.click(screen.getByText('ra.action.save'));
             await waitFor(() => {
                 expect(onSubmit).toHaveBeenCalledWith({ views: 3 });
@@ -252,7 +271,6 @@ describe('<NumberInput />', () => {
             );
             const input = screen.getByLabelText('resources.posts.fields.views');
             fireEvent.change(input, { target: { value: '' } });
-            fireEvent.blur(input);
             fireEvent.click(screen.getByText('ra.action.save'));
             await waitFor(() => {
                 expect(onSubmit).toHaveBeenCalledWith({ views: null });
@@ -280,7 +298,6 @@ describe('<NumberInput />', () => {
             );
             const input = screen.getByLabelText('resources.posts.fields.views');
             fireEvent.change(input, { target: { value: '3' } });
-            fireEvent.blur(input);
             await waitFor(() => {
                 expect(value).toEqual('3');
             });
@@ -304,7 +321,6 @@ describe('<NumberInput />', () => {
             );
             const input = screen.getByLabelText('resources.posts.fields.views');
             fireEvent.change(input, { target: { value: '3' } });
-            fireEvent.blur(input);
             expect(value).toEqual('3');
             fireEvent.click(screen.getByText('ra.action.save'));
             await waitFor(() => {
@@ -391,7 +407,6 @@ describe('<NumberInput />', () => {
             );
             const input = screen.getByLabelText('resources.posts.fields.views');
             fireEvent.change(input, { target: { value: '3' } });
-            fireEvent.blur(input);
 
             fireEvent.click(screen.getByText('ra.action.save'));
 
@@ -416,7 +431,6 @@ describe('<NumberInput />', () => {
             );
             const input = screen.getByLabelText('resources.posts.fields.views');
             fireEvent.change(input, { target: { value: '3' } });
-            fireEvent.blur(input);
 
             fireEvent.click(screen.getByText('ra.action.save'));
 

diff --git a/packages/ra-ui-materialui/src/input/NumberInput.stories.tsx b/packages/ra-ui-materialui/src/input/NumberInput.stories.tsx
@@ -1,6 +1,6 @@
 import * as React from 'react';
 import { required } from 'ra-core';
-import { useWatch } from 'react-hook-form';
+import { useFormState, useWatch, useFormContext } from 'react-hook-form';
 
 import { NumberInput } from './NumberInput';
 import { AdminContext } from '../AdminContext';
@@ -270,3 +270,57 @@ export const Sx = () => (
         </Create>
     </AdminContext>
 );
+
+const FormStateInspector = () => {
+    const {
+        touchedFields,
+        isDirty,
+        dirtyFields,
+        isValid,
+        errors,
+    } = useFormState();
+    return (
+        <div>
+            form state:&nbsp;
+            <code style={{ backgroundColor: 'lightgrey' }}>
+                {JSON.stringify({
+                    touchedFields,
+                    isDirty,
+                    dirtyFields,
+                    isValid,
+                    errors,
+                })}
+            </code>
+        </div>
+    );
+};
+
+const FieldStateInspector = ({ name = 'views' }) => {
+    const formContext = useFormContext();
+    return (
+        <div>
+            {name}:
+            <code style={{ backgroundColor: 'lightgrey' }}>
+                {JSON.stringify(
+                    formContext.getFieldState(name, formContext.formState)
+                )}
+            </code>
+        </div>
+    );
+};
+
+export const FieldState = () => (
+    <AdminContext>
+        <Create
+            resource="posts"
+            record={{ id: 123, views: 23 }}
+            sx={{ width: 600 }}
+        >
+            <SimpleForm>
+                <NumberInput source="views" />
+                <FormStateInspector />
+                <FieldStateInspector />
+            </SimpleForm>
+        </Create>
+    </AdminContext>
+);
diff --git a/packages/ra-ui-materialui/src/input/NumberInput.tsx b/packages/ra-ui-materialui/src/input/NumberInput.tsx
@@ -11,9 +11,6 @@ import { sanitizeInputRestProps } from './sanitizeInputRestProps';
 /**
  * An Input component for a number
  *
- * Due to limitations in React controlled components and number formatting,
- * this input only updates the form value on blur.
- *
  * @example
  * <NumberInput source="nb_views" />
  *
@@ -29,7 +26,6 @@ export const NumberInput = ({
     helperText,
     label,
     margin,
-    onBlur,
     onChange,
     parse,
     resource,
@@ -58,8 +54,9 @@ export const NumberInput = ({
 
     const inputProps = { ...overrideInputProps, step, min, max };
 
-    // This is a controlled input that doesn't transform the user input on change.
-    // The user input is only turned into a number on blur.
+    // This is a controlled input that renders directly the string typed by the user.
+    // This string is converted to a number on change, and stored in the form state,
+    // but that number is not not displayed.
     // This is to allow transitory values like '1.0' that will lead to '1.02'
 
     // text typed by the user and displayed in the input, unparsed
@@ -82,22 +79,8 @@ export const NumberInput = ({
         ) {
             return;
         }
-        setValue(event.target.value);
-    };
-
-    // set the numeric value on the form on blur
-    const handleBlur = (...event: any[]) => {
-        if (onBlur) {
-            onBlur(...event);
-        }
-        const eventParam = event[0] as React.FocusEvent<HTMLInputElement>;
-        if (
-            typeof eventParam.target === 'undefined' ||
-            typeof eventParam.target.value === 'undefined'
-        ) {
-            return;
-        }
-        const target = eventParam.target;
+        const target = event.target;
+        setValue(target.value);
         const newValue = target.valueAsNumber
             ? parse
                 ? parse(target.valueAsNumber)
@@ -106,24 +89,15 @@ export const NumberInput = ({
             ? parse(target.value)
             : convertStringToNumber(target.value);
         field.onChange(newValue);
-        field.onBlur();
-    };
-
-    const handleKeyUp = (event: React.KeyboardEvent<HTMLInputElement>) => {
-        if (event.key === 'Enter') {
-            handleBlur(event);
-        }
     };
 
     return (
         <TextField
             id={id}
             {...field}
-            // override the react-hook-form value, onChange and onBlur props
+            // use the locally controlled state instead of the react-hook-form field state
             value={value}
             onChange={handleChange}
-            onBlur={handleBlur}
-            onKeyUp={handleKeyUp}
             className={clsx('ra-input', `ra-input-${source}`, className)}
             type="number"
             size="small"