Is there a better way to code this

[evuseem]

I have a javascript code written as:

 function change_it(itemID){
      // Toggle visibility between none and inline
      if ((document.getElementById(itemID).style.display == 'none'))
      {
        document.getElementById(itemID).style.display = 'inline';
      } else {
        document.getElementById(itemID).style.display = 'none';
      }
  }
  
  function close_it(itemID){
    document.getElementById(itemID).style.display = 'none';
  }

And I have like say 5 menu items (menu1, menu2, menu3, menu4,menu5)

On click of a particular menu, i want the table displayed, relevant to that menu and hide the others. To do this I have called the javascript as follows:

example:

<table id="menu_tbl" cellpadding=5 style="display:none;" size=10>
   <tr>
   <td><a href="#h" onClick="change_it('menu1') & close_it('menu2') & close_it('menu3') & close_it('menu4') & close_it('menu5')">menu1</a></td>
   <td><a href="#e" onClick="change_it('menu2') & close_it('menu1') & close_it('menu3') & close_it('menu4') & close_it('menu5')">menu2</td>
   </td>
               .....

As you see, more the menu's the longer gets the onClick line .. I am wondering if there is a better way of doing this

Thank you,
Evu

[Deadeye]

thats one way of doing it, or have a close all and call the before you "open" the correct menu.

function change_it(itemID) {
closeEverything();
document.getElementById(itemId).style.display = 'inline';
}
function closeEverything() {
document.getElementById("menu1").style.display = 'none';
document.getElementById("menu2").style.display = 'none';
document.getElementById("menu3").style.display = 'none';
...
}

OR

you can close everything and open the item all within the same method.

function change_it(itemID) {
switch(itemID)
case 1:
   document.getElementById("menu1").style.display = 'inline';
   document.getElementById("menu2").style.display = 'none';
   document.getElementById("menu3").style.display = 'none';
case 2:
   document.getElementById("menu1").style.display = 'none';
   document.getElementById("menu2").style.display = 'inline';
   document.getElementById("menu3").style.display = 'none';
....

}

I'd do it the first way that I've listed with the close everything of the 3 options listed so far.

Wesley

[curtiss]

I agree with Deadeye. The best way to do this is to write a close_all function that will systematically loop through all of your items and hide them.

[evuseem]

Sorry for the late reply. I was trying to use the first method to run the code. But somethings not working fine or it must be just that I am going wrong somewhere. Anyway with the help of your code I came up with something like this:

var a= new Array();
a[0]= "menu1";
a[1]= "menu2";
a[2]= "menu3";
a[3]= "menu4";

function change_it(itemID) {
document.getElementById(itemID).style.display='block';
var i= a.findindex(itemID);
b=a.slice(0,4);
b.splice(i,1);
document.getElementById(b).style.display='none';
}

It works fine for the first time but second time, when i click on the link it shows the portion of html and suddenly disappears. Could someone look at it and tell me where could the code be going wrong.

Thank you.