Adding type="button" to SaveButton overrides custom onSubmit behaviour on TabbedForm

[andrico1234]

What you were expecting:
Adding type="button" to the SaveButton simple changes the button's behaviour to not fire submit on enter, as per the documentation

What happened instead:
Changing the type property seems to have additional side-effects. It doesn't seem to respect the a custom onSubmit function passed through to the form component. Instead it seems to revert to the default onSubmit behaviour

Steps to reproduce:
Access the repro
Navigate to edit post page
Change the text value
Press save

Observe that the default save behaviour is used, instead of our custom onSubmit

Related code:

• Preferably, a CodeSandbox forked from
  • https://codesandbox.io/s/github/marmelab/react-admin/tree/master/examples/simple (v4)
  • https://codesandbox.io/s/github/marmelab/react-admin/tree/3.x/examples/simple (v3)
• A link to a GitHub repo with the minimal codebase to reproduce the issue

insert short code snippets here

Other information:

Environment

• React-admin version:
• Last version that did not exhibit the issue (if applicable):
• React version:
• Browser:
• Stack trace (in case of a JS error):

[slax57]

Hi!
I don't think this is a bug.
The SaveButton falls back to calling the default save action, since you did not provide neither a mutationOptions.onSuccess nor a onClick callback (with e.preventDefault()).
If you provide either of them, it will not call the default onSubmit.

[christiaanwesterbeek]

@slax57 I have this problem too. Looking at the code of SaveButton, your remarks kind of make sense. To be sure what you mean I have setup a codesandbox that reproduce the problem.

In the sandbox. I added 2 SimpleForm's, each with its own Toolbar and SaveButton. The first one is a regular SaveButton the second has SaveButton with type="button". You can see how the submitForm function in the second one is not called. How do you propose exactly to fix that?

https://codesandbox.io/s/focused-ellis-xvjqkj?file=/src/customRouteNoLayout.tsx

I'm guessing, I should wrap the SimpleForm with a SaveContextProvider...

[christiaanwesterbeek]

Yep, got the solution.

import * as React from "react";
import {
  Toolbar,
  SaveButton,
  SimpleForm,
  TextInput,
  SaveContextProvider
} from "react-admin";

const MyToolbarWithSubmitSave = () => (
  <Toolbar>
    <SaveButton label="submit save" />
  </Toolbar>
);
const MyToolbarWithButtonSave = () => (
  <Toolbar>
    <SaveButton label="button save" type="button" />
  </Toolbar>
);

const CustomRouteNoLayout = ({ title = "SimpleForm" }) => {
  const submitForm = (formData) => {
    console.log("submitForm", formData);
  };

  return (
    <div>
      <h4>Simpleform with regular submit SaveButton (submitForm is called)</h4>
      <SimpleForm onSubmit={submitForm} toolbar={<MyToolbarWithSubmitSave />}>
        <TextInput source="title" />
      </SimpleForm>
      <h4>Simpleform with type=button SaveButton (submitForm is NOT called)</h4>
      <SimpleForm onSubmit={submitForm} toolbar={<MyToolbarWithButtonSave />}>
        <TextInput source="title" />
      </SimpleForm>
      <h4>
        Simpleform with type=button SaveButton wrapped in a SaveContextProvider
        (submitForm is called)
      </h4>
      <SaveContextProvider value={{ save: submitForm }}>
        <SimpleForm toolbar={<MyToolbarWithButtonSave />}>
          <TextInput source="title" />
        </SimpleForm>
      </SaveContextProvider>
    </div>
  );
};

[slax57]

@christiaanwesterbeek Actually the solution I had in mind was to call the submitForm callback from the SaveButton with onClick: <SaveButton label="button save" type="button" onClick={submitForm} /> ; and if needed fetch the form inputs values from the FormContext.
Overriding the SaveContextProvider is also a valid solution, although looking a bit hacky.

Now, after reading through this issue a 2nd time, I realize it could be nice to have the SaveButton also have a look at the context to see if a custom onSubmit was provided, in addition to looking at the SaveContext.

The original idea was that <SaveButton type="button" /> is meant for cases when you don't want to trigger the default Form submit action (this is why we expect a custom onClick or mutationOptions).
However, I realize it can also be used to simply disable the submit on enter functionality, and in this case it should submit the Form the same way as the <SaveButton type="submit" />.
Hence I'm reopening this and marking it as a bug.

[christiaanwesterbeek]

Yes, and the reason I wanted to add type="button, because I'm in te process of migrating a large project from RA3 to RA4 where a SimpleForm had a submitOnEnter={false} which isn't supported anymore.

By default, <SimpleForm> and <TabbedForm> submit when the user presses Enter. To disable this behavior, you must now turn the <SaveButton> (which renders as a <input type="submit" /> by default) into an <input type="button"> element, by setting the type prop to “button”.

[fzaninotto]

I suggest we fix this via documentation: in the section about submit on enter

https://marmelab.com/react-admin/EditTutorial.html#submit-on-enter

We should warn that this approach isn't compatible with a custom onSubmit, and that developers should pass an onClick to the SaveButton instead.