Replace tab event bind with .delegate?

[scottrabin]

The tabs.js code can use a less memory intensive jQuery function, .delegate, to accomplish the same functionality in fewer LOC and with a smaller memory footprint. Instead of iterating over each ul.tabs and creating a new anonymous function each time, you could bind as follows:

$('body').delegate('ul.tabs > li > a', function(e) {
    //Get Location of tab's content
    var contentLocation = $(this).attr('href');

    //Let go if not a hashed one
    if(contentLocation.charAt(0)=="#") {

        e.preventDefault();

        //Make Tab Active
        $(this).parent().siblings().children('a').removeClass('active');
        $(this).addClass('active');

        //Show Tab Content & add active class
        $(contentLocation).show().addClass('active').siblings().hide().removeClass('active');

    }
});

The above is an example of the approach, and not actually tested. I can create a tested pull request if you'd prefer. An additional advantage of this approach is that it supports dynamic tabs; developers can create tabs that will still have the same functionality without rebinding events, or add new tabs to existing tab bars. The number of cases where that will be useful is limited, hence why the primary reason to modify the tabs is the reduced memory footprint/advantage in speed.

[dhg]

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 :)

[dhg]

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 :)

[jellysandwich]

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

[scottrabin]

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.

[dhg]

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

[scottrabin]

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.

[dhg]

Hey man -

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

[dhg]

Merged new ".on" and jQuery 1.7.1