marmelab / react-admin

A frontend Framework for single-page applications on top of REST/GraphQL APIs, using TypeScript, React and Material Design
http://marmelab.com/react-admin
MIT License
24.89k stars 5.23k forks source link

Save button is disabled on form when using the clone button #9217

Open tdesmet opened 1 year ago

tdesmet commented 1 year ago

What you were expecting: In version 3 of react admin when you clicked on a clone button and the create form is shown the save button was enabled.

What happened instead: From version 4 the save button is by default disabled. So, the user must change a value on the form before being able to save

fzaninotto commented 1 year ago

Reproduced in the simple example (https://stackblitz.com/github/marmelab/react-admin/tree/master/examples/simple), thanks.

Now the problem is tha I have no idea how to fix this one...

slax57 commented 1 year ago

If my understanding is correct, the root issue is that, when there is a record passed as source query param, we use it as record to populate the form's default values. Instead, we should use it to populate the form's values, by calling setValue or similar.

It would be a significant change though. Besides, I don't know if we use the source query param for anything other than cloning. If we do, then this might not be possible at all. @fzaninotto do you know?

fzaninotto commented 1 year ago

Switching from default value to a change set is a semantic change. However, I see no other consequence than having the SaveButton always enabled when one passes a source query param - which seems OK.

So I tend to agree with your solution @slax57. And no, we don't use the source parameter (or the record route state) for anything else.

lvernaillen commented 1 year ago

An option is to do something similar as you do with useScrollToTop hook. You could change the CloneButton implementation to not only pass the location state _scrollToTop but also pass a location state called _enableSaveButton.

Then in your SaveButton component check for that location state and enable the button if that state is set.

It is a current workaround we do now, but in my opinion it's a bug that should be fixed in react-admin. I should be able to clone records without needing to change values or needing a workaround

fzaninotto commented 1 year ago

Let's challenge the fact that this is really a bug. If which case would you want to save a cloned record without modifying it?

lvernaillen commented 1 year ago

For example in case you want to provide the user with an easy way to "clone" a record with everything already prefilled.

There might be some changes needed, but those changed values are available from context when you click clone. So they can be applied automatically without the user needing to enter those new values. This is example 1 below.

In that case we provide a custom button and do the necessary changes automatically as described in the prefilling the form paragraph. So the new state, with changed values, is passed to the create screen. However, that sends the user to a create form where everything is already filled in but the save button is disabled... So although changes were applied, the create form does not recognize this as change. I suppose because of assigning them as default values as @slax57 mentioned above.

You also might want to clone something, prefill with exactly the same values and give the user the option to change something or not to change anything. This is example 2 below.

Example 1:

We have a pricelist with prices for products in a country. Imagine a t-shirt of 13 euro in Germany and 15 euro in Spain. I am a price manager for France and want to introduce that product in France as well. I need to set a price for France and decide to use the same price as in Germany. I don't want to bother going to the create screen and choosing that product, setting the same price, etc... I want to be able to click a button which prefills all that data. That's easy to accomplish in react-admin Create a custom button that will:

All I need to do now as a price manager in France is accept that creation form. All data already correctly filled in. That's nice! However, I cannot... the save button is disabled...

Example 2:

Imagine I create a react-admin app to make lunch orders. When I login I can see a list of all orders I made. I want to select an order and click "Order this again". This opens a create window with all the details prefilled as they were in my previous order. The user can, but is not obliged to, make some changes. However currently he must make changes otherwise the save button is disabled.

fzaninotto commented 1 year ago

All valid use cases, thanks!

So we need to move forwards with the fix suggested by @slax57. @tdesmet would you like to open a PR for that?

ErSuyog16 commented 1 year ago

I can resolve the bug can you please assign to me?

fzaninotto commented 1 year ago

Done!

ErSuyog16 commented 1 year ago

Can you please clear my doubt, As far now I understand the issue is like whenever I click to clone button I must have to do some changes such that it is possible to save the changes . basically for fixing this bug I have to put logic such that I can able to save data after cloning even I wouldn't change it right?

image

fzaninotto commented 1 year ago

@ErSuyog16 Right. But the solution is simple (see discussion above). If you're not comfortable with it, we can assign it to someone else.

djhi commented 1 year ago

A simple workaround until we fix this issue is to set the alwaysEnable prop on the SaveButton. This should be the default by the way (https://twitter.com/housecor/status/1701582293549174989, https://axesslab.com/disabled-buttons-suck/).

The issue with @slax57 solution is that it's currently the useCreateController responsibility to read the location to get the initial values. The form components know nothing of it and the useCreateController does not know about the form: we don't have access to the form context.

mikhail-fedosenko commented 1 year ago

If fixing this, please note, that we can prefill the form with react-router's state: { record: {...} } property... It should be checked too, in addition to record param in search query.

By the way, the current documentation states that CloneButton works using react-router's state property, while this is not true:

By default, the view starts with an empty record. However, if the location object (injected by react-router-dom) contains a record in its state, the view uses that record instead of the empty object. That’s how the works under the hood.

https://marmelab.com/react-admin/Create.html#prefilling-the-form

afilp commented 7 months ago

I came also here for that problem.

I clone a record, I do not want to change anything but just press the SAVE. I cannot. I would need to use toolbar={<SaveButton alwaysEnable />} I assume, but this requires to replicate the Toolbar, with code, styling etc. just for adding the alwaysEnable prop.

Couldn't this just be a prop at the <SimpleForm/> level?

For example: <SimpleForm saveButtonAlwaysEnable ... />

Thank you.