Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix <NumberInput> state only changes on blur #8033

Merged
merged 5 commits into from
Aug 4, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 24 additions & 10 deletions packages/ra-ui-materialui/src/input/NumberInput.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import * as React from 'react';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { useFormContext, useWatch, useFormState } from 'react-hook-form';
fzaninotto marked this conversation as resolved.
Show resolved Hide resolved

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 = {
Expand Down Expand Up @@ -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>
);
};
Expand All @@ -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>
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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 });
Expand All @@ -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 });
Expand Down Expand Up @@ -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');
});
Expand All @@ -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(() => {
Expand Down Expand Up @@ -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'));

Expand All @@ -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'));

Expand Down
56 changes: 55 additions & 1 deletion packages/ra-ui-materialui/src/input/NumberInput.stories.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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>
);
38 changes: 6 additions & 32 deletions packages/ra-ui-materialui/src/input/NumberInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
*
Expand All @@ -29,7 +26,6 @@ export const NumberInput = ({
helperText,
label,
margin,
onBlur,
onChange,
parse,
resource,
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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"
Expand Down