Event listener set on the wrong elment

Question

I was wondering why this javascript doesn't work:

var element = document.querySelector('#element1');
var button= document.querySelector('#button1');
button.addEventListener('click', function() {
    element.doSomething();
});

var element = document.querySelector('#element2');
var button= document.querySelector('#button2');
button.addEventListener('click', function() {
    element.doSomething();
});

Both of the buttons are doing something to "element2".

Is it something like the event listener keeps a pointer to the variable instead of its value ?

Thanks for your answers

Why the same name to both variables? The later one is overwriting the previously defined ones. — Tushar, Apr 28 '16 at 15:29
Related: [How do JavaScript closures work?](http://stackoverflow.com/q/111102/218196) — Felix Kling, Apr 28 '16 at 15:54

Mr. Polywhirl · Accepted Answer · 2016-04-28T18:34:02.333

You can wrap your block-level code with an anonymous; self-executing function.

This is similar to when you wrap { /* {{code}} */ } a block of code, with curly-braces, in Java. You can have duplicate variables, as long as they exist in their own block.

That way, the variables can be the same. You should really create a separate function and pass in the selector IDs; especially if they are doing the same thing.

(function() {
  HTMLDivElement.prototype.doSomething = function () { 
    this.innerHTML = this.innerHTML == 'Hello' ? 'Goodbye' : 'Hello';
  }
}());

(function() {
  var element = document.querySelector('#element1');
  var button = document.querySelector('#button1');
  button.addEventListener('click', function() {
    element.doSomething();
  });
}());

(function() {
  var element = document.querySelector('#element2');
  var button = document.querySelector('#button2');
  button.addEventListener('click', function() {
    element.doSomething();
  });
}());

div {
  display: inline-block;
  width: 5em;
  height: 2em;
  border: thin solid black;
  text-align: center;
  line-height: 2em;
  margin-bottom: 0.25em;
}

<div id="element1">?</div>
<input type="button" id="button1" value="Button 1" />
<br />
<div id="element2">?</div>
<input type="button" id="button2" value="Button 2" />

You should try this.

function addButtonClick(btnSelectorId, elSelectorId) {
  document.querySelector(btnSelectorId).addEventListener('click', function() {
    document.querySelector(elSelectorId).doSomething();
  });
}

addButtonClick('#button1', '#element1');
addButtonClick('#button2', '#element2');

score 0 · Answer 2 · edited Apr 28 '16 at 15:35

0

Try this, you have given same name to both the elements & buttons which is overwriting the previous one :

var element1 = document.querySelector('#element1');
var button1= document.querySelector('#button1');
button1.addEventListener('click', function() {
    element1.doSomething();
});

var element2 = document.querySelector('#element2');
var button2 = document.querySelector('#button2');
button2.addEventListener('click', function() {
    element2.doSomething();
});

edited Apr 28 '16 at 15:35

SpoonMeiser

19,918
8
50
68

answered Apr 28 '16 at 15:34

Vaibhav Jain

687
4
15

Yes I know, I am not asking to correct the code, but to understand why it doesn't work on a lower level – Tribes Apr 28 '16 at 15:37
Yeah, the new value replaces the previous value. – Vaibhav Jain Apr 28 '16 at 15:38

score 0 · Answer 3 · answered Apr 28 '16 at 15:36

The problem is that you are referencing the elment variable inside your event handler functions. When you assign a new value to elment, you change its value and this affects both handlers.

The solution: use a different variable name for each:

var element1 = document.querySelector('#element1');
var button = document.querySelector('#button1');
button.addEventListener('click', function() {
    element1.doSomething();
});

var element2 = document.querySelector('#element2');
var button = document.querySelector('#button2');
button.addEventListener('click', function() {
    element2.doSomething();
});

Or better yet, perhaps, do away with the closure variables altogether and just query as needed:

document.querySelector('#button1').addEventListener('click', function() {
    var element = document.querySelector('#element1');

    element.doSomething();
});

document.querySelector('#button2').addEventListener('click', function() {
    var element = document.querySelector('#element2');

    element.doSomething();
});

Ok, so you are saying that the internal function is evaluated at the end of the "main" function ? — Tribes, Apr 28 '16 at 15:40
@Tribes No, the internal function is evaluated when a click event occurs, which could be seconds, minutes, or hours after your code originally runs. — JLRishe, Apr 28 '16 at 15:41

Bubble Hacker · Answer 4 · 2016-04-28T15:42:57.217

0

You are using the same variable for both the elements and so the last declared variable is the one that will be used.
For example:

x = "Hello";
x = "World";
console.log(x); //This will print out "World"

You need to declare them as different variables. Example with your code(Notice I changed the second element):

var elment = document.querySelector('#element1');
var button= document.querySelector('#button1');
button.addEventListener('click', function() {
    elment.doSomething();
});

var elment2 = document.querySelector('#element2'); //Changed to elment2
var button2 = document.querySelector('#button2'); //Changed to button2
button2.addEventListener('click', function() {
    elment2.doSomething();
});

edited Apr 28 '16 at 15:42

answered Apr 28 '16 at 15:37

Bubble Hacker

6,425
1
17
24

Yes but x = "Hello"; console.log(x); //This will print out "Hello" x = "World"; console.log(x); //This will print out "World" – Tribes Apr 28 '16 at 15:38
@Tribes That's because you are writing the values to the console immediately. Event handlers do not execute immediately when you add them; they execute when the event happens. – JLRishe Apr 28 '16 at 15:40
@tribes the thing is the events are registered when the DOM is loaded, which gives the browser to replace the values :). – Vaibhav Jain Apr 28 '16 at 15:41
No. It will only print out the last declared since we are declaring it to a new value. So in the beginning the value was "Hello" and after we decleared "x" again, value is now "World" and so it will print out "World". – Bubble Hacker Apr 28 '16 at 15:41

Event listener set on the wrong elment

4 Answers4