Replace tab event bind with .delegate? about skeleton HOT 8 CLOSED

Hey Scott -

Thanks for the note - I will definitely check this out and see if it's cross-device and cross-browser compatible. I will be testing that before the next release :)

from skeleton.

Hey Scott -

Just started doing some work on the next version of Skeleton and tried the above code out and it didn't work. I would love to get a pull request going for this (or at least fully functional code). Not a JS optimization expert, but would love to hear more about the advantages too.

Will leave this open for now, but am going to work to close down all the issues and make sure Skeleton is in tip top shape for it's next release :)

from skeleton.

Scott forgot to include one of the parameters in the delegate function. It should be:

$('body').delegate('ul.tabs > li > a', 'click', function(e) {

instead of

$('body').delegate('ul.tabs > li > a', function(e) {

That works for me. As to what the benefits are, I wouldn't know. :P

from skeleton.

I actually forked and initiated a pull request shortly after Dave noted the issue, so it's been fixed for a while. If you're curious about the benefits, this is a good starting point, and you can find references all over the place.

Though, on researching that link again, there's now a disclaimer that the functionality will be encompassed in the on method... so we'll see how this evolves alongside jQuery.

from skeleton.

Couldn't merge in cause the typo, but this is in the branch now :)

from skeleton.

Hey Dave -

Since the commit for this is so recent, I'd recommend switching to the .on syntax instead of .delegate. The jQuery docs state that the latter is being phased out, and Skeleton now includes jQuery-1.7.1 by default. It's a quick fix -

$('body').delegate('ul.tabs > li > a', 'click', function(e) {

to

$('body').on('click', 'ul.tabs > li > a', function(e) {

I can pull-request this after a quick test, if you'd like; but the two calls are functionally identical, according to jQuery.delegate docs, and it really is very cosmetically trivial. It's only for future-proofing when .delegate is properly deprecated.

from skeleton.

Hey man -

Go for that and get a pull request and I will merge it in :) Thanks so much!

from skeleton.

Merged new ".on" and jQuery 1.7.1

from skeleton.