0

The value of u and v are coming correctly as 1 and 0... but the values are not being used in the statement "t[i].children[v].onclick=function(){}" ..!! This works perfectly when I used 1 in the place of u and 0 in the place of v!!! Here is the code :

    <div class="dropdown">
    <span class="menu-toggler">Menu</span>
    <ul class="dropdown-menu">
    <li>First Part</li>
    <li>Second Part</li>
    </ul>
</div>



<script type="text/javascript">

function init()
{
    t = document.getElementsByClassName("dropdown");
    for(i=0;i<t.length;i++)
    {
        v = getMenuToggler(t[i]); // v is becoming 0 correctly
        u = getDropDown(t[i]); // u is becoming 1 correctly
 // the next statement is not working properly!!!
        t[i].children[v].onclick=function(){if(this.parentNode.children[u].classList.contains("menu-

open")){this.parentNode.children[u].classList.remove("menu-open");}else{this.parentNode.children

[u].classList.add("menu-open");}}
    }
}

function getDropDown(x) // this function is all right... u can ignore this
{
    for(i=0;i<x.childElementCount;i++)
    {
        if(x.children[i].classList.contains("dropdown-menu"))
        return i;
    }
    return -1;
}

function getMenuToggler(y) //this function is all right... u can ignore this
{
    for(i=0;i<y.childElementCount;i++)
    {
        if(y.children[i].classList.contains("menu-toggler"))
        return i;
    }
    return -1;
}
</script>
<script>window.onload=init;</script>

thank you in advance... :)

  • 2
    shouldn't the class be defined for the
      tag?
    – goelakash Apr 12 '15 at 20:21
  • 1
    nope @goelakash ...cuz thats y i added the t[i].children[0] .... :) so u don't need the class for
      – Samarpan Das Apr 12 '15 at 20:24
    • 2
      @vihan1086 `childNodes` will also get Text nodes. Bad idea. – Sebastian Simon Apr 12 '15 at 20:24
    • @vihan1086 tried already... but its even worse... it gets all the text and comment nodes... – Samarpan Das Apr 12 '15 at 20:24
    • Have you read about the infamous '[closures within loops](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example)' problem? – David Thomas Apr 12 '15 at 20:26
    • 1
      `init` shouldn't be called as a function. Then it is called immediately and `window.onload`'s value is set to what `init` returns. It doesn't return anything. Also, because it's called before `onload`, it is called before any of the elements are defined – Downgoat Apr 12 '15 at 20:31

    3 Answers3

    2
    window.onload=init();
    

    If you write it like this then init is immediately called instead of the function assigned as an event listener.

    What you want is:

    window.onload=init;
    

    Or in a more standardized and modern way:

    window.addEventListener('load',init);
    

    If you call init too early by mistake like in your original code then the DOM hasn’t loaded yet so you can’t get any elements.

    Now in your assigned function to the onclick property t[i] won’t be defined when you write t[i].children[1].

    Because the function is assigned to the onclick property of t[i].children[0] you can replace t[i].children[1] by this.parentNode.children[1] which will be equivalent.

    Sebastian Simon
    • 18,263
    • 7
    • 55
    • 75
    • thank you.. but I have tried that too.... the result is the same!!! error : Cannot read property 'children' of undefined. – Samarpan Das Apr 12 '15 at 20:32
    • and heres what... if I do not use the loop and instead just use : t[0].children[0].onclick=function(){t[i].children[1].classList.add("menu-open");} ... it works beautifully.... but I want to do it using an array!!! bcoz later on i might want to add more elements with class="dropdown" .... – Samarpan Das Apr 12 '15 at 20:36
    • For me the `onclick` part works fine but the assigned function brings some issues as `t[i]` won’t be defined within that function. I’ll edit my answer for a possible fix. – Sebastian Simon Apr 12 '15 at 20:39
    • Also `t[i]` is not defined because in your `for` loop you have set `i` as a global variable. At the end of each iteration the third statement in the `for` head — `i++` — executes. After the loop executes for the last time, `i` is incremented one last time. An access to `t[i]` will essentially lead to an index in `t` that exceeds the maximum index by one. – Sebastian Simon Apr 12 '15 at 20:45
    • it works flawlessly.... thank you once again.... the this.parentNode.children[0] does it!!!! :) thank you – Samarpan Das Apr 12 '15 at 20:48
    • Yeah, I had to edit it again… it should be `this.parentNode.children[1]`, right? I accidentally left the wrong index number in there… – Sebastian Simon Apr 12 '15 at 20:49
    0
    <div class="dropdown" onclick="add_class(this)">
        <span>Menu</span>
        <ul>
            <li>First Part</li>
            <li>Second Part</li>
        </ul>
    </div>
        <script>
    
        function add_class(dropdown) {
          dropdown.getElementsByTagName("ul")[0].className = (dropdown.getElementsByTagName("ul")[0].className == "menu-open") ? "" : "menu-open";
       }
        </script> 
    
    Sašo Krajnc
    • 133
    • 2
    • 3
    • 11
    -1

    There are 1 flaws in your code

    1. you are not calling the function from anywhere , it is just declared, so i called from ul when clicked .
      • so there is no need to call this line t[i].onclick ,
      • my code is adding class menu-open to the element ul.

    DEMO : http://jsfiddle.net/yosrknv3/1/

        <div class="dropdown">
            <span>Menu</span>
            <ul onclick="init_menu()">
                <li >First Part</li>
                <li>Second Part</li>
                </ul>
            </div>
    <script>
        function init_menu()
        {
                t=document.getElementsByClassName("dropdown");                                for(i=0;i<t.length;i++)
                {
                     console.log("i am clicked"+t[i],t[i].children[1]);
                     t[i].children[1].classList.add("menu-open");
                }
       }
    </script> 
    
    Shelly
    • 355
    • 1
    • 7
    • thanks a lot @Shelly but i knew that adding onclick="init_menu()" would do it... :) but i actually wanted it to get ready for clicking as soon as the page loaded.... replacing with this.parentNode.children[1] does the trick... :) thanks – Samarpan Das Apr 12 '15 at 20:52