Targeting One Tab Set in Multiple Tab Sets

Question

I have some JQuery that controls the open and close of tabs. It works great - until I have multiple sets of tabs. On click functionality impacts all the sets of tabs. I want to be able to isolate the functionality to the set of tabs that are being interacted with. How do I do this efficiently?

Each tab has a unique ID associated with it

data-tab="tab-{{ blockID }}{{ tabCounter }}">


  
    
      TAB ONE
    CONTENT GOES HERE
  
  
    
      TAB TWO
    
      CONTENT TWO GOES HERE

$('ul.tabs li').click(function() {
  var tab_id = $(this).attr('data-tab');
  var icon = $(this).find('span i');

  //Remove on all list items
  $('ul.tabs li').removeClass('active');
  $('ul.tabs li span i').removeClass('teal rm-90');

  //Remove on all div items
  $('.tab-content').removeClass('db');
  $('.tab-content').addClass('dn');

  //Add on current list item
  $(icon).addClass('teal rm-90');
  $(this).addClass('active');

  //Add on current div item
  $('ul.tabs').find("#" + tab_id).removeClass('dn');
  $('div.tabs').find("#" + tab_id).removeClass('dn');
  $('ul.tabs').find("#" + tab_id).addClass('db');
  $('div.tabs').find("#" + tab_id).addClass('db');
})

Alexander Nied · Accepted Answer

The problem is that your code is mostly iterating over the whole document looking for matches. I would recommend finding the tab container (ul.tabs) of the clicked element, then scoping all of your subsequent operations to that container. For instance, rather than searching for all li elements inside of a tab container and removing the .active class, like you have at line six of your JS:

$('ul.tabs li').removeClass('active');

...we can instead leverage .parents() to find the specific tab container, and only look inside that container, like so:

var tabContainer = $(this).parents('ul.tabs').eq(0); // this is a little overly safe-- if you are confident that the ul.tabs will always be the immediate parent, you can just use .parent()
tabContainer.find('li').removeClass('active');

I created a snippet using this technique, and verified that, while it was not working quite right with your code above, when we scope our search to the tab container of the clicked element, it works correctly. I took the liberty of guessing at a few styles to help illustrate the point:

$('ul.tabs li').click(function() {
  // cache $this
  var $this = $(this);
  
  var tab_id = $this.attr('data-tab');
  var icon = $this.find('span i');
  
  // find the parent tab container
  var tabContainer = $this.parents('ul.tabs').eq(0);

  //Remove on all list items
  tabContainer.find('li').removeClass('active');
  tabContainer.find('li span i').removeClass('teal rm-90');

  //Remove on all div items
  tabContainer.find('.tab-content').removeClass('db');
  tabContainer.find('.tab-content').addClass('dn');

  //Add on current list item
  $(icon).addClass('teal rm-90');
  $(this).addClass('active');

  //Add on current div item
  tabContainer.find("#" + tab_id).removeClass('dn');
  $('div.tabs').find("#" + tab_id).removeClass('dn');
  tabContainer.find("#" + tab_id).addClass('db');
  $('div.tabs').find("#" + tab_id).addClass('db');
})

.active {
  background-color: pink;
}

.db {
  display: block;
}

.dn {
  display: none;
}



  
    
      TAB ONE
    CONTENT GOES HERE
  
  
    
      TAB TWO
    
      CONTENT TWO GOES HERE
    
  



  
    
      TAB ALPHA
    CONTENT GOES HERE
  
  
    
      TAB BETA
    
      CONTENT TWO GOES HERE

As long as we're reviewing your code, a few other things--

You're not taking advantage of chaining to shorten operations, nor are you caching repeated selections in variables-- this will hurt the performance of your code.
At least as far as the example as posted is concerned, $('div.tabs') never matches anything, and could be omitted.

With these two adjustments, we can shorten the code up a bit and improve its performance:

$('ul.tabs li').click(function() {
  var $this = $(this);
  var tab_id = $this.attr('data-tab');
  var icon = $this.find('span i');
  var tabContainer = $this.parents('ul.tabs').eq(0);

  //Remove on all list items
  tabContainer.find('li').removeClass('active');
  tabContainer.find('li span i').removeClass('teal rm-90');

  //Remove on all div items
  tabContainer.find('.tab-content')
    .removeClass('db')
    .addClass('dn');

  //Add on current list item
  icon.addClass('teal rm-90');
  $this.addClass('active');

  //Add on current div item
  tabContainer.find("#" + tab_id)
    .removeClass('dn')
    .addClass('db');

})

.active {
  background-color: pink;
}

.db {
  display: block;
}

.dn {
  display: none;
}



  
    
      TAB ONE
    CONTENT GOES HERE
  
  
    
      TAB TWO
    
      CONTENT TWO GOES HERE
    
  



  
    
      TAB ALPHA
    CONTENT GOES HERE
  
  
    
      TAB BETA
    
      CONTENT TWO GOES HERE

Finally, if you intend on including a lot of these on any given page, it might be advisable to leverage event delegation in .on in lieu of .click to get a bit more of a performance boost.

Targeting One Tab Set in Multiple Tab Sets

Answers (1)

Related Questions