showOn not functioning as expected

[mikeCGrant]

In the documentation the showOn property description is as follows:
A function that, when it returns true, will show the step. If it returns false, the step will be skipped.

What I'm seeing:
Setting the return of showOn to false in the first step prevents rendering of any following step/s.
This can be seen from the following code:

var tour = new Shepherd.Tour({
    useModalOverlay: true,
    steps: [
        {
            showOn: function() {
                return false;
            },
            title: 'Title',
            buttons: [
                {
                    text: 'Next',
                    action() { return this.next(); }
                }
            ]
        },
        {
            title: 'Title 2',
            buttons: [
                {
                    text: 'Next',
                    action() { return this.next(); }
                }
            ]
        }
    ]
});
tour.start();

[RobbieTheWagner]

@mikeCGrant I wonder if this is an edge case or an off by one issue. If the next step is the last step, and it is skipped, we call tour.complete() https://github.com/shipshapecode/shepherd/blob/master/src/js/tour.js#L356

[mikeCGrant]

@mikeCGrant I wonder if this is an edge case or an off by one issue. If the next step is the last step, and it is skipped, we call tour.complete() https://github.com/shipshapecode/shepherd/blob/master/src/js/tour.js#L356

I tested it with 3 steps too, setting the first to return false in the showOn method and still the subsequent steps didn't render.

I also set step 2 to false to make sure it wasn't a first step only thing, again subsequent steps didn't render.

[liam-jones-lucout]

I have this issue. It seems to be caused by this logic:

_skipStep(step, forward) {
    const index = this.steps.indexOf(step);
    const nextIndex = forward ? index + 1 : index - 1;

    if (nextIndex === this.steps.length - 1) {
      this.complete();
    } else {
      this.show(nextIndex, forward);
    }
  }

it seems like it should be comparing step.length the index, not nextIndex

I have created a pull request that I believe resolves it: #1931

[RobbieTheWagner]

Merged the PR and released 9.1.1, please let me know if the issue is fixed!

[liam-jones-lucout]

Thanks for jumping on this so quickly! Will try it out shortly and let you know