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 input validation and dirty state after re-enabling disabled fields #10163

Merged
merged 2 commits into from
Aug 30, 2024
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
2 changes: 1 addition & 1 deletion examples/simple/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"react": "^18.3.1",
"react-admin": "^5.1.2",
"react-dom": "^18.3.1",
"react-hook-form": "^7.52.0",
"react-hook-form": "^7.53.0",
"react-router": "^6.22.0",
"react-router-dom": "^6.22.0"
},
Expand Down
4 changes: 2 additions & 2 deletions packages/ra-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"jscodeshift": "^0.15.2",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-hook-form": "^7.52.0",
"react-hook-form": "^7.53.0",
"react-router": "^6.25.1",
"react-router-dom": "^6.25.1",
"react-test-renderer": "^18.2.0",
Expand All @@ -53,7 +53,7 @@
"peerDependencies": {
"react": "^18.0.0 || ^19.0.0",
"react-dom": "^18.0.0 || ^19.0.0",
"react-hook-form": "^7.52.0",
"react-hook-form": "^7.53.0",
"react-router": "^6.22.0",
"react-router-dom": "^6.22.0"
},
Expand Down
6 changes: 4 additions & 2 deletions packages/ra-core/src/form/useFormGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,14 @@ export const useFormGroup = (name: string): FormGroupState => {
const { dirtyFields, touchedFields, validatingFields, errors } =
useFormState();

// dirtyFields, touchedFields and validatingFields are objects with keys being the field names
// dirtyFields, touchedFields, validatingFields and errors are objects with keys being the field names
// Ex: { title: true }
// However, they are not correctly serialized when using JSON.stringify
// To avoid our effects to not be triggered when they should, we extract the keys and use that as a dependency
const dirtyFieldsNames = Object.keys(dirtyFields);
const touchedFieldsNames = Object.keys(touchedFields);
const validatingFieldsNames = Object.keys(validatingFields);
const errorsNames = Object.keys(errors);

const formGroups = useFormGroups();
const [state, setState] = useState<FormGroupState>({
Expand Down Expand Up @@ -121,7 +122,8 @@ export const useFormGroup = (name: string): FormGroupState => {
[
// eslint-disable-next-line react-hooks/exhaustive-deps
JSON.stringify(dirtyFieldsNames),
errors,
// eslint-disable-next-line react-hooks/exhaustive-deps
JSON.stringify(errorsNames),
// eslint-disable-next-line react-hooks/exhaustive-deps
JSON.stringify(touchedFieldsNames),
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down
90 changes: 89 additions & 1 deletion packages/ra-core/src/form/useInput.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useFormContext, useWatch } from 'react-hook-form';
import { CoreAdminContext, SourceContextProvider } from '../core';
import { testDataProvider } from '../dataProvider';
import { Form } from './Form';
import { useInput, InputProps } from './useInput';
import { useInput, InputProps, UseInputValue } from './useInput';
import { required } from './validate';

const Input: FunctionComponent<
Expand Down Expand Up @@ -566,5 +566,93 @@ describe('useInput', () => {
);
});
});

it('should validate and be dirty for inputs that were disabled and re-enabled', async () => {
let inputProps: UseInputValue | undefined;

const DisabledEnableInput = () => {
const [disabled, setDisabled] = React.useState(false);

return (
<>
<button
type="button"
onClick={() => setDisabled(disabled => !disabled)}
>
Toggle
</button>
<Input
source="title"
resource="posts"
validate={required()}
disabled={disabled}
>
{props => {
inputProps = props; // Capture the latest props
return (
<input
type="text"
id={props.id}
aria-label="Title"
{...props.field}
/>
);
}}
</Input>
</>
);
};

render(
<CoreAdminContext dataProvider={testDataProvider()}>
<Form onSubmit={jest.fn()} mode="onChange">
<DisabledEnableInput />
</Form>
</CoreAdminContext>
);

// Initial state assertions
expect(inputProps?.fieldState.isDirty).toBe(false);
expect(inputProps?.field.disabled).toBe(false);

// Disable the input
fireEvent.click(screen.getByText('Toggle'));

await waitFor(() => {
expect(inputProps?.fieldState.isDirty).toBe(false);
expect(inputProps?.field.disabled).toBe(true);
});

// Re-enable the input
fireEvent.click(screen.getByText('Toggle'));

await waitFor(() => {
expect(inputProps?.fieldState.isDirty).toBe(false);
expect(inputProps?.field.disabled).toBe(false);
});

// Type in the input
fireEvent.change(screen.getByLabelText('Title'), {
target: { value: 'A title' },
});

// Assert that the field is now dirty
await waitFor(() => {
expect(inputProps?.fieldState.isDirty).toBe(true); // Now the input should be dirty
expect(inputProps?.field.value).toBe('A title');
});

// Clear the input
fireEvent.change(screen.getByLabelText('Title'), {
target: { value: '' },
});

// Assert that the field is now dirty and invalid because it is required
await waitFor(() => {
expect(inputProps?.fieldState.isDirty).toBe(true); // Now the input should be dirty
expect(inputProps?.field.value).toBe('');
expect(inputProps?.fieldState.invalid).toBe(true);
});
});
});
});
4 changes: 0 additions & 4 deletions packages/ra-core/src/form/useInput.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ export const useInput = <ValueType = any>(
},
},
...options,
// Workaround for https://github.com/react-hook-form/react-hook-form/issues/10907
// FIXME - remove when fixed
// @ts-ignore - only exists since react-hook-form 7.46.0
disabled: options.disabled || undefined,
});

