-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat(webforms): RHF Validator #184
Conversation
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.
This looks solid to me, id be great if you got a thumbs up from @daniel-belcher as well so he can add any thoughts.
} | ||
}; | ||
|
||
if (SLOT.rhf === "Input") { |
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.
SLOTs can only be one of the rhf types, any reason this can't be a switch, or at least an if/else tree to lower amount of checks?
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 try to avoid switches, because it's always felt too much "tool" for the job. In addition, not every developer is familiar with the rules of using a "switch". Even I occasionally forget to break after every case statement.
As for if/else
, it's mostly a personal preference. For me, I lean towards conditional logic that are simpler with less mental overhead. So, the closer the logic can be expressed in a flat list structure, the easier it'll be to read.
@@ -17,12 +17,12 @@ export const FieldGroup = <TFields extends FieldValues>( | |||
}); | |||
|
|||
const onAppend = () => { | |||
fieldArr.append(props.fields.reduce(slotReducer, {}) as any); | |||
fieldArr.append(props.fields.reduce(slotInitializer, {}) as any); |
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.
New initializers are great!
🎉 This PR is included in version 1.5.0-val.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Purpose
Linked Issues to Close
https://qmacbis.atlassian.net/browse/OY2-26175
Approach
Assorted Notes/Considerations/Learning
List any other information that you think is important... a post-merge activity, someone to notify, what you learned, etc.