Suggestion - rework onNext

[IGreatlyDislikeJavascript]

Headline: there seems to be some (human) logical issues with the onNext callbacks etc. Also, fair warning: I greatly hate javascript and I'm not a js coder.

It's not possible to call tour methods in custom onNext callback because of the way Tour is structured. I've hacked around in the code for a bit and it seems like the approach to moving between steps is incompatible with the way a human would expect to use the feature.

This code:

promise = this._makePromise(step.onNext != null ? step.onNext(this) : void 0);
return this._callOnPromiseDone(promise, showNextStepHelper);

seems to look for the onNext function creating a promise, then tries to obey the promise and immediately calls showNextStepHelper() which in turn uses showStep() to always show the next step.

That means that any validation or logic around "can I move to the next step" in onNext is basically ignored. For example, if you try to do this:

						element: "#myelem",
						title: "Fill this out",
						content: "Fill out some stuff now",
						onNext:	function(tour)
								{
									if(isStuffFilledOut() === false)
										tour.goTo(some_other_step);
								}

It won't work, because although the tour WILL correctly call goTo, it's immediately superseded by the promise-y stuff in my first code excerpt above. That makes the goTo() step appear then immediately disappear because of _callOnPromiseDone(promise, showNextStepHelper).

Looking around, this is causing a few people some frustration and they're posting how "goTo doesn't work in onNext".

It's probably not an edge case that you'd want a tour to highlight an input element, ask the user to fill it out, and only continue if they do actually fill it out - or some similar scenario where the tour wants the user take some action before continuing to next step.

Because onNext is called when the "next" tour button is clicked, human logic says "I should do my validation checks there". But even if we do the checks in onHide the same problem exists. So it feels like onNext is the right place to check if the tour should continue, except the code behind the "next" button has already made the decision to move to the next step regardless of what happens in onNext callback. This results in any call to any method that changes the step (goTo etc) having no effect (called, then overridden).

My suggestion:

1. either recode the flow so that onNext, onPrev etc do not make the absolute decision to go to the next/prev step, obeying any call to goTo etc in the onNext() callback
2. Add logic so that if the callback returns FALSE, the tour does not move to a new step. This allows us to write an onNext() function that checks if conditions are met (is div visible, was button clicked, is input filled out etc), and if not, return FALSE to prevent automated move to the next step. It would also allow a jump based on new conditions, e.g.:

onNext: function(tour){
if(someinput.val() == "banana)
    tour.goto(66); // the banana step
else
   tour.goto(99); // no banana

return false;
}

I've hacked a solution into Tour for this for now, but would be good to get it added by someone who can actually code in that hateful, hateful javascript. This hack is horrible, because it allows Tour to hide the current step before reshowing it again. But it works. Kind of.

If anyone wants to duplicate this, change the _showNextStep() on line 471 to the following:

		Tour.prototype._showNextStep = function () {
			var promise,
			showNextStepHelper,
			step;
			step = this.getStep(this._current);
			
			showNextStepHelper = (function (_this) {
				return function (e) {
					return _this.showStep(step.next);
				};
			})(this);
			
			promise = void 0;

			if (step.onNext != null)
			{
				rslt = step.onNext(this);
				
				if(rslt === false)
				{
					return this.showStep(this._current);
				}
				
				promise = this._makePromise(rslt);
			}

			return this._callOnPromiseDone(promise, showNextStepHelper);
		};

Now you can return false from your onNext handler, and Tour won't ignore you:

onNext: function(tour){
    if($('#inputBanana').val() !== "banana")
    {
        // no banana? highlight the banana field
        $('#inputBanana').css("background-color", "red");
        // do not jump to the next tour step!
        return false;
    }
}

You can follow the same model for onPrevious.

[IGreatlyDislikeJavascript]

Added a pull request for these features and some others that were useful/seem to be requested

• return false from onNext / onPrev function to (a) prevent tour from auto-progressing and (b) allow tour flow code tour.goTo etc to work correctly

• use new option reflexOnly: true to hide the "Next" button and force user to click on reflex element to continue tour

• use a function to dynamically determine element during tour - element: function() { return your_element; },

#710

Oh and for the love of all that matters, why do JavaScript coders on github refuse to add comments to their source? Maybe I'm just old and coding for too many decades, maybe I'm inept, but without comments I can barely remember what my own code does 3 minutes after writing it. How anyone is meant to intelligently contribute to OSS code without spending ages scratching explanations of obscurely vars and functions onto sticky notes while keeping one eye on static source, one eye on watch and call stack, and the third spare eye on the notepad++ search window whilst maniacally typing obfuscated names like $_thiscbinnerimportantoverridden. Yes, 3 eyes is a requirement for github forks. Seriously. We want to help contribute in some tiny way. Throw us a bone, add a comment, tell us why that apparently critical anonymous function only gets called if it's raining outside.

[IGreatlyDislikeJavascript]

Added pull request for:

onElementUnavailable - called when step element is missing, hidden etc

fixed flickering scroll/continual reload of popover

removed need to call init(), fixed logic that forces tour to start on page reload, which conflicts with hidden elements in tour

added progress bar and progress text options

[IGreatlyDislikeJavascript]

Closing this, opening a new thread with all the fixes