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 form default values #6272

Merged
merged 4 commits into from
May 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion cypress/support/CreatePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ export default url => ({
if (clearPreviousValue) {
cy.get(this.elements.input(name, type)).clear();
}
cy.get(this.elements.input(name, type)).type(value);
cy.get(this.elements.input(name, type)).type(
`${clearPreviousValue ? '{selectall}' : ''}${value}`
);
if (type === 'rich-text-input') {
cy.wait(500);
}
Expand Down
131 changes: 130 additions & 1 deletion packages/ra-core/src/form/FormWithRedirect.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as React from 'react';
import { renderWithRedux } from 'ra-test';
import FormWithRedirect from './FormWithRedirect';
import useInput from './useInput';
import { waitFor } from '@testing-library/dom';

describe('FormWithRedirect', () => {
const Input = props => {
Expand Down Expand Up @@ -39,8 +40,57 @@ describe('FormWithRedirect', () => {
/>
);

expect(renderProp.mock.calls[1][0].pristine).toEqual(true);
expect(renderProp.mock.calls[3][0].pristine).toEqual(true);
expect(getByDisplayValue('Foo')).not.toBeNull();
// 4 times because the first initialization with an empty value
// triggers a change on the input which has a defaultValue
// This is expected and identical to what FinalForm does (https://final-form.org/docs/final-form/types/FieldConfig#defaultvalue)
expect(renderProp).toHaveBeenCalledTimes(4);
});

it('Does not make the form dirty when initialized from a record with a missing field and this field has an initialValue', () => {
const renderProp = jest.fn(() => (
<Input source="name" initialValue="Bar" />
));
const { getByDisplayValue } = renderWithRedux(
<FormWithRedirect
save={jest.fn()}
redirect={false}
saving={false}
version={0}
render={renderProp}
record={{ id: 1 }}
/>
);

expect(renderProp.mock.calls[0][0].pristine).toEqual(true);
expect(getByDisplayValue('Bar')).not.toBeNull();
// 4 times because the first initialization with an empty value
// triggers a change on the input which has a defaultValue
// This is expected and identical to what FinalForm does (https://final-form.org/docs/final-form/types/FieldConfig#defaultvalue)
expect(renderProp).toHaveBeenCalledTimes(1);
});

it('Makes the form dirty when initialized from a record with a missing field and this field has a defaultValue', () => {
const renderProp = jest.fn(() => (
<Input source="name" defaultValue="Bar" />
));
const { getByDisplayValue } = renderWithRedux(
<FormWithRedirect
save={jest.fn()}
redirect={false}
saving={false}
version={0}
render={renderProp}
record={{ id: 1 }}
/>
);

expect(renderProp.mock.calls[1][0].pristine).toEqual(false);
expect(getByDisplayValue('Bar')).not.toBeNull();
// 4 times because the first initialization with an empty value
// triggers a change on the input which has a defaultValue
// This is expected and identical to what FinalForm does (https://final-form.org/docs/final-form/types/FieldConfig#defaultvalue)
expect(renderProp).toHaveBeenCalledTimes(2);
});

Expand Down Expand Up @@ -77,4 +127,83 @@ describe('FormWithRedirect', () => {
expect(getByDisplayValue('Foo')).not.toBeNull();
expect(renderProp).toHaveBeenCalledTimes(2);
});

it('Makes the form dirty when reinitialized from a different record with a missing field and this field has a defaultValue', () => {
const renderProp = jest.fn(() => (
<Input source="name" defaultValue="Bar" />
));
const { getByDisplayValue, rerender } = renderWithRedux(
<FormWithRedirect
save={jest.fn()}
redirect={false}
saving={false}
version={0}
record={{ id: 1, name: 'Foo' }}
render={renderProp}
/>
);

expect(renderProp.mock.calls[0][0].pristine).toEqual(true);
expect(getByDisplayValue('Foo')).not.toBeNull();

rerender(
<FormWithRedirect
save={jest.fn()}
redirect={false}
saving={false}
version={0}
record={{
id: 1,
anotherServerAddedProp: 'Bar',
}}
render={renderProp}
/>
);

expect(renderProp.mock.calls[2][0].pristine).toEqual(false);
expect(getByDisplayValue('Bar')).not.toBeNull();
expect(renderProp).toHaveBeenCalledTimes(3);
});

it('Does not make the form dirty when reinitialized from a different record with a missing field and this field has an initialValue', async () => {
const renderProp = jest.fn(() => (
<Input source="name" initialValue="Bar" />
));
const { getByDisplayValue, rerender } = renderWithRedux(
<FormWithRedirect
save={jest.fn()}
redirect={false}
saving={false}
version={0}
record={{ id: 1, name: 'Foo' }}
render={renderProp}
/>
);

expect(renderProp.mock.calls[0][0].pristine).toEqual(true);
expect(getByDisplayValue('Foo')).not.toBeNull();

rerender(
<FormWithRedirect
save={jest.fn()}
redirect={false}
saving={false}
version={0}
record={{
id: 1,
name: undefined,
anotherServerAddedProp: 'Bar',
}}
render={renderProp}
/>
);

await waitFor(() => {
expect(getByDisplayValue('Bar')).not.toBeNull();
expect(
renderProp.mock.calls[renderProp.mock.calls.length - 1][0]
.pristine
).toEqual(false);
});
});
});
8 changes: 3 additions & 5 deletions packages/ra-core/src/form/FormWithRedirect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,9 @@ const FormWithRedirect = ({
[setOnSave]
);

const finalInitialValues = getFormInitialValues(
initialValues,
defaultValue,
record
);
const finalInitialValues = useMemo(
() => getFormInitialValues(initialValues, defaultValue, record),
[JSON.stringify({initialValues, defaultValue, record})]); // eslint-disable-line

const submit = values => {
const finalRedirect =
Expand Down
14 changes: 9 additions & 5 deletions packages/ra-core/src/form/getFormInitialValues.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import merge from 'lodash/merge';

export default function getFormInitialValues(
initialValues,
defaultValue,
Expand All @@ -9,11 +11,13 @@ export default function getFormInitialValues(
);
}

return {
...getValues(defaultValue, record),
...getValues(initialValues, record),
...record,
};
const finalInitialValues = merge(
{},
getValues(defaultValue, record),
getValues(initialValues, record),
record
);
return finalInitialValues;
}

function getValues(values, record) {
Expand Down
16 changes: 12 additions & 4 deletions packages/ra-core/src/form/useInitializeFormWithRecord.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useEffect } from 'react';
import { useForm } from 'react-final-form';
import merge from 'lodash/merge';
import isEqual from 'lodash/isEqual';
import getFormInitialValues from './getFormInitialValues';

/**
* Restore the record values which should override any default values specified on the form.
Expand All @@ -13,13 +14,20 @@ const useInitializeFormWithRecord = record => {
return;
}

const initialValues = form.getState().initialValues;
const initialValuesMergedWithRecord = merge({}, initialValues, record);
const initialValues = getFormInitialValues(
form.getState().initialValues,
undefined,
record
);

if (isEqual(form.getState().initialValues, initialValues)) {
return;
}

// Disable this option when re-initializing the form because in this case, it should reset the dirty state of all fields
// We do need to keep this option for dynamically added inputs though which is why it is kept at the form level
form.setConfig('keepDirtyOnReinitialize', false);
form.restart(initialValuesMergedWithRecord);
form.restart(initialValues);
form.setConfig('keepDirtyOnReinitialize', true);
}, [form, JSON.stringify(record)]); // eslint-disable-line react-hooks/exhaustive-deps
};
Expand Down
30 changes: 28 additions & 2 deletions packages/ra-core/src/form/useInput.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { useCallback, ChangeEvent, FocusEvent, useEffect } from 'react';
import {
useField as useFinalFormField,
FieldProps,
FieldRenderProps,
FieldInputProps,
useForm,
} from 'react-final-form';
import { Validator, composeValidators } from './validate';
import isRequired from './isRequired';
import { useCallback, ChangeEvent, FocusEvent, useEffect } from 'react';
import { useFormGroupContext } from './useFormGroupContext';
import { useFormContext } from './useFormContext';
import { useRecordContext } from '../controller';

export interface InputProps<T = any>
extends Omit<
Expand Down Expand Up @@ -51,6 +53,7 @@ const useInput = ({
const finalName = name || source;
const formGroupName = useFormGroupContext();
const formContext = useFormContext();
const record = useRecordContext();

useEffect(() => {
if (!formContext || !formGroupName) {
Expand All @@ -68,7 +71,8 @@ const useInput = ({
: validate;

const { input, meta } = useFinalFormField(finalName, {
initialValue: initialValue || defaultValue,
initialValue,
defaultValue,
validate: sanitizedValidate,
...options,
});
Expand Down Expand Up @@ -107,6 +111,28 @@ const useInput = ({
[onFocus, customOnFocus]
);

// Every time the record changes and didn't include a value for this field
const form = useForm();
const recordId = record?.id;
useEffect(() => {
if (!!input.value) {
return;
}
// Apply the default value if provided
// We use a change here which will make the form dirty but this is expected
// and identical to what FinalForm does (https://final-form.org/docs/final-form/types/FieldConfig#defaultvalue)
if (!!defaultValue) {
form.change(source, defaultValue);
}

if (!!initialValue) {
form.batch(() => {
form.change(source, initialValue);
form.resetFieldState(source);
});
}
}, [recordId, input.value, defaultValue, initialValue, source, form]);

// If there is an input prop, this input has already been enhanced by final-form
// This is required in for inputs used inside other inputs (such as the SelectInput inside a ReferenceInput)
if (options.input) {
Expand Down
1 change: 1 addition & 0 deletions packages/ra-ui-materialui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"final-form-arrays": "^3.0.2",
"ignore-styles": "~5.0.1",
"ra-core": "^3.15.1",
"ra-test": "^3.15.1",
"react": "^17.0.0",
"react-dom": "^17.0.0",
"react-final-form": "^6.5.2",
Expand Down
24 changes: 22 additions & 2 deletions packages/ra-ui-materialui/src/input/DateInput.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import * as React from 'react';
import expect from 'expect';
import { render, fireEvent } from '@testing-library/react';
import { Form } from 'react-final-form';
import { required, FormWithRedirect } from 'ra-core';
import { renderWithRedux } from 'ra-test';
import format from 'date-fns/format';

import DateInput from './DateInput';
import { Form } from 'react-final-form';
import { required } from 'ra-core';
import { FormApi } from 'final-form';

describe('<DateInput />', () => {
const defaultProps = {
Expand All @@ -24,6 +27,23 @@ describe('<DateInput />', () => {
);
});

it('should not make the form dirty on initialization', () => {
const publishedAt = new Date().toISOString();
let formApi: FormApi;
const { getByDisplayValue } = renderWithRedux(
<FormWithRedirect
onSubmit={jest.fn}
record={{ id: 1, publishedAt }}
render={({ form }) => {
formApi = form;
return <DateInput {...defaultProps} />;
}}
/>
);
expect(getByDisplayValue(format(publishedAt, 'YYYY-MM-DD')));
expect(formApi.getState().dirty).toEqual(false);
});

it('should call `input.onChange` method when changed', () => {
let formApi;
const { getByLabelText } = render(
Expand Down
15 changes: 4 additions & 11 deletions packages/ra-ui-materialui/src/input/DateInput.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as React from 'react';
import { useEffect } from 'react';
import PropTypes from 'prop-types';
import TextField, { TextFieldProps } from '@material-ui/core/TextField';
import { useInput, FieldTitle, InputProps } from 'ra-core';
Expand Down Expand Up @@ -49,6 +48,7 @@ const DateInput = ({
format = getStringFromDate,
initialValue,
label,
name,
options,
source,
resource,
Expand All @@ -74,6 +74,7 @@ const DateInput = ({
format,
formatOnBlur: true,
initialValue: sanitizedInitialValue,
name,
onBlur,
onChange,
onFocus,
Expand All @@ -86,21 +87,13 @@ const DateInput = ({

const { error, submitError, touched } = meta;

// Workaround for https://github.com/final-form/react-final-form/issues/431
useEffect(() => {
// Checking for meta.initial allows the format function to work
// on inputs inside an ArrayInput
if (defaultValue || initialValue || meta.initial) {
input.onBlur();
}
}, [input.onBlur, meta.initial]); // eslint-disable-line

return (
<TextField
id={id}
{...input}
// Workaround https://github.com/final-form/react-final-form/issues/529
value={input.value || ''}
// & https://github.com/final-form/react-final-form/issues/431
value={format(input.value) || ''}
variant={variant}
margin={margin}
type="date"
Expand Down
Loading