HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

jQuery Tab function

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
tabjqueryfunction

Problem

I have this script that I need to run a tab (jquery). Mainly I need to hide some div and add class (you sure have understood).

How should it be written in a more elegant and readable?

function fun1(){
$('#tab1 a').addClass('selected');
$('#tab2 a').removeClass('selected');
$('#tab3 a').removeClass('selected');

document.getElementById('div1').style.display='block';
document.getElementById('div2').style.display='none';
document.getElementById('div3').style.display='none';
}
function fun2(){
$('#tab1 a').removeClass('selected');
$('#tab2 a').addClass('selected');
$('#tab3 a').removeClass('selected');

document.getElementById('div1').style.display='none';
document.getElementById('div2').style.display='block';
document.getElementById('div3').style.display='none';
}
function fun3(){
$('#tab1 a').removeClass('selected');
$('#tab2 a').removeClass('selected');
$('#tab3 a').addClass('selected');

document.getElementById('div1').style.display='none';
document.getElementById('div2').style.display='none';
document.getElementById('div3').style.display='block';
}

window.onload = function() {
    document.getElementById('tab1').onclick=fun1;
    document.getElementById('tab2').onclick=fun2;
    document.getElementById('tab3').onclick=fun3;

    document.getElementById('div1').style.display='block';
    document.getElementById('div2').style.display='none';
    document.getElementById('div3').style.display='none';
}

Solution

It sure could be written in a more agreeable way ;)

First off, you might want to handle more tabs and divs in the future, so you can gather their ids into an array:

Either 2 arrays :

var tabs = ['tab1','tab2','tab3'];

var divs = ['div1','div2','div3'];

Or maybe 1 array with objects like { tabId : 'tab1' , divID : 'div1' }

Or maybe you could even just consider tracking how many tabs you have:

var tabCount = 3;

And go from there.

Also, using .onclick is too old skool, consider using https://developer.mozilla.org/en-US/docs/Web/API/EventTarget.addEventListener

Personally, I would probably go for something like this ( after renaming the divs and tabs tab0 ,tab1,tab3 and div0,div1, and div2 ):

var tabCount = 3;

function handleTabClick()
{
  //`this` contains the selected tab
  var selectedTabId = this.id;
  //Compare, and contrast
  for( var i = 0 ; i < tabCount ; i++ )
  {
    //Make sure the clicked tab id looks selected 
    if( 'tab' + i == selectedTabId )
    {
      $('#' + selectedTabId + ' a').addClass('selected');
      document.getElementById('div'+i).style.display='block';
    }
    else
    {
      $('#' + tab + i + ' a').removeClass('selected');  
      document.getElementById('div'+i).style.display='none';
    }
  }
}

document.addEventListener( "load" , function() initializeTabs{

  for( var i = 0 ; i < tabCount ; i++ )
  {
    //Get element
    var element = document.getElementById('tab'+i);
    //Add generic even listener
    element.addEventListener( "click" , handleTabClick );
    //Only show the first block ( when i is 0, and hence equates false )
    if( i )
      element.style.display='none';
    else
      element.style.display='block';   
  }
});


I did not test this code, but the intent and approach should be clear.

Code Snippets

var tabCount = 3;

function handleTabClick()
{
  //`this` contains the selected tab
  var selectedTabId = this.id;
  //Compare, and contrast
  for( var i = 0 ; i < tabCount ; i++ )
  {
    //Make sure the clicked tab id looks selected 
    if( 'tab' + i == selectedTabId )
    {
      $('#' + selectedTabId + ' a').addClass('selected');
      document.getElementById('div'+i).style.display='block';
    }
    else
    {
      $('#' + tab + i + ' a').removeClass('selected');  
      document.getElementById('div'+i).style.display='none';
    }
  }
}

document.addEventListener( "load" , function() initializeTabs{

  for( var i = 0 ; i < tabCount ; i++ )
  {
    //Get element
    var element = document.getElementById('tab'+i);
    //Add generic even listener
    element.addEventListener( "click" , handleTabClick );
    //Only show the first block ( when i is 0, and hence equates false )
    if( i )
      element.style.display='none';
    else
      element.style.display='block';   
  }
});

Context

StackExchange Code Review Q#45036, answer score: 3

Revisions (0)

No revisions yet.