-
-
Notifications
You must be signed in to change notification settings - Fork 555
feat(form-core): add onDynamicListenTo validator option #1896
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
base: main
Are you sure you want to change the base?
Changes from all commits
9b96af3
27a14c0
cdce7dd
0c7b91f
e734907
5f7b420
89adf3a
5dbb044
b5afe4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,7 +26,7 @@ export interface ValidationLogicProps { | |
| | undefined | ||
| | null | ||
| event: { | ||
| type: 'blur' | 'change' | 'submit' | 'mount' | 'server' | ||
| type: 'blur' | 'change' | 'submit' | 'mount' | 'server' | 'dynamic' | ||
| fieldName?: string | ||
| async: boolean | ||
| } | ||
|
|
@@ -147,6 +147,11 @@ export const defaultValidationLogic: ValidationLogicFn = (props) => { | |
| cause: 'submit', | ||
| } as const | ||
|
|
||
| const onDynamicValidator = { | ||
| fn: isAsync ? props.validators.onDynamicAsync : props.validators.onDynamic, | ||
| cause: 'dynamic', | ||
| } as const | ||
|
|
||
| // Allows us to clear onServer errors | ||
| const onServerValidator = isAsync | ||
| ? undefined | ||
|
|
@@ -193,6 +198,13 @@ export const defaultValidationLogic: ValidationLogicFn = (props) => { | |
| form: props.form, | ||
| }) | ||
| } | ||
| case 'dynamic': { | ||
| // Run dynamic, server validation | ||
| return props.runValidation({ | ||
| validators: [onDynamicValidator, onServerValidator], | ||
| form: props.form, | ||
| }) | ||
| } | ||
|
Comment on lines
+201
to
+207
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assumed the point of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there's an architectural issue here too.
@LeCarbonator Should I'm wait for your review on the best approach. |
||
| default: { | ||
| throw new Error(`Unknown validation event type: ${props.event.type}`) | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if adding this to the simple example makes sense, since
Having linked fields as examples is a good idea though. Maybe it could be its own example? Perhaps it can be added to
large-forminstead. I'll leave it up to you.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it makes more sense to include it in
large-formrather than as a separate example.Linked field validation is a pattern commonly used in real-world scenarios, so it fits naturally within that example.
I'll move it to
large-form. Is that okay with you?