patternjavascriptMinor
jQuery Tab function
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?
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
Either 2 arrays :
Or maybe 1 array with objects like
Or maybe you could even just consider tracking how many tabs you have:
And go from there.
Also, using
Personally, I would probably go for something like this ( after renaming the divs and tabs tab0 ,tab1,tab3 and div0,div1, and div2 ):
I did not test this code, but the intent and approach should be clear.
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.addEventListenerPersonally, 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.