Changing a <div> class with JavaScript

Question

I am trying to change the class of certain tags with an onclick() event. The basic premise is to have the background image of each tag change when the user clicks on them, sort of stimulating a "menu selection".

Here is the code I have:

<style type="text/css">

.navCSS0
{
    background-image:url('news_selected.png');
    width:222px;
    height:38px;
}

.navCSS1
{
    width:222px;
    height:38px;
}

.container_news
{
    background-image:url('itsupdates.png');
    height:330px;
    width:965px;
}

.container_left
{
    margin-top:90px;
    margin-left:20px;
    float:left;
    height:auto;
    width:auto;
}

</style>
</header>

<script>
//global arrays to store nav positions, menu options, and the info text
var navid_array = new Array();
navid_array[0] = 'nav1';
navid_array[1] = 'nav2';
navid_array[2] = 'nav3';
navid_array[3] = 'nav4';
navid_array[4] = 'nav5';


//Takes the navid selected, and goes into a loop where the background of the selected menu item is changed to a image
//with rounded corners, while the backgrounds of the other menu items are changed back to light grey or stay the same.
//There is also a call to the change_info() function when the selected menu item has been located.
function nav_color_swap(navid)
  {
    for(var i = 0; i < navid_array.length; i++) {
          if(navid == navid_array[i]) 
            {
                document.getElementById(navid).className = "navCSS0";
            }
          else 
            {
                document.getElementById(navid).className = "navCSS1";
            } 
        }
  }

</script>

<body>
<div class="container_news">
<div class="container_left">
    <div class="navCSS1" id="nav1" onclick="nav_color_swap(this.id)"><a href="#" onclick="nav_color_swap(this.id)">Blah 1</a></div>
    <div class="navCSS1" id="nav2" onclick="nav_color_swap(this.id)"><a href="#" onclick="nav_color_swap(this.id)">Blah 2</a></div>
    <div class="navCSS0" id="nav3" onclick="nav_color_swap(this.id)"><a href="#" onclick="nav_color_swap(this.id)">Blah 3</a></div>
    <div class="navCSS1" id="nav4" onclick="nav_color_swap(this.id)"><a href="#" onclick="nav_color_swap(this.id)">Blah 4</a></div>
    <div class="navCSS1" id="nav5" onclick="nav_color_swap(this.id)"><a href="#" onclick="nav_color_swap(this.id)">Blah 5</a></div>
    </div>
    </div>
</body>
</html>

The problem is, when I run this code, nothing happens... the only one that actually changes is the last menu item ("Blah 5")... any thoughts?

I really suggest you look into jquery and start using it, it will make everything much more simple. — Andreas Bonini
– Andreas Bonini, Commented Dec 22, 2009 at 22:38

corn on the cob · Accepted Answer · 2020-08-28 08:13:57Z

6

the header tag does now exist, but that's where you put the header of your page, like the navigation. For titles and meta tags, use the head tag.
the only legal children of <html> are <head> and <body>. The sample code has a <script> element that's a child of <html>
nav_color_swap sets the classname for navid, rather than navid_array[i], when navid != navid_array[i]. This is likely the source of your problem.
When that's fixed, the click handlers for the <a> will set the class name for all elements to "navCSS1", since the <a> don't have ID attributes.

Since, at most, two element classes will need to change with each click, I recommend keeping track of the current element rather than looping over all elements:

    <style type="text/css">
    .selected {
        background-image:url('news_selected.png');
    }
    .news, .news li {
        padding: 0;
        margin: 0;
        list-style-type: none;
    }
    ...
    </style>
    <script type="text/javascript">
    function createNavSelector(curr_nav, selectedClass, unselectedClass) {
        return function (nav) {
            curr_nav.className = unselectedClass;
            curr_nav = nav;
            curr_nav.className = selectedClass;
        }
    }
    </script>
    </head>
    <body>
    ...
    <div class="container_news">
      <ul class="news">
        <li id="nav1" onclick="nav_select(this)">Blah 1</li>
        <li id="nav2" onclick="nav_select(this)">Blah 2</li>
        <li id="nav3" class="selected" onclick="nav_select(this)">Blah 3</li>
        <li id="nav4" onclick="nav_select(this)">Blah 4</li>
        <li id="nav5" onclick="nav_select(this)">Blah 5</li>
      </ul>
    </div>
    <script type="text/javascript">
    var nav_select = createNavSelector(document.getElementById('nav3'), 'selected', '');

