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

Automatically skip missing steps #65

Closed
wattsbn opened this issue May 26, 2015 · 7 comments
Closed

Automatically skip missing steps #65

wattsbn opened this issue May 26, 2015 · 7 comments

Comments

@wattsbn
Copy link

wattsbn commented May 26, 2015

It would be very convenient for me if shepherd would automatically skip a step if the element/selector specified was not found. Currently an error is thrown and the html is just appended to the body without using tether. At the very least I would expect a way for me to implement this myself through the event system, but that doesn't seem to work either. I tried cancelling a step in the 'before-show' event, but I got console errors from tether.

@geekjuice
Copy link
Contributor

I'm actually not comfortable just skipping a step if a selector is not found. However, I do think it would be cool if we could conditional determine whether to show a step or not. For example:

tour.addStep('Hamlet', {
  title: 'To be or not to be',
  text: 'that is the question,
  attachTo: '.dear-life',
  advanceOn: '.vengeance click',
  showOn() {
    return document.querySelector('.king-claudius') ? true : false;
  }
});

In this case, if the selector .king-claudius exists, we do show the step, otherwise, return false which would trigger Shepherd to skip this step. If you would like to open a PR, I'd be more than happy to help get this implemented as soon as possible.

@mtgibbs
Copy link
Contributor

mtgibbs commented Aug 24, 2015

@geekjuice

#78

Take a look at this implementation of the idea. We were doing something similar and I took your comments into consideration.

@geekjuice
Copy link
Contributor

@mtgibbs Awesome! I'll do my best to take a look as soon as I can (been fairly busy and behind on PR reviews 😓)

@mtgibbs
Copy link
Contributor

mtgibbs commented Sep 23, 2015

@geekjuice

Just a bump on this thread. The PR is still in need of review. I'm sure there's some work that needs to be done to it (versioning, alignment with your standards, etc). It's going on 30 days now.

@geekjuice
Copy link
Contributor

Thanks for the heads up and reminding me about this. I'll take a look a today and get back to hopefully by the end of the day or early tomorrow. Sorry again for the delay

@mtgibbs
Copy link
Contributor

mtgibbs commented Oct 7, 2015

Suggesting that this issue be closed now that the PR has been accepted.

@geekjuice
Copy link
Contributor

👍 Good call. Thanks!

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

No branches or pull requests

3 participants