Need help changing getElementById

Hi,

I am rather inexperienced with javascript, so please excuse any incorrect references to certain types of statements etc…

I have a CSS based menu that I wanted to tweak with a little javascript. I have some code that works now, but I need to expand it a bit further. Currently the layout is as such:


<div id="leftmenu">
<ul>
  <li>...

My javascript works when it is structured as listed above, but I didn’t realise that when I put it into my clients CMS, it was going to add an extra div like so:


<div id="leftmenu">
<div id="cat_763640_divs"> /*extra line here*/
<ul>
  <li>...

I can’t simply switch the target name to the cat_763640_divs because this id changes for each menu they create.

This is what my javascript code looks like, could someone explain how to get this working? Or why it is not working? Thanks in advance :slight_smile:

startList = function() {
 
if (document.getElementById) {
	navRoot = document.getElementById("leftmenu");
  for (j=0; j<navRoot.childNodes.length; j++) {
    ulNode = navRoot.childNodes[j];
    if (ulNode.nodeName=="UL") {
      for (i=0; i<ulNode.childNodes.length; i++) {
        node = ulNode.childNodes[i];
        if (node.nodeName=="LI") {
          /* open the selected menu on page load */
          if (node.className=="selected") {
            node.className = "selected on";
          } else {
            node.className = "off";
          }
        node.onclick=function() {
          
          /* close all other menus */
	        navRoot = document.getElementById("leftmenu");
          for (j=0; j<navRoot.childNodes.length; j++) {
            ulNode = navRoot.childNodes[j];
            if (ulNode.nodeName=="UL") {
              for (i=0; i<ulNode.childNodes.length; i++) {
                node = ulNode.childNodes[i];
                if (node.nodeName=="LI") {
                  node.className = "off";
                }
              }
            }
          }

          /* open the selected menu - added selected class when turned on -JD */
          this.className = (this.className == "on" || this.className == "selected on") ? "off" : "selected on";
          }
        }
      }
    }
   }
  }
}
window.onload=startList;


This is the existing code that gets the UL node.

for (j=0; j<navRoot.childNodes.length; j++) {
    ulNode = navRoot.childNodes[j];
    if (ulNode.nodeName=="UL") {

You can remove the above code from where it’s used in a couple of different places, and replace it with this instead:


ulNode = navRoot.getElementsByTagName('ul')[0];

When removing the old code, don’t forget to remove the for’s matching closing brace.

Hey Paul,

Thanks a ton for the help! I did what you said and replaced the for statement… When I replace both appearances of the ‘for’ statement, none of the submenus open on click.

If I just replace the first ‘for’ statement they open and close on click, but when you click on one, the others stay open. I was hoping to have the others close when you click on one of them…

Any thoughts?

You can see it here:
http://dustingrof.com/WSI-AccutracCapitalSolutions/dgc-bugs/index2.html

What is getting in your way is that each time you click, the entire page is reloaded once again. You can tell that it’s happening because the location bar puts a # symbol at the end of the URL.

That’s happening because you aren’t preventing the default behaviour from occurring, for the click.

You need to return false from the onclick function in order to prevent that default behaviour.

As a bonus, here’s some cleaning up of that code.

First, there’s a lot of duplication of code going on. Let’s move that duplicated code out to a separate function called forEachLI


function forEachLI(root, func) {
    var navRoot, ulNode, i, node;
    navRoot = document.getElementById("leftmenu");
    ulNode = navRoot.getElementsByTagName('ul')[0];
    for (i = 0; i < ulNode.childNodes.length; i += 1) {
        node = ulNode.childNodes[i];
        if (node.nodeName == "LI") {
            func.call(node);
        }
    }
}

You can use the above function as follows - I don’t even think the comment is necessary now:


/* close all other menus */
forEachLI(navRoot, function () {
    this.className = "off";
});

The onclick function should be in a separate function, instead of being newly defined each and every time that it’s assigned.


function handleNavClick() {
    var navRoot = document.getElementById("leftmenu");

    /* close all other menus */
    forEachLI(navRoot, function () {
        this.className = "off";
    });

    /* open the selected menu - added selected class when turned on -JD */
    this.className = (this.className == "on" || this.className == "selected on") ? "off" : "selected on";
    return false;
}

Now the onclick assignment can use a reference to that handleNavClick function instead:


node.onclick = handleNavClick;

Next, the other part of the repeated code can use the forEachLI function too:


var navRoot = document.getElementById("leftmenu");
forEachLI(navRoot, function () {
    /* open the selected menu on page load */
    if (this.className=="selected") {
        this.className = "selected on";
    } else {
        this.className = "off";
    }
    this.onclick = handleNavClick;
});

Finally, by moving the script down to the end of the body, just before the </body> tag, you can remove the necessity for the onload part of the code


[s][color="red"]startList = function() {
    if (document.getElementById) {[/color][/s]
        ...
    [s][color="red"]}
};
window.onload=startList;[/color][/s]

You are left then with this cleaner and fully working code:


function forEachLI(root, func) {
    var navRoot, ulNode, i, node;
    navRoot = document.getElementById("leftmenu");
    ulNode = navRoot.getElementsByTagName('ul')[0];
    for (i = 0; i < ulNode.childNodes.length; i += 1) {
        node = ulNode.childNodes[i];
        if (node.nodeName == "LI") {
            func.call(node);
        }
    }
}
function handleNavClick() {
    var navRoot = document.getElementById("leftmenu");

    /* close all other menus */
    forEachLI(navRoot, function () {
        this.className = "off";
    });

    /* open the selected menu - added selected class when turned on -JD */
    this.className = (this.className == "on" || this.className == "selected on") ? "off" : "selected on";
    return false;
}

var navRoot = document.getElementById("leftmenu");
forEachLI(navRoot, function () {
    /* open the selected menu on page load */
    if (this.className=="selected") {
        this.className = "selected on";
    } else {
        this.className = "off";
    }
    this.onclick = handleNavClick;
});

Wow, thanks for the bonous, this was a big help getting me to understand what is happening in this code. I will have to start looking at some basic javascript tutorials so I don’t have this problem again :blush:

Thanks again!