SitePoint Sponsor

User Tag List

Results 1 to 6 of 6
  1. #1
    SitePoint Member
    Join Date
    Feb 2011
    Posts
    8
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    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:

    HTML Code:
    <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:

    HTML Code:
    <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

    Code JavaScript:
    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;

  2. #2
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,729
    Mentioned
    104 Post(s)
    Tagged
    4 Thread(s)
    This is the existing code that gets the UL node.

    Code:
    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:

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

    When removing the old code, don't forget to remove the for's matching closing brace.
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  3. #3
    SitePoint Member
    Join Date
    Feb 2011
    Posts
    8
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    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-AccutracCa...gs/index2.html

  4. #4
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,729
    Mentioned
    104 Post(s)
    Tagged
    4 Thread(s)
    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.
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  5. #5
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,729
    Mentioned
    104 Post(s)
    Tagged
    4 Thread(s)
    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

    Code javascript:
    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:

    Code javascript:
    /* 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.

    Code javascript:
    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:

    Code javascript:
    node.onclick = handleNavClick;

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

    Code javascript:
    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

    Code:
    startList = function() {
        if (document.getElementById) {
            ...
        }
    };
    window.onload=startList;
    You are left then with this cleaner and fully working code:

    Code javascript:
    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;
    });
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  6. #6
    SitePoint Member
    Join Date
    Feb 2011
    Posts
    8
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by paul_wilkins View Post
    As a bonus, here's some cleaning up of that code.
    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

    Thanks again!


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •