-
Notifications
You must be signed in to change notification settings - Fork 107
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: Implement phone and email validations #414
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.
There is a failing test case that prevents us from merging: https://github.com/bpmn-io/form-js/actions/runs/3486984421/jobs/5834087642#step:6:486
If this is fixed, we can merge it 👍
e0ebacc
to
8c9187a
Compare
@pinussilvestrus I think this is ready for review again |
|
||
export default function ValidationGroup(field, editField) { | ||
const { type } = field; | ||
const validate = get(field, [ 'validate' ], {}); | ||
const isCustomValidation = [ undefined, VALIDATION_TYPE_OPTIONS.custom.value ].includes(validate?.validationType); |
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.
Just to raise alternatives: instead of changing the schema by adding validationType
we could detect the type by the pattern. But I guess this is too much magic, so having the validationType
is the most explicit way 👍
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.
✅ Let's get this out of the door and gather feedback.
Closes https://github.com/camunda/team-hto/issues/52, https://github.com/camunda/team-hto/issues/53
Let's keep the issues open, as it includes some other things we should follow up later (update docs, adding lint rules)
Closes camunda/team-hto#52, camunda/team-hto#53