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

Shepherd might need jQuery... #79

Closed
tannermares opened this issue Sep 1, 2015 · 4 comments
Closed

Shepherd might need jQuery... #79

tannermares opened this issue Sep 1, 2015 · 4 comments

Comments

@tannermares
Copy link

First

I'd like to say I love this library. It's combo with tether make it super powerful. It's also mostly not very opinionated. You leave it up to the developer to implement any uncommon or specific features. Like skipping missing steps or highlighting the current step. Which, up until now, I have been totally fine with. I do think there is one thing Shepherd might need to be opinionated about and that is a dependency on jQuery.

Why I think this is needed

Any tour that goes beyond pointing to things in the current DOM(steps in modals and on other pages) is becomes immediately apparent that there needs to be a mechanism to skip steps to allow the user multiple paths on the tour. This is a really helpful method for teaching people to use an app ("If you don't have any files, click here to add them, or hit next to move on."). Rather then adding arbitrary attributes to the DOM for document.querySelector() to look for using jQuery's :visible pseudo selector becomes immensely useful. With it one could select from an array of similar items and tether to the one that is actually showing.

Why haven't I done it already?

I am completely willing to add the dependency and make the change necessary to use it. But before I did I wanted to make sure it was something others thought would be useful and would benefit from. Else, I'll just fork it and make my own jQuery-Shepherd 😬 (I don't want to).

NOTE: AFAIK this would be the only place that needs it.

FROM

opts.element = document.querySelector(selector);

TO

opts.element = $(selector).first();

Thanks again for all your work on this. I'd love to contribute if it was something you're interested in.

@geekjuice
Copy link
Contributor

Thanks again for using Shepherd!

While I completely understand the possible ease of use with including jQuery, I'm also in favor of keeping the dependencies for the library as few as possible.

For your use case, would it be possible to adjust how getAttachTo works by allowing arrays (nodes and selectors), selector, and nodes?

Another option could be to invert control on the step target and allow it to be a function that can return the target node. With this route, the end user is free to determine how to obtain the element, be it vanilla Javascript, jQuery, Zepto, etc. For example:

tour.addStep('example', {
  attachTo: {
    element() {
      return $('.selector').first(); 
    },
    to: 'left'
  }
  ...
});

Thoughts?

@tannermares
Copy link
Author

@geekjuice: The second option you mention would be perfect. It also maintains the theme of letting the end user implement the details. If it used document.querySelector() as a sensible default or let you use the method to include the library of your choice, I would say that is a win win. There are just too many awesome convenience and otherwise impossible selectors jQuery adds. Makes the tours that much more powerful.

@mtgibbs
Copy link
Contributor

mtgibbs commented Oct 7, 2015

@tannermares Have you made any progress with this implementation idea?

I'm not sure exactly what your goals are, but this change may help you hack a solution in until someone implements this idea.

#78

@darrynten
Copy link

A million times NO

Please do not add jQuery as a dependency there are so many outer plugins like this that have a hard dep on jQuery and it's really no longer needed in this day and age.

This is nice and fast on mobile w/o jQuery, and adding it will do way more harm than good.

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

4 participants