2
\$\begingroup\$

I've written a small function which displays a div related to an anchor element using data attributes.

The data attribute is used to match the class of the relevant div which is then displayed whilst the others are hidden.

All very basic, however I'm wondering if there is anyway to improve it? Is there a better solution than using the data attribute? Any best practices?

$(".tabs li a").click(function() {
    var cssClass = "." + $(this).data("class");

    // Add active class to current tab, remove from siblings
    $(this).addClass("active").parent().siblings().find("a").removeClass("active");

    // Find the associated content and show it, hide its siblings
    $(".tabs-content").find(cssClass).show().siblings().hide();

    return false;
});

jsfiddle example here


Edit

This ended up being re-written for production using classes instead of data attributes, like so:

$(".tabs li a").click(function() {
    var cssClass = "." + $(this).attr("class");
    
    $('.tabs a.active').removeClass("active"); // Find the previously active tab and remove it
    $(this).addClass("active");                // Add active class to current tab
    $(".tabs-content").find('> div').hide();   // Hide the all tab divs
    $(".tabs-content").find(cssClass).show()   // Find the associated div and display it
    
    return false;
});​

Updated jsfiddle

\$\endgroup\$
1
\$\begingroup\$

$(".tabs-content").find(cssClass).show().siblings().hide(); is confusing and not easy to read.

Otherwise it seems to be working. As for best practices:

  • Make sure your content is accessible to non JS users, such as search engines or browsers without JS.
  • Consider using the Bootstrap tab plugin or jQuery UI tab functionality. Of course if you don't already use one of those, keep your simple jQuery function.
\$\endgroup\$
1
  • \$\begingroup\$ Thanks Cygal, I've broken the confusing line down into two lines (jsfiddle updated). I'm using the modernizr library so I can add rules to display the content to non JS users. I've also built the site using ZURB's foundation framework which includes a tab plugin, I just have to use IDs rather than classes in my markup.. \$\endgroup\$ – Damian Mar 21 '12 at 17:46
0
\$\begingroup\$

Agree with cygal, I would however use the id(#hash) to call the tabcontent

var tabs = function(){

   var tabContent = $(".tabcontent"),
       tabMenu    = $("ul.tabs");

       tabContent.hide().filter(":first").show();

       $("ul.tabs li a").on('click', function(){

          tabMenu.find("li").removeClass("active");

          $(this).parent().addClass("active");

          tabContent.filter(this.hash).fadeIn('slow'); 

          return false;

        }).filter(":first").click(); //trigger click on first anchor so active class is added
}

//invoke
tabs();
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.