jQuery Select Tabs based on URL #ID

Posted on

Problem

Activating jQuery tabs from URL #id is www.somedomain.com/index.php#tab3

Given this my best shot, and it does work but I feel it could be far far better.

Please tell me if this is totally shockingly bad JS.

$(document).ready(function() {
//When page loads...
$(".tab_content").hide(); //Hide all content
    //On Click Event
$("ul.tabs li").click(function (e) {
    e.preventDefault();
    $("ul.tabs li").removeClass("active"); //Remove any "active" class
    $(this).addClass("active"); //Add "active" class to selected tab
    $(".tab_content").hide(); //Hide all tab content
    var activeTab = $(this).find("a").attr("href"); //Find the href attribute value to identify the active tab + content
    $(activeTab).fadeIn(); //Fade in the active ID content
});
if(document.location.hash!='') {
    //get the index from URL hash
    var tabName = window.location.hash;
    $(tabName).show(); //Show tab from url hash
    var tabNumber = tabName.substr(4,1); //shorten #tab2 to just last digit "2"
    $("ul.tabs li:nth-child(tabNumber)").addClass("active").show();    
} else {

    $("ul.tabs li:first").addClass("active").show(); //Activate first tab
    $(".tab_content:first").show(); //Show first tab content
}

});

Thank you

Solution

Well, you could do some refactoring with this code, the only issue I see is that the low level functinality is implemented at the top level of your code. It would be better to extract the logic to separate functions, something like this (I also improved your code formatting a little bit):

$((function() { // Deleted the "document).ready" part, that is not necessary.
    // Hide all content.
    $(".tab_content").hide();

    // If hash is present in the URL, show tab by hash. Otherwise, show the first tab.
    if(document.location.hash!='') {
        showTabByHash();
    }
    else {
        showFirstTab()
    }

    // On the click event of a tab.
    $("ul.tabs li").click(function (e) {
        e.preventDefault();
        showTab($this);
    });
});

function showTab(tab) {
    // Remove any "active" class.
    $("ul.tabs li").removeClass("active");

    // Add "active" class to selected tab.
    tab.addClass("active");

    // Hide all tab content
    $(".tab_content").hide();

    // Find the href attribute value to identify the active tab + content
    var activeTab = tab.find("a").attr("href");

    // Fade in the active ID content.
    $(activeTab).fadeIn();
}

function showTabByHash() {
    // Get the index from URL hash.
    var tabName = window.location.hash;
    // Shorten #tab2 to just last digit "2".
    var tabNumber = tabName.substr(4,1);

    // Why are these two lines needed? Don't they work with the same tab element?
    $(tabName).show(); //Show tab from url hash
    $("ul.tabs li:nth-child(tabNumber)").addClass("active").show();
}

function showFirstTab() {
    // Activate first tab.
    $("ul.tabs li:first").addClass("active").show();

    // Show first tab content.
    $(".tab_content:first").show();
}

I do not see any other problems with the code (of course, it could be generalized a little bit, but that is not your goal if I understand right), apart from your coding style, which was a little bit cluttered in my opinion (I had to read it more than one time to completely understand it).
And if you are developing in Visual Studio, and these functions are going to be used from somewhere else, then adding XML comments to them would generally be a good idea.

Leave a Reply

Your email address will not be published. Required fields are marked *