Skip to content
This repository was archived by the owner on Aug 7, 2020. It is now read-only.

Conversation

@radireddy
Copy link
Contributor

@radireddy radireddy commented Apr 2, 2019

Custome validation is stepper component

Use case: Execute costume validation on submitting a stepper and do not go to the next step if the validation fails.

The only way to achieve this is to use the valid method of the stepper component. I have added my custom validation in a valid method but this method gets executed many many times which is not very efficient and taking longer time. I want to execute (only once) complex validation on form submit and go to the next step only if the validation succeeds.



Current behavior
a valid method of oui-step-form gets fired many many times even though the stepper is not focused. For example, if a stepper has 5 steps and the last step has a valid method. This method gets executed as soon as the stepper is loaded and get executed many times, more than 100, based on the complexity of the stepper form.
In the above example, isNodesValid() method gets called many times even before reaching this step. Since I have a complex validation in isNodesValid() method, it's taking too long.

Expected behavior
valid method must be executed only on clicking the submit button for that step.

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Title of the Pull Requests

Conditional step navigation

Description of the Change

Go to the next step in stepper component only if on submit method return true or nothing. Stop step navigation if this method returns false.

Benefits

Possible Drawbacks

Applicable Issues

if (isNil(gotToNextStep) || gotToNextStep === true) {
// Focus next step
this.stepperCtrl.addForm(form, this.stepper.index);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could just do that I guess:

if (form.$valid && this.valid && this.onSubmit({ form })) {
    this.stepperCtrl.addForm(form, this.stepper.index);
}

Copy link
Contributor

@AxelPeter AxelPeter Apr 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide more informations about your use case (with code example) in the description of this PR ?

Because, in a case of an async check (eg. API), this will not work. The check could be done with the blur or change event of an input, before submiting the form for example.
Or in an additional step too.

That's why we have the valid and navigation attributes.
The step buttons could be shown when all the checks are green :)

A thing we could still fix here, is to call this.onSubmit({ form }); outside the condition, to allow access of the form value even when the form is not valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@euhmeuh, we can do that because in all the places where stepper is used, the onSubmit method does not return anything but still need to go to the next step. The condition to go to the next step is if nothing is returned or true is returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AxelPeter , i have added detail description with my usecase and code sample.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AxelPeter , I undo my changes and moved onSubmit call before if statement. This will be sufficient for my use case and looks very neat as well.

if (isNil(gotToNextStep) || gotToNextStep === true) {
// Focus next step
this.stepperCtrl.addForm(form, this.stepper.index);
}
Copy link
Contributor

@AxelPeter AxelPeter Apr 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide more informations about your use case (with code example) in the description of this PR ?

Because, in a case of an async check (eg. API), this will not work. The check could be done with the blur or change event of an input, before submiting the form for example.
Or in an additional step too.

That's why we have the valid and navigation attributes.
The step buttons could be shown when all the checks are green :)

A thing we could still fix here, is to call this.onSubmit({ form }); outside the condition, to allow access of the form value even when the form is not valid.

@AxelPeter
Copy link
Contributor

AxelPeter commented Apr 2, 2019

@radireddy Is something like this ticket could help you with your use case (UK-89) ?
It's similar to your proposition but in another event.

go to next step only if on submit method return true
@radireddy radireddy force-pushed the hotfix/stepper-next-step branch from df9196d to bc996b8 Compare April 3, 2019 10:28
@radireddy
Copy link
Contributor Author

@AxelPeter, As you suggested, I undo my changes and moved onSubmit call before if statement. This will be sufficient for my use case and looks very neat as well. Can you prod this now?

@AxelPeter
Copy link
Contributor

@radireddy This will be released with the next minor version (v2.27.0), when all the others PRs are resolved (ui-angular and ui-kit) :)

@AxelPeter AxelPeter merged commit 008583c into master Apr 3, 2019
@AxelPeter AxelPeter deleted the hotfix/stepper-next-step branch April 3, 2019 12:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Development

Successfully merging this pull request may close these issues.

4 participants