Skip to content
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

issues on previous-step #57

Open
HIRANO-Satoshi opened this issue Dec 19, 2018 · 12 comments
Open

issues on previous-step #57

HIRANO-Satoshi opened this issue Dec 19, 2018 · 12 comments

Comments

@HIRANO-Satoshi
Copy link

Buttons with previous-step shows the preloader and hang up.

As another issue, I think the data-feedback function is not called on previous-step.

Background:

With 2.1.4 I used data-feedback for validation and the stepchange event to catch transition to setup next validator.
The stepchange event was sent after the transition both for next steps and prev steps. So I was able to setup validators using getActiveStep().

With 3.0.0 beta the stepchange event is sent for next steps before transition. I think no such event is sent for prev steps. So I'm trying to use data-feedback and it calls nextStep() and destroyFeedback(). Maybe that differs from your intention.

<div class="step-actions">
  <button class="next-step" data-feedback="validateStep">Next</button>
  <button class="previous-step" data-feedback="prevStep">Prev</button>
</div>
init() {
    this.stepper = new MStepper(stepperElement, {
        autoFocusInput: false,
        autoFormCreation: false,
    });
    // stepperElement.addEventListener('stepchange', () => {this.stepChanged(); });

    window['validateStep'] = () => {this.validateStep();};
    window['prevStep'] = () => {this.prevStep();};
}

stepChanged() {
    var step = this.stepper.getSteps().active.index;
    this.validationController.addObject(this.entity, this.ValidateRules[step]);
}

validateStep() {
    this.validationController.validate().then((valid) => {
        if (valid) {
            this.stepper.nextStep(() => {this.stepChanged();});
            this.log.debug('ValidateStep: valid');
        } else {
            this.Stepper.destroyFeedback();
            this.log.debug('ValidateStep: invalid');
        }
    });
}

prevStep() {
    this.stepper.prevStep(() => {this.stepChanged();});
}

@Kinark
Copy link
Owner

Kinark commented Dec 19, 2018

About the issue

In the docs, it's like this:

Feedback function
There's a way to make the buttons run a function instead of proceeding, just add a data-feedback attribute with the function name to a ".next-step" classified button. Just like that:

So .previous-step buttons were not intended or supposed to run data-feedback functions. It only works with .next-step buttons.

About the events and validations

Yes, you're right, now the events are triggered independent from the transitions, but this shouldn't be an issue.

There are several ways to add both general validation (for all steps) and custom validations (for each one):

You can add a general validation function (that runs everytime the method nextStep() runs, including when you click .next-step buttons) by using the validationFunction option as described in here and in here:

var generalValidationFunction = function(stepperForm, activeStepContent) {
   // You can use the 'stepperForm' to valide the whole form around the stepper:
   someValidationPlugin(stepperForm);
   // Or you can do something with just the activeStepContent
   someValidationPlugin(activeStepContent);
   // Return true or false to proceed or show an error
   return true;
}

var stepper = document.querySelector('.stepper');
var stepperInstace = new MStepper(stepper, { validationFunction: generalValidationFunction })

Or, for custom steps validations, you can use the feedback functions themselves. The feedback functions are called with three parameters: destroyFeedback, form, activeStepContent, so you can do any kind of validation inside there and just call destroyFeedback(true) when you're done, as it goes in here or in here:

<button class="waves-effect waves-dark btn next-step" data-feedback="customValidation">CONTINUE</button>
<script>
   function customValidation(destroyFeedback, form, activeStepContent) {
      // Do your stuff here like:
      form.validate(function(validation) {
            // Call destroyFeedback() function when you're done
            // The true parameter will proceed to the next step besides destroying the preloader
            if(validation) return destroyFeedback(true);
            return destroyFeedback();
      })
   }
</script>

Just a tip

By taking a look at your code, I can see you're adding methods to the MStepper class in order to achieve somethings which I believe you could achieve by using the plugin's native ways.

Like, you're defining a method called validateStep and binding it to the window object in the init method. Why don't you just declare validateStep function in the global scope instead?

Also, I guess you're trying to create a workaround for the events not being triggered at the end of the transitions with some extra methods, but as I explained above, I believe it's not neccessary.

Well, I don't know your objectives, but maybe you really need to edit all those things, but maybe there's a better way. I'm just trying to help :)

