Fix <SaveButton> form dirty status check

Author: djhi

packages/ra-ui-materialui/src/button/SaveButton.spec.tsx

@@ -23,6 +23,7 @@ import { AdminContext } from '../AdminContext';
 import {
     AlwaysEnable,
     Basic,
+    ComplexForm,
     EnabledWhenFormIsPrefilled,
 } from './SaveButton.stories';
 
@@ -401,7 +402,7 @@ describe('<SaveButton />', () => {
         );
     });
 
-    it('should render enabled if the form is prefilled', async () => {
+    it('should be enabled if the form is prefilled', async () => {
         render(<EnabledWhenFormIsPrefilled />);
         await waitFor(() =>
             expect(screen.getByLabelText('ra.action.save')['disabled']).toEqual(
@@ -410,6 +411,73 @@ describe('<SaveButton />', () => {
         );
     });
 
+    it('should enable/disable consistently in a complex form', async () => {
+        render(<ComplexForm />);
+        await waitFor(() =>
+            expect(screen.getByLabelText('ra.action.save')['disabled']).toEqual(
+                true
+            )
+        );
+        fireEvent.change(await screen.getByDisplayValue('Lorem ipsum'), {
+            target: { value: 'Lorem ipsum dolor' },
+        });
+        await waitFor(() =>
+            expect(screen.getByLabelText('ra.action.save')['disabled']).toEqual(
+                false
+            )
+        );
+        fireEvent.click(screen.getByText('ra.action.save'));
+        await waitFor(() =>
+            expect(screen.getByLabelText('ra.action.save')['disabled']).toEqual(
+                true
+            )
+        );
+        fireEvent.change(await screen.getByDisplayValue('bazinga'), {
+            target: { value: 'bazingaaaa' },
+        });
+
+        await waitFor(() =>
+            expect(screen.getByLabelText('ra.action.save')['disabled']).toEqual(
+                false
+            )
+        );
+        fireEvent.click(screen.getByText('ra.action.save'));
+        await waitFor(() =>
+            expect(screen.getByLabelText('ra.action.save')['disabled']).toEqual(
+                true
+            )
+        );
+        fireEvent.click(screen.getByLabelText('ra.action.add'));
+        await waitFor(() =>
+            expect(screen.getByLabelText('ra.action.save')['disabled']).toEqual(
+                false
+            )
+        );
+        await waitFor(() =>
+            expect(
+                screen.queryAllByLabelText('resources.posts.fields.tags.name')
+                    .length
+            ).toEqual(2)
+        );
+        fireEvent.change(
+            screen.getAllByLabelText('resources.posts.fields.tags.name')[1],
+            {
+                target: { value: 'plop' },
+            }
+        );
+        await waitFor(() =>
+            expect(screen.getByLabelText('ra.action.save')['disabled']).toEqual(
+                false
+            )
+        );
+        fireEvent.click(screen.getByText('ra.action.save'));
+        await waitFor(() =>
+            expect(screen.getByLabelText('ra.action.save')['disabled']).toEqual(
+                true
+            )
+        );
+    });
+
     it('should not be enabled if no inputs have changed', async () => {
         render(
             <AdminContext dataProvider={testDataProvider()}>

packages/ra-ui-materialui/src/button/SaveButton.stories.tsx

@@ -1,10 +1,15 @@
 import * as React from 'react';
-import { Form, TestMemoryRouter } from 'ra-core';
+import { Form, ResourceContextProvider, TestMemoryRouter } from 'ra-core';
 import { Paper } from '@mui/material';
+import fakeRestDataProvider from 'ra-data-fakerest';
+import { useFormContext } from 'react-hook-form';
 
 import { SaveButton } from './SaveButton';
+import { ArrayInput } from '../input/ArrayInput/ArrayInput';
+import { TextInput } from '../input/TextInput';
+import { SimpleFormIterator } from '../input/ArrayInput/SimpleFormIterator';
 import { AdminContext } from '../AdminContext';
-import { useFormContext } from 'react-hook-form';
+import { Edit } from '../detail';
 
 export default {
     title: 'ra-ui-materialui/button/SaveButton',
@@ -81,3 +86,45 @@ export const Submitting = () => (
         </AdminContext>
     </TestMemoryRouter>
 );
+
+export const ComplexForm = () => (
+    <AdminContext
+        dataProvider={fakeRestDataProvider(
+            {
+                posts: [
+                    {
+                        id: 1,
+                        title: 'Lorem ipsum',
+                        tags: [{ name: 'bazinga' }],
+                    },
+                ],
+            },
+            process.env.NODE_ENV !== 'test',
+            300
+        )}
+    >
+        <Paper>
+            <ResourceContextProvider value="posts">
+                <Edit id={1} redirect={false}>
+                    <Form>
+                        <TextInput source="title" />
+                        <ArrayInput source="tags">
+                            <SimpleFormIterator>
+                                <TextInput source="name" />
+                            </SimpleFormIterator>
+                        </ArrayInput>
+                        <SaveButton />
+                        <FormInspector />
+                    </Form>
+                </Edit>
+            </ResourceContextProvider>
+        </Paper>
+    </AdminContext>
+);
+
+const FormInspector = () => {
+    const {
+        formState: { isDirty, dirtyFields },
+    } = useFormContext();
+    return <p>{JSON.stringify({ isDirty, dirtyFields })}</p>;
+};

packages/ra-ui-materialui/src/button/SaveButton.tsx

@@ -20,6 +20,7 @@ import {
     setSubmissionErrors,
     useRecordFromLocation,
 } from 'ra-core';
+import isEmpty from 'lodash/isEmpty';
 
 /**
  * Submit button for resource forms (Edit and Create).
@@ -74,7 +75,7 @@ export const SaveButton = <RecordType extends RaRecord = any>(
     const saveContext = useSaveContext();
     const { dirtyFields, isValidating, isSubmitting } = useFormState();
     // useFormState().isDirty might differ from useFormState().dirtyFields (https://github.com/react-hook-form/react-hook-form/issues/4740)
-    const isDirty = Object.keys(dirtyFields).length > 0;
+    const isDirty = hasDirtyFields(dirtyFields);
     // Use form isDirty, isValidating and form context saving to enable or disable the save button
     // if alwaysEnable is undefined and the form wasn't prefilled
     const recordFromLocation = useRecordFromLocation();
@@ -226,3 +227,44 @@ declare module '@mui/material/styles' {
         };
     }
 }
+
+const hasDirtyFields = (
+    dirtyFields: Partial<
+        Readonly<{
+            [x: string]: any;
+        }>
+    >
+): boolean => {
+    // dirtyFields can contains simple keys with boolean values, nested objects or arrays
+    // We must ignore values that are false
+    return Object.values(dirtyFields).some(value => {
+        if (typeof value === 'boolean') {
+            return value;
+        } else if (Array.isArray(value)) {
+            // Some arrays contain only booleans (scalar arrays), some arrays contain objects (object arrays)
+            for (const item of value) {
+                if (item === true) {
+                    return true;
+                }
+                // FIXME: because we currently don't set default values correctly for arrays,
+                // new items are either empty objects, or undefined in dirtyFields. Consider them as dirty.
+                if (
+                    (typeof item === 'object' && isEmpty(item)) ||
+                    item === undefined
+                ) {
+                    return true;
+                }
+                if (
+                    typeof item === 'object' &&
+                    item !== null &&
+                    hasDirtyFields(item)
+                ) {
+                    return true;
+                }
+            }
+        } else if (typeof value === 'object' && value !== null) {
+            return hasDirtyFields(value);
+        }
+        return false;
+    });
+};

yarn.lock

@@ -19904,11 +19904,11 @@ __metadata:
   linkType: hard
 
 "react-hook-form@npm:^7.53.0":
-  version: 7.53.0
-  resolution: "react-hook-form@npm:7.53.0"
+  version: 7.53.2
+  resolution: "react-hook-form@npm:7.53.2"
   peerDependencies:
     react: ^16.8.0 || ^17 || ^18 || ^19
-  checksum: 6d62b150618a833c17d59e669b707661499e2bb516a8d340ca37699f99eb448bbba7b5b78324938c8948014e21efaa32e3510c2ba246fd5e97a96fca0cfa7c98
+  checksum: 18336d8e8798a70dcd0af703a0becca2d5dbf82a7b7a3ca334ae0e1f26410490bc3ef2ea51adcf790bb1e7006ed7a763fd00d664e398f71225b23529a7ccf0bf
   languageName: node
   linkType: hard