-
Notifications
You must be signed in to change notification settings - Fork 143
Support validate intent to validate a full fieldset (all fields with a given prefix) #830
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
Conversation
More templates
@conform-to/dom
@conform-to/react
@conform-to/validitystate
@conform-to/yup
@conform-to/zod
commit: |
|
Hi @oxc, thanks for the PR! I can see this solution works for many cases. But the errors object might not have every fields and so there will be some minor inconsistency depending on the validation logic. Ideally, I think we need a different data structure so we can express is as every fields under But as a short term solution, how about making the validate intent accept an array of string? We can combine it with a simple utility to look for all the fields under it to achieve the same functionality, like: form.validate({
// This can be a string or an array of string
name: ['address', 'address.city', ...],
});
form.validate({
// Use a helper to look up fields from the DOM
name: getFields(formElement, name),
});
// Example `getFields()` implementation
function getFields(formElement: HTMLFormElement, name: string) {
const fields: string[] = [];
for (const element of formElement.elements) {
if (isInputElement(element) && element.name.startsWith(name)) {
fields.push(element.name);
}
}
return fields;
} |
|
Thanks, I will look into this an prepare a new PR. |
|
Hi @edmundhung, I looked into your suggestion. It will probably work for all my own use cases, but not all conceivable ones. More specifically, it will not return all errors that will be returned for this fieldset when validate is called without name. Especially, it will not report errors on missing values for which there was no field in the DOM, for example. Consider a nested array with its own insert intent button. Nevertheless, I think it's useful to have this feature to validate multiple specific fields at the same time, so I will prepare a PR for it. |
|
Would you think different about this PR if I renamed the new property from EDIT: Created #879 |
I am not sure if I follow. I think that's what the array of names solves as I expect people to provide all the field names instead. Do you mean the expectation of validating without a name specified doesn't match?
This is interesting as it is intentional not to have the newly insert item being validated. Won't it be awkward if you insert a new field and we show the error of that right away? I feel like you have a specific use cases in mind which I might have missed. Can you elaborate more? |
Yes, exactly. When I validate a field set (like the Todo in the example), I expect it to show exactly the same errors it will show when I submit the full form. That's currently not possible.
I might have described that badly. What I meant is: if you have a nested array with insert button, you only have field elements for the single entries, not for the array field itself. Consider having such an array inside a field set, e.g. a hypothetical "labels" array for each task in the Todo list example. If you have constraints on the array field ("Add at least one tag"), they won't be shown if you validate the surrounding field set. They will, however, show once the full form is submitted. This is the discrepancy I would like to solve. |
|
I have create a StackBlitz based on the multi-validate PR (#878) that demonstrates the problem, based on your suggestion to collect the field names from the Form element: https://stackblitz.com/edit/uzog2zz5?file=src%2Ftodos.tsx
Footnotes
|
|
As commented in #878 (comment), you will be able to validate nested fields in v2. Let me know if you have any questions 🙌🏼 |
Proposed fix for #828
Here is a sandbox with the same example from above Issue, where this patch is applied, and a second button is added to the task fieldset that demonstrates the behaviour:
https://codesandbox.io/p/devbox/sad-tree-fdj7hj?workspaceId=ws_3qoruoNJ3FvmBgCfSAACYX