Conclusion

I'll keep this open for now, since I feel you maybe have some questions yet.

Thanks for using MStepper :)

@HIRANO-Satoshi
Copy link
Author

Thanks for detailed explanation. I appreciate.

The validationFunction should return true or false. How do I use it with Promise, for responses from the server for example? That's the reason why I'm trying to use data-feedback. Do we need a promise version?

I like MStepper. Thanks much for your contribution to the world.

@Kinark
Copy link
Owner

Kinark commented Dec 19, 2018

Yeah, the promise inside validationFunction is a problem.

I'll be doing some tests and edit this answer asap, but in the next version I'll change it to something that works with promises. Also, I'm thinking about adding a default validation to the steps (if none is passed to the option).

--- EDIT ---

@HIRANO-Satoshi
I've recalled the reason for not to support promises in general validationFunction. It shouldn't take no time to validate. It was supposed to work like browser's native validation (or as fast as it), but continuing being custom.

If it waits for a promise, a loader should be displayed, and this is not the intention of the general validationFunction.

If you need to do async validations (like checking db), you should use feedback functions, yeah.

However, I believe you still doesn't need to add a method and bind it to the window object. Just declare it in the global scope and use the methods that are sent to it (destroyFeedback, form, activeStepContent).

@Kinark
Copy link
Owner

Kinark commented Dec 19, 2018

(I've updated the above answer)

@HIRANO-Satoshi
Copy link
Author

I see.

We need to wait for the result of validation anyway regardless the response is quick or slow before going to the next step. One of typical usages is to confirm the server whether email address is unique or not in the signup process.

Use of data-feedback for validation is not good as you pointed out. Some people might use both validationFunction and data-feedback according to sync or async and be confused.

Validation is validation. So would validationFunction(): boolean|Promise be a simple API and great?

@Kinark
Copy link
Owner

Kinark commented Dec 20, 2018

When I said "fast" or "slow", I meant sync or async. A sync task is usually faster (in a UX pov) than an async one, if it's not, it should be async instead. A simple validation (such as jQuery validation plugin or the browser's native one) is always sync and instantaneous. If validationFunction option would support promises, a lot of people would put async logic in there, so every step would have a preloader and this should be avoided both for UX and programming flow reasons.

In the versions 1.x.x and 2.x.x, there was an integration with jQuery validation plugin, but it was removed due to the removing of jQuery from the project's dependencies. However, there a lot of validation libs out there, and that's why I added validationFunction, for people to be able to make "offline" validations using whatever they want to (even jQuery). Usually these libs work synchronously, so for this one and for the above-mentioned reasons, I didn't add promises support for the validationFunction.

It's 100% ok to use validationFunction and data-feedback together for sync/async validations, respectively. The stepper would validate using the validationFunction and, if it passes, it'll run the data-feedback.

However, you're about people getting confused. I should've made explicit that the validationFunction is for sync tasks and the data-feedback is for async ones.

--- EDIT ---

Maybe a more clear and short explanation would be that the validationFunction stands for client-side sync form validation, and the data-feedback stands for everything else.

@HIRANO-Satoshi
Copy link
Author

HIRANO-Satoshi commented Dec 21, 2018

Thanks. I can understand you. Users need explanation and one ore more examples.

I as a user think validationFunction(): boolean|Promise is a cleaner API and much easier to use than your example.

Both sync and async validations are needed anyway. Users could do sync validations first for fast response and async validations later in it if they like to do so.

validationFunction(): boolean | Promise<boolean>{
    if (!this.email)
        return false;   // sync boolean

    return this.server.checkEmail(this.email);  // async promise
}

EDIT:

Or, it could be a Promise only API. Promise is not slow actually if you return Promise.resolve().
aurelia-validation is promise only actually.

validationFunction(): Promise<boolean>{
    if (!this.email)
        return Promise.resolve(false);   // sync boolean

    return this.server.checkEmail(this.email);  // async promise
}

@Kinark
Copy link
Owner

Kinark commented Dec 21, 2018

But async functions are usually used to contact the database. Using your example, why would users want to checkEmail() in every step?

validationFunction can't be changed once you initialize the stepper. That's the reason I a a promise support would be weird. It was supposed to be just a way to check if required inputs are filled and, idk, type="email" inputs were filled with actual emails. It was supposed to be a simple validation, since it's the same for every step.

