jQuery Tab function

Question

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';
}

Answer 1

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.

Answer 2

You're hardly using jQuery at all here. You can either get rid of jQuery entirely (as in konijn's answer), or you can use it to your advantage. Either way is perfectly acceptable.

Secondly, the logic is basically the same for each tab, it's only the elements that change. What you have now is very repetitive because you basically re-do the logic for each tab. There's no need for this.

And as konijn points out, you're using old school event handling; again you can go with konijn's jQuery-less code, or you can use jQuery to your advantage.

Lastly, instead of having to hard-code your element IDs, it's more flexible to declare the relationship between a tab link and a DIV in HTML. For instance:

<a class="tab" href="#div1">1st tab</a>
<a class="tab" href="#div2">2nd tab</a>
<a class="tab" href="#div3">3rd tab</a>

<div id="div1">...</div>
<div id="div2">...</div>
<div id="div3">...</div>

Note that the href attribute for the links point to a DIV. The nice part is that this works even if the user doesn't have JavaScript enabled; a #someId link will scroll the page to show the element with that ID. It's not the same as having real tabs, but it's as good a substitute as any.

Point is that if you add more tabs, you only have to change the HTML. The JavaScript will "just work".

In the code below, I'm using jQuery to put all this together. Here's a demo

function initializeTabs(selector) {
  var tabs = $(selector), // get the tab links
      first;

  // helper function to hide a tab's element
  function deselectTab(tab) {
    tab.removeClass("selected").data("pane").hide();
  }

  // helper function to show a tab's element
  function selectTab(tab) {
    tab.addClass("selected").data("pane").show();
  }

  // get the elements that the tabs should show/hide
  tabs.each(function () {
    var tab = $(this),
        paneId = tab.attr("href");
    tab.data("pane", $(paneId));
  });

  // set up the click-event handling
  tabs.on("click", function (event) {
    var target  = $(this);

    // don't follow the links
    event.preventDefault()

    // stop here if the tab's already selected
    if( target.hasClass("selected") ) { return; }

    // hide the currently selected tab
    deselectTab(tabs.filter(".selected"));

    // show the clicked tab
    selectTab(target);
  });

  // show the first tab
  first = tabs.eq(0);
  selectTab(first);

  // hide the others
  tabs.not(first).each(function () {
    deselectTab($(this));
  });
}

// onload
$(function () {
  initializeTabs("a.tab");
});

Answer 3

Thought id add a version in here that simplifies the JavaScript but relies on the html using specific classes.

HTML

<div class="tabs-nav">
    <a href="#" class="item1 selected">Item 1</a>
    <a href="#" class="item2">Item 2</a>
    <a href="#" class="item3">Item 3</a>
</div>

<div class="content content-item1"></div>
<div class="content content-item2 hidden"></div>
<div class="content content-item3 hidden"></div>

JAVASCRIPT

$(function () {
    $('.tabs-nav a').click(function (e) {
        e.preventDefault();
        if (!$(this).hasClass("selected")) {
            $('.tabs-nav a').removeClass("selected");
            $('.content').hide();

            $('.content-' + $(this).attr("class")).show();
            $(this).addClass("selected");
        }
    });
});

CSS

.hidden{display:none;}
.selected{font-weight:bold;}

Job done! I am using classes but you could modify the code to use ids if you prefer. throwing it into the mix as an alternative to the 2 suggestions you already have.