// Because our forms may receive an asynchronously loaded record for instance,
Expand Down
2 changes: 1 addition & 1 deletion packages/ra-input-rich-text/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"ra-ui-materialui": "^5.1.2",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-hook-form": "^7.52.0",
"react-hook-form": "^7.53.0",
"rimraf": "^3.0.2",
"tippy.js": "^6.3.7",
"typescript": "^5.1.3"
Expand Down
2 changes: 1 addition & 1 deletion packages/ra-ui-materialui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"ra-language-english": "^5.1.2",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-hook-form": "^7.52.0",
"react-hook-form": "^7.53.0",
"react-is": "^18.2.0",
"react-router": "^6.25.1",
"react-router-dom": "^6.25.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/react-admin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"ra-i18n-polyglot": "^5.1.2",
"ra-language-english": "^5.1.2",
"ra-ui-materialui": "^5.1.2",
"react-hook-form": "^7.52.0",
"react-hook-form": "^7.53.0",
"react-router": "^6.22.0",
"react-router-dom": "^6.22.0"
},
Expand Down
20 changes: 10 additions & 10 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -17716,7 +17716,7 @@ __metadata:
react: "npm:^18.3.1"
react-dom: "npm:^18.3.1"
react-error-boundary: "npm:^4.0.13"
react-hook-form: "npm:^7.52.0"
react-hook-form: "npm:^7.53.0"
react-is: "npm:^18.2.0"
react-router: "npm:^6.25.1"
react-router-dom: "npm:^6.25.1"
Expand All @@ -17729,7 +17729,7 @@ __metadata:
peerDependencies:
react: ^18.0.0 || ^19.0.0
react-dom: ^18.0.0 || ^19.0.0
react-hook-form: ^7.52.0
react-hook-form: ^7.53.0
react-router: ^6.22.0
react-router-dom: ^6.22.0
languageName: unknown
Expand Down Expand Up @@ -17896,7 +17896,7 @@ __metadata:
ra-ui-materialui: "npm:^5.1.2"
react: "npm:^18.3.1"
react-dom: "npm:^18.3.1"
react-hook-form: "npm:^7.52.0"
react-hook-form: "npm:^7.53.0"
rimraf: "npm:^3.0.2"
tippy.js: "npm:^6.3.7"
typescript: "npm:^5.1.3"
Expand Down Expand Up @@ -17993,7 +17993,7 @@ __metadata:
react-dom: "npm:^18.3.1"
react-dropzone: "npm:^14.2.3"
react-error-boundary: "npm:^4.0.13"
react-hook-form: "npm:^7.52.0"
react-hook-form: "npm:^7.53.0"
react-is: "npm:^18.2.0"
react-router: "npm:^6.25.1"
react-router-dom: "npm:^6.25.1"
Expand Down Expand Up @@ -18182,7 +18182,7 @@ __metadata:
ra-i18n-polyglot: "npm:^5.1.2"
ra-language-english: "npm:^5.1.2"
ra-ui-materialui: "npm:^5.1.2"
react-hook-form: "npm:^7.52.0"
react-hook-form: "npm:^7.53.0"
react-router: "npm:^6.22.0"
react-router-dom: "npm:^6.22.0"
rimraf: "npm:^3.0.2"
Expand Down Expand Up @@ -18291,12 +18291,12 @@ __metadata:
languageName: node
linkType: hard

"react-hook-form@npm:^7.52.0":
version: 7.52.0
resolution: "react-hook-form@npm:7.52.0"
"react-hook-form@npm:^7.53.0":
version: 7.53.0
resolution: "react-hook-form@npm:7.53.0"
peerDependencies:
react: ^16.8.0 || ^17 || ^18 || ^19
checksum: 058bf5596f314c071863bb133979deb56d0a7817d5bf1908a569c003fe03a15736402b040d3e18aeb259723c6e15c243fe75d2d887ea47ff4be87fc472f31ad5
checksum: 6d62b150618a833c17d59e669b707661499e2bb516a8d340ca37699f99eb448bbba7b5b78324938c8948014e21efaa32e3510c2ba246fd5e97a96fca0cfa7c98
languageName: node
linkType: hard

Expand Down Expand Up @@ -19672,7 +19672,7 @@ __metadata:
react: "npm:^18.3.1"
react-admin: "npm:^5.1.2"
react-dom: "npm:^18.3.1"
react-hook-form: "npm:^7.52.0"
react-hook-form: "npm:^7.53.0"
react-router: "npm:^6.22.0"
react-router-dom: "npm:^6.22.0"
typescript: "npm:^5.1.3"
Expand Down
Loading