Fix Save button might stay disabled when using <ArrayInput> with default values

[henryhobhouse]

Problem

Currently the Array Input tries to update individual child values using react-hook-forms resetField function from their useFormContext hook. The issue is this bypasses react-hook-forms own control functions for updating array fields that we use to instantiate the fields initially. Although this appears to work fine the issue is that it seems to stop the form control checking if the fields are updated and as such does not make the form touched or dirty on update and subsequently the submit button remains disabled.

Example (bottom input is nested in array input):

Solution

Update the Array Input to use the return control APIs from useFieldArray (https://www.react-hook-form.com/api/usefieldarray/) to replace the field values and then reset the form with the same values to remove context that the fields have been updated.

In simple terms it looks like:

// BEFORE
resetField(source, { defaultValue });

// AFTER
fieldArrayInputControl.replace(defaultValue);
reset({}, { keepValues: true });

[slax57]

I'm really impressed by how deep you managed to dive into react-admin to come up with this fix, well done and thank you so much.

You may want to remove the manual call to reset() in your test and replace it with a RecordContextProvider as I suggested, but other than that, I really could find no issues with this change (although I tried hard, since this is a sensitive part of the framework 😁 )

So yeah, thanks, and well done! 👏

@fzaninotto if you have time for a second review I would appreciate it, since we can't afford any regressions on such a key component. As I said I could not find any, but another pair of eyes would be welcome to be extra cautious 🙂

[slax57]

Thanks for taking my review into account 🙂

Actually, after more thinking, we figured it might be best to actually keep the array logic in useApplyInputDefaultValues.ts instead of moving it in a dedicated hook, not because we are not happy with the separation of logic (it's the opposite actually), but because it might create regressions in custom input components.

Indeed the useApplyInputDefaultValues is used by useInput, so other custom inputs may rely on it to also support array inputs.

Hence, could you please move the logic back inside useApplyInputDefaultValues.ts?
Thank you so much.

[henryhobhouse]

I've updated. I have moved to use an object for the props with a union type to make sure the consumer is aware what props are needed to be passed dependant on type of input. Hopefully allows for some form of safe scalability if their is constriction to use a single entry point due to potential outlier regressions.

The key questions:

• Assume check for index and if parent prefixing that in path should happen for all to your point on regressions?
• Are you happy to declarative on both stating it's for an array and seperately requiring the array input controls. I felt it would be a more readable solution than assume if has array input control it therefore must be for an array input?

(NOTE: if you see this soon after this posting there is a problem in github with PR's at the moment so you won't see changes here, you will instead need to check the underlying branch. Hopefully they will solve soon.)

[slax57]

Thanks! 🙂

Assume check for index and if parent prefixing that in path should happen for all to your point on regressions?

It was the case before, so we should keep it that way

Are you happy to declarative on both stating it's for an array and seperately requiring the array input controls. I felt it would be a more readable solution than assume if has array input control it therefore must be for an array input?

Your solution looks good to me 🙂

Again, I'll request one more approval from @fzaninotto and we should be good to go.

[slax57]

Indeed this description is much more accurate, nice catch!

[slax57]

Alright we have @fzaninotto 's blessing, let's merge! 😁

Thanks again @henryhobhouse for your contribution 🙂