Since the #nav* appear to be a list of news items or a list of navigation items, I switched to using elements, since they carry more semantic information than <div>s. At this point, div-vs-ol/ul is largely a matter of personal preference.

I also renamed functions and element classes to reflect their purpose, rather than how they fulfill that purpose. That way, you can change their behavior without requiring a change in name.

Did you use the <a> to support old versions of IE? I wouldn't worry about anything older than IE 6.

As per No Refund, No Return's comment, here's some links to get you started on debugging JS using Firebug. Safari also has a good debugger, if that's your browser of choice.

edited Aug 28, 2020 at 8:13

corn on the cob

2,3403 gold badges24 silver badges36 bronze badges

answered Dec 22, 2009 at 22:42

outis

77.7k23 gold badges155 silver badges228 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

No Refunds No Returns Over a year ago

gimme some free debugging, please.

Napoli Over a year ago

I changed the if and else clauses to do this instead: document.getElementByID(navid_array[i]).className = etc... It works! Thank you very much.

outis Over a year ago

@NRNR: I know, I shouldn't just give Napoli the fish. I mostly said what's wrong with the original code because I was going to offer the more efficient, scalable alternative.

mrvisser · Accepted Answer · 2009-12-22 22:43:30Z

0

A few things with your code:

Your 'head' tags should be <head> </head>, not <header> </header>
Your <script> tag should be inside the <head> element, and it's best practice to specify the language with: <script language="JavaScript" type="text/javascript"> ... </script>
Invoke your code in a function that is triggered at the onload event of the body to be sure the elements your javascript is referencing have actually been initialised on the page

answered Dec 22, 2009 at 22:43

mrvisser

9445 silver badges9 bronze badges

2 Comments

kangax Over a year ago

No, there's no need to include deprecated-for-more-than-10-years-now "language" attribute.

mrvisser Over a year ago

You mean there's no need to type those 22 chars every time I open a script tag any more? This calls for some champagne.

Gabriel McAdams · Accepted Answer · 2009-12-22 22:47:07Z

0

It looks like the problem is this:

Your anchor tag (link) is surrounded by a div (the div itself doesn't take up any more space than the link inside it in this case). When the user clicks the link, its running the onclick of the link (and its passing the id of the link, which is not added as an attribute).

Also, you need to move your script tag into your header (by the way, its "head", not "header")

edited Dec 22, 2009 at 22:47

answered Dec 22, 2009 at 22:40

Gabriel McAdams

58.5k12 gold badges67 silver badges78 bronze badges

Comments

Senthil · Accepted Answer · 2014-05-03 15:24:51Z

0

You could optimize your code in several ways.. a quick solution would be correcting a logical issue in your code

Replace 'navid' in two places with navid_array[i]

document.getElementById(navid_array[i]).className

    if(navid == navid_array[i]) 
        {
            document.getElementById(navid_array[i]).className = "navCSS0";
        }
     else 
        {
            document.getElementById(navid_array[i]).className = "navCSS1";
        }

answered May 3, 2014 at 15:24

Senthil

2891 gold badge2 silver badges10 bronze badges

2 Comments

Brandon Buck Over a year ago

If navid == navid_array[i] then what benefit do you gain by replacing navid with navid_array[i] in the getElementById calls? If you would like to make suggestions then please make comments, answers should provide a solution to the problem.

Senthil Over a year ago

Please have a closer look at the for loop. navid is common for the entire function whereas navid_array[i] changes for every iteration of the loop. What happened in that logic is Eg: 1st iteration, if cond is true and changes class name of navid; 2nd iteration, cond will fail and changes the classname again for navid. That's why class name change only affects last menu item which is at the last iteration. That is @Napoli code can affect only for last menu which comes as last iteration. clear :) Also try what I suggested and then tell it is a solution or not. Sorry!

Collectives™ on Stack Overflow

Changing a <div> class with JavaScript

4 Answers 4

3 Comments

2 Comments

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

3 Comments

2 Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related