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

Event complete isn't called in case of last steps skipped #1353

Closed
thomasguittonneau opened this issue Mar 3, 2021 · 4 comments · Fixed by #1479
Closed

Event complete isn't called in case of last steps skipped #1353

thomasguittonneau opened this issue Mar 3, 2021 · 4 comments · Fixed by #1479

Comments

@thomasguittonneau
Copy link
Contributor

Hi,

I'm working with shepherd but in my case sometimes last elements are skipped.
in this case, the completed event is never called.

Maybe we can modify function _skipStep to check if the skippedStep is the last step.
(Like next function)

@RobbieTheWagner
Copy link
Member

@thomasguittonneau I don't understand the issue here. complete is called whenever you complete the tour, so if you want to have less steps or skip some steps, just make sure that when the tour is "done" you call tour.complete. If you're wanting to run some logic on both complete and cancel, you could listen to both events.

@thomasguittonneau
Copy link
Contributor Author

thomasguittonneau commented Mar 3, 2021

For my case, i have 5 steps and the last step have showOn option who return false.

In a case who user use next() function it's calling show but if shouldSkipStep = true it's calling _skipStep function

But if this skipped step is the last, _skipStep call show function on inexsitant step. It's closing the modal
without complete()

Maybe this can fix this problem ?

  _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);
    }
  }

@RobbieTheWagner
Copy link
Member

@thomasguittonneau definitely seems like an edge case we are not accounting for. We will think about a fix.

@stale
Copy link

stale bot commented Jun 9, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

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

Successfully merging a pull request may close this issue.

2 participants