Toggle Classname using document.getElementsByClassName - JavaScript

Question

I have a left sidebar menu which has submenus, i want each menu item to toggle a classname "active" so the submenu will open i have CSS for it.

The thing is i am using document.getElementsByClassName to select and iterate all of the menu items and is only working for the first element, i have been searching and it has something to do with closures and i am trying different solutions but its not working.

i am making the function so i can use it to toggle a classname of another div and not the one clicked, in that case i use and ID.

var toggleClassname = function (otherDiv, sameDiv) {
    var divToToggleClass;
//are we going to use ID and toggle the classname of another div ?
    if (sameDiv) {
        divToToggleClass = this;
    } else {
        divToToggleClass = document.getElementById(otherDiv);
    }
    console.log(divToToggleClass);
    var className = divToToggleClass.className + ' ';

    if (~className.indexOf(' active ')) {
        divToToggleClass.className = className.replace(' active ', '');
    } else {
        divToToggleClass.className += ' active';
    }
};
var MenuItemsArray = document.getElementsByClassName("classOfMyMenuItems");

for (var i = 0; i < subMenuItemsArray.length; i++) {
    MenuItemsArray[i].addEventListener('click', function () { toggleClassname(null, true) }, false);
}

i have been trying using [].forEach.call or wrapping the function in another that returns the function, not working.

I am doing this in pure javascript, cant use the new .classList.toggle i would also use attachEvent to be more backwards compatible (old IE).

edited, sorry that was something i tried, it should just call `toggleClassname ` — David Lee, Jun 22 '17 at 04:03
OK. Currently `toggleClassname` is only receiving the arguments `null` and `true`, so it doesn't know what the current element is and its `this` is *not* the current element so `divToToggleClass = this` isn't doing what you want. You could fix this by using `toggleClassname.call(this, null, true)`, to set its `this` to the same `this` as in the event handler (which *is* the right element), or you could just add another argument to the function to take a reference to the current element. — nnnnnn, Jun 22 '17 at 04:04
yup, that is working, but its not clear to me, doesnt know which is "this" isnt the element that was "clicked" since i am using ".addEventListener" ? — David Lee, Jun 22 '17 at 04:11

score 0 · Answer 1 · answered Jun 22 '17 at 04:09

0

This answer might help you:

addEventListener using for loop and passing values

Without going too deep into your code, I'd say if you try and make it

for (var i = 0; i < subMenuItemsArray.length; i++) {
    (function () {
        var k = i;

         MenuItemsArray[k].addEventListener('click', function () { toggleClassname(null, true) }, false);
    }()); // immediate invocation
}

That should work.

answered Jun 22 '17 at 04:09

Erick T.

92
3

That won't make any difference because neither `i` nor `k` is referenced in the event handler. – nnnnnn Jun 22 '17 at 04:19

score 0 · Accepted Answer · answered Jun 22 '17 at 04:23

The problem is that within your toggleClassname() function this is not equal to the clicked element. It will actually be either undefined or window depending on whether your code is running in strict mode or not.

A click handler bound with addEventListener() will have this set to the clicked element, so within the following anonymous function:

function () { toggleClassname(null, true) }

...the value of this is the element in question. But then you call toggleClassname() and don't pass it a reference to the clicked element or set its this value. You can explicitly set it using .call():

function () { toggleClassname.call(this, null, true) }

Toggle Classname using document.getElementsByClassName - JavaScript

2 Answers2