-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
fix: add domain parser hook and domain condition #5413
Conversation
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for novu-design ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
@jainpawan21 I approve this so that we can ship but PTAL at my comments. I think it's worth using the validate function.
@@ -161,12 +174,6 @@ export function QuestionnaireForm() { | |||
<Controller | |||
name="domain" | |||
control={control} | |||
rules={{ |
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.
Alternatively, we can add it as a custom rule and let it trigger as part of react-hook-form lifecycle. For example:
rules = {{
validate: {
isValidDomain: v => parse(v)...
}
}}```
For more information PTAL https://www.react-hook-form.com/api/useform/register/#options:~:text=%7D%0A/%3E-,validate,-Function%20%7C%20Object
I am not sure what other benefits this gives us but If I had to guess it would make it simpler to trigger all validations at once on form click or maybe on blur if we want to.
What changed? Why was the change needed?
Add
useDomainParser
Hookhandle invalid domain field value
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer