search

simontabor / jquery-toggles

jQuery plugin to make easy toggle buttons
http://simontabor.com/labs/toggles
MIT License
362 stars 67 forks source link

Option to rename toggle event, don't depend on '$' being jQuery #34

Closed GerryG closed 10 years ago

GerryG commented 10 years ago

In attempting to use this widget, I ran into a couple of snags. Our javascript dependencies are a bit complex, and the trigger('toggle') was getting intercepted by some prototype code, so I added on option to give it a name.

Also, the jQuery '$' isn't available everywhere, so I renamed some of the references to jQuery. Probably better to use the jQuery conventions of passing in the jQuery reference and binding it locally to $, but this was simpler.

simontabor commented 10 years ago

Could your issue be caused by the fact that root.jQuery isn't defined? I won't be using the jQuery variable, as I automatically try to support other libraries (see https://github.com/simontabor/jquery-toggles/blob/master/toggles.js#L286).

simontabor commented 10 years ago

It does, however, look as if I've left it so that $ is only used properly in the factory, not throughout the while Toggles class

GerryG commented 10 years ago

I think it is this second comment that is the issue. I think you need to bind $ for the whole plugin. I was just trying to get it to work for my env.

GerryG commented 10 years ago

You only addressed the ability to rename the event, and not the other problems.

The use of $ is problematic for us and needs to be fixed.

The data-toggle-on seems like a bug, or am I missing something? We'll have to keep using a fork until these issues are addressed.

simontabor commented 10 years ago

https://github.com/simontabor/jquery-toggles/commit/caff5cae6d9b1ab3dcbf805812b7c7f63685fa82 fixes the data-toggle-on issue (as mentioned in the separate issue)

the use of $ should have been fixed in https://github.com/simontabor/jquery-toggles/commit/c1848e04625f1edab154efc3c9887b35aead8731, is it still not working?

GerryG commented 10 years ago

Yesh, missing the on thing at first. Looking at the $ thing.

GerryG commented 10 years ago

Our release that uses this is too far down the process to add this. I will try to at least test it, but I doubt I'll be able to change back to your updated version from my fork to get it working. Thanks.

simontabor commented 10 years ago

https://github.com/simontabor/jquery-toggles/blob/master/js/wrap.js#L23 - those will now be used as the context for the whole module (before i'd made a mistake and the module was just using $ regardless. let me know if it doesn't work