addEventListener is just running the function, not saving the function

Question

So I am trying to set up a function that will hide and show certain parts of the page, without the use of any outside libraries with Javascript. My problem seems to be that addEventListener is not attaching the event listener to the DOM=object but just running it.

The parts on the site I am using are:

    <a class="tab" href="#index" id="index">Weapons</a>
    <a class="tab" href="#armor" id="armor">Armor</a>
    <a class="tab" href="#items" id="items">Items</a>

    <div id="index_content" class="tab_content">
        This is the content to display in weapons
    </div>
    <div id="armor_content" class="tab_content">
        Welcome to armor!
    </div>
    <div id="items_content" class="tab_content">
        Items are probably the best of the  tabs.
    </div>

My Javascript is:

function clear(key){
  "use strict";
  key = String(key);


  //hides all content in items
  for (var i = 0; i < itemArray.length; i++) {
      document.getElementById(itemArray[i]+"_content").style.display = "none";
  }

  //shows current item
  document.getElementById(key).style.display = "block";
  return;
}

function tabsInit(){
  "use strict";
  for(var i = 0; i < itemArray.length; i++){
document.getElementById(itemArray[i]).addEventListener("click",clear(itemArray[i]));
  }
}

window.onload = function(){
  "use strict";
  tabArray = document.getElementsByClassName("tab");

  //add Items into item array
  for(var i = 0; i < tabArray.length; i++){
      itemArray[i] = tabArray[i].id;
  }
  tabsInit();
}

You call the function and assign what it returns to the event listener. — epascarello, Jan 15 '18 at 18:10
I am ok with the function running and all that; I am more upset that it isn't adding the event listener and I can't tell why. — Kyle Atterson, Jan 15 '18 at 18:13
Possible duplicate of [How to use addEventListener](https://stackoverflow.com/questions/13771025/how-to-use-addeventlistener) — Loïc Faure-Lacroix, Jan 15 '18 at 18:15
@KyleAtterson: Because you're running the function, so you're not giving it an event listener... you're giving it the *return value* of the function you're running. — , Jan 15 '18 at 18:17

score 0 · Answer 1 · answered Jan 15 '18 at 18:13

0

use let to keep the variable i in the correct scope and use an anonymous function to call clear on it..

function tabsInit(){
  "use strict";
  for(let i = 0; i < itemArray.length; i++){
    document.getElementById(itemArray[i])
      .addEventListener("click",()=>clear(itemArray[i]));
  }
}

answered Jan 15 '18 at 18:13

I wrestled a bear once.

22,983
19
69
116

I never heard of the let keyword before. Since I am self taught on JS, it probably isn't that much of a surprise that I never heard it. Thanks for your help, and now I got a new keyword to look up and try to understand! – Kyle Atterson Jan 15 '18 at 18:38

score 0 · Accepted Answer · 2018-01-15T18:28:43.270

You're assuming you need the ID to get to the bound element. You don't. Assign clear as the event handler, and use this inside the function, though I think you were setting the wrong item to "block".

Also, I'm not sure why you're creating an array of IDs. You can just use the list of elements directly, and create a similar list of the _content elements if needed.

Here's a rewrite of your code.

document.addEventListener("DOMContentLoaded", function() {
  "use strict";

  const tabArray = document.querySelectorAll(".tab");
  const contentArray = document.querySelectorAll(".tab_content");

  tabsInit();

  function tabsInit() {
    for (var i = 0; i < tabArray.length; i++) {
      tabArray[i].addEventListener("click", clear);
    }
  }

  function clear() {
    //hides all content in items
    for (var i = 0; i < contentArray.length; i++) {
      contentArray[i].style.display = "none";
    }

    //shows current item
    document.querySelector("#" + this.id + "_content").style.display = "block";
  }
});

div[id$=_content]:not(:first-of-type) {
  display: none;
}

<a class="tab" href="#index" id="index">Weapons</a>
<a class="tab" href="#armor" id="armor">Armor</a>
<a class="tab" href="#items" id="items">Items</a>

<div id="index_content" class="tab_content">
  This is the content to display in weapons
</div>
<div id="armor_content" class="tab_content">
  Welcome to armor!
</div>
<div id="items_content" class="tab_content">
  Items are probably the best of the tabs.
</div>

Honestly I don't know why I was doing half the things I did in my initial draft, I been "stuck" on this problem probably for about two months. — Kyle Atterson, Jan 15 '18 at 18:36

score 0 · Answer 3 · answered Jan 15 '18 at 18:15

0

You're assigning the function result as a event listener. You should use a function instead:

.addEventListener("click", function() { clear(itemArray[i]); })

Now that will be called every time the event fires.

answered Jan 15 '18 at 18:15

Pedro Fialho

444
8
18

this won't work without either a closure or using `let` since the value of `i` will always be `itemArray.length` by the time it's called. not my -1 btw. – I wrestled a bear once. Jan 15 '18 at 18:17
Fair enough; I just provided the solution for the main problem ("running the function, not 'saving' the function"), there are lots of other non-related things in the code that can be improved if we're gonna review it. – Pedro Fialho Jan 15 '18 at 18:23
@PedroFialho: It was non-related to the question, but it's directly related to your solution. Using `var` isn't an issue in the question, but it is here. – Jan 15 '18 at 18:38

score 0 · Answer 4 · answered Jan 15 '18 at 18:48

0

var demo = document.getElementById("demo")

demo.addEventListener("click",function(){
//code here

}
)

<button id="demo"></button>

answered Jan 15 '18 at 18:48

Jasim Droid

137
1
3

addEventListener is just running the function, not saving the function

4 Answers4