@HIRANO-Satoshi
Copy link
Author

I see. validationFunction is a global one, while data-feedback is per step one.

Sorry, my promise example was too simplified. I supposed to change validators each step, as I showed at stepChanged() in my example above. I understood that data-feedback is a way to do so.

But I'm a bit confused. I don't make sense a global validation function could work for all steps, because inputs are blank in the future steps.

And I think each step should have a validation function. server.checkEmail() is needed at the step where a user types an email address in order to show that it is invalid at the step.
If you do async validations at a later step or at the last, how do you show errors? Open the email step by code? If some steps have errors? Force the user to check steps one by one?

Maybe what I need is an API that enables a step validator when step changes.
Maybe nextstep and prevstep event handlers?

Note: our app is a single page app (SPA) using Aurelia-validation

@Kinark
Copy link
Owner

Kinark commented Dec 21, 2018

So, the flow I had in mind when I refactored the MStepper was:

Simple global input validation:
A simple way to check if required inputs are empty or not. Could use some client-side validation lib such as jQuery Validation.
For this, we have:

var generalValidationFunction = function(stepperForm, activeStepContent) {
   // Simple client-side validation for the form to work like it was outside the stepper
   return activeStepContent.areTheRequiredInputsFilled()
}

var stepper = document.querySelector('.stepper');
var stepperInstace = new MStepper(stepper, { validationFunction: generalValidationFunction })

This function would run every time you hit a .next-step button, so you can validate the inserted information in the active step before proceeding.

That's why it doesn't support async, cause it should be a simple function that do a simple validation.

Like, let's suppose you're asking for a password in a step, the user's name in the other and the user's phone number in the other. All of them are required, so you have 3 steps with the same "kind" of validation, that is: "check if the required inputs are filled before proceeding to the next step". That's where validationFunction is useful.

If you make a simple form with no JS involved, and you try to submit the form with a required input empty, the browser's gonna stop you from submitting and show a balloon saying "This field is required". Since the stepper is fully controlled by JS, the validationFunction is just a way for you to imitate this browser's native function. Got it?

Validation per step, maybe async:
For custom, per-step and advanced validations, we have:

<ul class="stepper linear">
   <li class="step active">
      <div class="step-title waves-effect">E-mail</div>
      <div class="step-content">
         <input type="email" id="email_input" required>
         <div class="step-actions">
            <button class="waves-effect waves-dark btn next-step" data-feedback="checkEmailDB">CONTINUE</button>
         </div>
      </div>
   </li>
</ul>

<script>
   function checkEmailDB(destroyFeedback, form, activeStepContent) {
      // Get the email the user is trying to use in the active step while a preloader is spinning
      var email = activeStepContent.getElementById('email_input');
      // Use a function to call your DB
      checkInDbIfEmailExist(email, function(exist){
         // If it exists, return the destroyFeedback to destroy the preloader
         // The true parameter will also skip the step so like "ok, email doesn't exist, proceed"
         if(exist) return destroyFeedback(true);
         // Otherwise, destroy the preloader but doesn't proceed to the next one
         return destroyFeedback();
         // If you want, call wrongStep to show an error
         // Maybe I'll add this to destroyFeedback native working flow
         stepperInstace.wrongStep();
      });
   }
</script>

This way, the checkEmailDB function will be called when the user tries to proceed, so you can validate the email he inserted before actually proceeding. If the email exists, the an error is shown and the step doesn't change. If it doesn't, he can go to the next step.

If you need to validate another step, create another function and bind it to the .next-step button inside the other step you want to do a custom async validation.

---EDIT---

Oh, I got your doubt. validationFunction is a global function that is called per step. It's a common function to validate each step, not the entire stepper. Like a metal detector door, it's a common validation that runs the same way every time a person walks through.

The feedback-function is specific specific function that is called only on specific steps.

@Kinark
Copy link
Owner

Kinark commented Dec 21, 2018

Updated answer!

@HIRANO-Satoshi
Copy link
Author

I visited your HP. It's nice.

If you intended to validate each step with one validationFunction() , we would appreciate if you add some more explanation about activeStepContent in the doc.

We need to know what step we are doing to switch validators in the validationFunction().

Could you add the above checkEmailDB() example to the doc?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants