Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've got a tab menu and functions that change the tabs color on click, giving the clicked tab a different color from the others. The thing is, when I click one, I give it another background-color easily but I'm also setting all the other tabs background-color.

In my largest menu there are 6 tabs:

        document.getElementById('tab1').onclick = changeColor1;
        document.getElementById('tab2').onclick = changeColor2;
        document.getElementById('tab3').onclick = changeColor3;
        document.getElementById('tab4').onclick = changeColor4;
        document.getElementById('tab5').onclick = changeColor5;
        document.getElementById('tab6').onclick = changeColor6;

        function changeColor1() {
            document.getElementById('tab1').style.backgroundColor = "white";
            document.getElementById('tab2').style.backgroundColor = "silver";
            document.getElementById('tab3').style.backgroundColor = "silver";
            document.getElementById('tab4').style.backgroundColor = "silver";
            document.getElementById('tab5').style.backgroundColor = "silver";
            document.getElementById('tab6').style.backgroundColor = "silver";
            return false;
        }

        function changeColor2() {
            document.getElementById('tab1').style.backgroundColor = "silver";
            document.getElementById('tab2').style.backgroundColor = "white";
            document.getElementById('tab3').style.backgroundColor = "silver";
            document.getElementById('tab4').style.backgroundColor = "silver";
            document.getElementById('tab5').style.backgroundColor = "silver";
            document.getElementById('tab6').style.backgroundColor = "silver";
            return false;
        }

...
...

... and it goes on for the 6 tabs. I'm sure this can be reduced, I just don't know how.

share|improve this question
add comment

1 Answer

up vote 4 down vote accepted

I would suggest changing your html and giving all the tags some class like 'tag', then you'll be able to do something like:

<button id="tab1" class="tab" onClick="changeColor(this.id)">B1</button>
<button id="tab2" class="tab" onClick="changeColor(this.id)">B2</button>
<button id="tab3" class="tab" onClick="changeColor(this.id)">B3</button>
<script type="text/javascript">
    function changeColor(id) {    
        var tabs = document.getElementsByClassName('tab')
        for (var i = 0; i < tabs.length; ++i) {
            var item = tabs[i];
            item.style.backgroundColor = (item.id == id) ? "white" : "silver";
        }
    }
</script>

see fiddle - http://fiddle.jshell.net/vP7ku/

must shorter, Hua?

share|improve this answer
    
That is great! I'll accept and upvote in a minute! Can you explain the line item.style.backgroundColor = (item.id == id) ? "white" : "silver";? –  chiapa Jun 25 at 14:29
    
Sure, its a short condition, meaning: if item.id equals given id return white, else return silver into item.style.backgroundColor –  user1213618 Jun 25 at 14:30
    
Hmm, interesting but weird syntax. What about the ++i instead of i++? –  chiapa Jun 25 at 14:32
    
Not matter, you can use i++ as well –  user1213618 Jun 25 at 14:35
    
About the short condition its a common useful syntax. see - sitepoint.com/shorthand-javascript-techniques –  user1213618 Jun 25 at 14:36
show 1 more comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.