How do you properly write a JavaScript "for" loop?

Question

The GitHub Repository that I have linked will not be used for any other questions (it will not be changed except to help THIS question.... IT WILL ONLY BE USED FOR THIS QUESTION

Note: I have done my research and I believe that my code should work

Ok, so if you require the rest of my code to make a good judgement on this, feel free to go to: My Github Repository which is only going to be used for this 1 question. In the GitHub Repository, there is also a CSS file, but there is no problem with it, just included it so that you can see ALL the code.

Yes, I know that many people on this website do not like it when people include "GitHub" links; however, it is easier for me to explain if I do not have all the code sitting here making a mess (it is easier if I can narrow down what code is giving me an error

Ok, so this "for" loop:

var dropdown = document.getElementsByClassName("dropdown-btn");
  var i;

  for (i = 0; i < dropdown.length; i++) { //this is not getting called for some reason
    dropdown[i].addEventListener("click", something());
    alert("Please work");
  }

is not actually running. When I put an "alert" above the for loop like:

var dropdown = document.getElementsByClassName("dropdown-btn");


 var i;
 alert("This works");
  for (i = 0; i < dropdown.length; i++) { //this is not getting called for some reason
    dropdown[i].addEventListener("click", something());
    alert("This does not work");
  }

The alert is called as soon as the method that the "for" loop is in, is called. I guess to explain my question, is there anything wrong with my "for" loop? (This works in a different .html file, so I am not sure why it is not working in my current workspace.)

UPDATE (February 18, 2019): Ok so I found what is causing the error.

For the person that commented and suggested that I use "console.log(dropdown.length);", this brought up an unexpected error:

function something(){


this.classList.toggle("active");
  var dropdownContent = this.nextElementSibling;
  if (dropdownContent.style.display === "block") {
  dropdownContent.style.display = "none";
  } else {
  dropdownContent.style.display = "block";
  }
}

As I originally said, this works in another file, but for some reason, it says that in "this.classList.toggle("active);", "toggle" is undefined. Is this supposed to be defined in this file, or is it like i think and a default "function"? Through all of my research, and through all the knowledge I have of the language of JavaScript, I am confident that it is a default "function", and unless someone can prove me wrong, I don't understand why this is not working.

Answer 1

The problem is that you are passing a function in as a parameter without wrapping it in a calling function.

There are two ways you would make this work.

1. Remove the () from the something function and call the alert inside of something

    for (i = 0; i < dropdown.length; i++) { //this is not getting called for some reason
      dropdown[i].addEventListener("click", something)
    }

    something () {
      alert('this is working')
      ...
    }

2. Put a single function call in your event handler and place both function calls inside

    for (i = 0; i < dropdown.length; i++) { //this is not getting called for some reason
      dropdown[i].addEventListener("click", function () {
        something();
        alert("This works");
      })
    }

Here is an example:

Answer 2

You are calling something instead of just passing the callable function when trying to add the eventListener. Remove the parentheses.

var dropdown = document.getElementsByClassName("dropdown-btn");
  var i;

  for (i = 0; i < dropdown.length; i++) { //this is not getting called for some reason
    dropdown[i].addEventListener("click", something);
  }
  
  function something() {
    this.classList.toggle("active");
    var dropdownContent = this.nextElementSibling;
    if (dropdownContent.style.display === "block") {
    dropdownContent.style.display = "none";
    } else {
    dropdownContent.style.display = "block";
    }
  }

<a class="dropdown-btn" href="#">Button 1</a>
<div style="display:none;">Dropdown Content Here</div>
<br />
<a class="dropdown-btn" href="#">Button 2</a>
<div style="display:none;">Dropdown Content Here</div>

Answer 3

I was watching the repository on GitHub, and I saw that there are a lot of errors in your html part, so that the JS part can not work.

Just about this loop, the current html part is:

<button class="dropdown-btn" onclick="drop()">OUR STORY 
  <i class="fa-caret-down"></i>
</button>

So that the first time the drop () function creates a new event listener on the button, but does not call it. If there is a second click on this button then 2 actions are then launched: the first adds again a new event listener click on the button, the second is the triggering of the event listener created the first time.

There are 2 possible solutions

Solution 1

<button class="dropdown-btn" onclick="something()">OUR STORY 
  <i class="fa-caret-down"></i>
</button>    

Solution 2

<button class="dropdown-btn" >OUR STORY 
  <i class="fa-caret-down"></i>
</button>

With in the part JS = ( ! remove function drop() declaration ! )

var dropdown = document.getElementsByClassName("dropdown-btn");

for (let i = 0, iMax=dropdown.length ; i < iMax ; i++) {
  dropdown[i].addEventListener("click", something);
}

for helping understand why your code is wrong, please try this sample code

var Compteur = 0;


function something(){
  console.log('call on something()', ++Compteur);
}


function drop(){
  var dropdown = document.getElementsByClassName("dropdown-btn");

  for (let i = 0; i < dropdown.length; i++) { 
    dropdown[i].addEventListener("click", something);
    console.log( 'adding event Listener on one dropdown-btn ', ++Compteur);
  }
}

<button class="dropdown-btn" onclick="drop()">OUR STORY </button>

Answer 4

Here is your problem.

Ask yourself why are you getting this.classList as undefined?

If you looked further you would find that this is the window object which has no classList . Now ask yourself why the window object?

It is because something() is an event listener that should be called in response to an event. In the line dropdown[i].addEventListener("click", something()); , you aren't assigning something() as an event handler, you are calling the method, without an event, so there is no this.classList as this is the window object.

As other answers have mentioned you need to change this to:

dropdown[i].addEventListener("click", something);

Which will assign the event something() to the click event, without calling it.

Complete Example

function something(){
  console.log(this); //For Debug prurposes
  this.classList.toggle("active");
  var dropdownContent = this.nextElementSibling;
  if (dropdownContent.style.display === "block") {
  dropdownContent.style.display = "none";
  } else {
  dropdownContent.style.display = "block";
  }
}


/* Loop through all dropdown buttons to toggle between hiding and showing its dropdown content - This allows the user to have multiple dropdowns without any conflict */
function drop(){
  var dropdown = document.getElementsByClassName("dropdown-btn");
  var i;

  for (i = 0; i < dropdown.length; i++) { //this is not getting called for some reason
    dropdown[i].addEventListener("click", something);
    console.log("Please wokrk");
  }
}

.sidenav a, .dropdown-btn {
  padding: 6px 8px 6px 16px;
  text-decoration: none;
  font-size: 20px;
  color: #818181;
  display: block;
  border: none;
  background: none;
  width: 100%;
  text-align: left;
  cursor: pointer;
  outline: none;
}

.active {
  background-color: green;
  color: white;
}

.dropdown-container {
  display: none;
  background-color: #262626;
  padding-left: 8px;
}

<button class="dropdown-btn" onclick="drop()">OUR STORY 
    <i class="fa-caret-down"></i>
  </button>
  <div class="dropdown-container">
    <a href="#about_us">ABOUT US</a>
    <a href="#community">COMMUNITY</a>
    <a href="#">HUMANITARIAN ACTION</a>
  </div>

On a side note, it is quite unusual to use inline events like onClick=drop() to then bind other event listeners to the same element. It is actually best practice to avoid inline javascript all together.

In the future, please include all code relevant to the question, in the question itself. As I mentioned in a comment it makes it easier for us to help you.