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

Prevent AutoForm validation with custom children to prevent being blocked unshown required fields #661

Merged

Conversation

MillanWangGadget
Copy link
Contributor

  • UPDATE
    • About 2 weeks ago, a change was shipped that prevented local field validations from running on fields that were not rendered as a result of include/exclude AutoForm properties
    • When AutoForm components have custom children all validations would still run, which would prevent form submission if one of the fields was required and not registered to react hook form in a child component.
    • Now, having any child components in AutoForm will now avoid all local validations so that the requests are always sent to the server to be validated there.
      • I don't think we can reasonably parse the child components to determine which validations should be ran

Comment on lines 158 to 161
const hasCustomChildren = !!props.children;
const fieldPathsToValidate = hasCustomChildren
? [] // With custom children, do not validate fields before sending them to avoid blocking submissions due to missing required fields
: fields.map(({ path }) => path);
Copy link
Contributor

@alanko0511 alanko0511 Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use useMemo to fieldPathsToValidate with a [hasCustomChildren] as the dep array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. Updated to add a useMemo to avoid remapping on fields

"@gadgetinc/react": patch
---

- Added an error message to AutoForm when passing in `include/exclude` options alongside child components
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're throwing a hard error, so maybe it should be "Throw an error" instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to say that a new error is thrown

@MillanWangGadget MillanWangGadget force-pushed the mill/preventAutoFormValidationsWithCustomChildren branch from 6a0b89a to d528d86 Compare September 30, 2024 22:01
@MillanWangGadget MillanWangGadget merged commit c847e08 into main Oct 1, 2024
9 checks passed
@MillanWangGadget MillanWangGadget deleted the mill/preventAutoFormValidationsWithCustomChildren branch October 1, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants