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

Added showOn options parameter function #78

Merged
merged 5 commits into from
Sep 24, 2015
Merged

Added showOn options parameter function #78

merged 5 commits into from
Sep 24, 2015

Conversation

mtgibbs
Copy link
Contributor

@mtgibbs mtgibbs commented Aug 24, 2015

We have conditionally shown controls on our site and didn't want to have to make individual tours for different user types. Please take a look at this proposed update inspired by Issue 65.

#65

JsFiddle showing it in action:

http://jsfiddle.net/mattgibbs/f8n0gLrL/4/

Implemented a version of @geekjuice comment in regards to skipping tours
that don't find their target element.

Reference to: #65
I'm  not sure about this commit, but if it creates a conflict on merge,
it's generated so it's easy to resolve.
});
if (typeof next.options.showOn !== 'undefined' && !next.options.showOn()) {
const index = this.steps.indexOf(next);
var nextIndex = forward ? index + 1 : index - 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor detail. var here should be const.

@geekjuice
Copy link
Contributor

Other than the small comment above, I think you can go ahead and minor version bump this to v1.2.0 since showOn is a new feature. To do that, just run gulp version:minor && gulp build to have all the .json files and distribution files updated.

Once that's done, I'll go ahead and merge this.

Thanks again for being patience while I was on hiatus 👍

@geekjuice suggested declaring nextIndex as a const.
gulp version:minor to 1.2.0
css output diffs that were generated appear to be line endings.

Omitting build output of:

dist/css/*.css
dist/eager/installHelper.js
@mtgibbs
Copy link
Contributor Author

mtgibbs commented Sep 24, 2015

Great! Thanks for reviewing it.

I've committed your feedback, bumped the version, and committed the install output.

I really enjoy these tours and if I run into something else I'll send it your way.

@geekjuice geekjuice self-assigned this Sep 24, 2015
@geekjuice
Copy link
Contributor

🎉 👍

geekjuice added a commit that referenced this pull request Sep 24, 2015
Added showOn options parameter function
@geekjuice geekjuice merged commit d35aa94 into shipshapecode:master Sep 24, 2015
@mtgibbs mtgibbs deleted the conditional-show-step branch September 24, 2015 02:59
@dmackerman
Copy link

Is there no documentation for showOn?

@mtgibbs
Copy link
Contributor Author

mtgibbs commented Nov 10, 2015

@dmackerman

I guess not... :(

I didn't know where to submit the changes to the documentation. But what is it you want to know about it? Basically you have to define the showOn option as a function that returns a boolean. If it is defined, when the step is stepped to it will execute the function and look at the return value to decide whether or not the step should be shown. If it shouldn't be, it will skip to the next one.

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

Successfully merging this pull request may close these issues.

3 participants