QuerySelectorAll not working with onclick event
Asked Answered
T

1

13

Scenario

I've got some text inputs and I want them to have a width of 500px when they're clicked.

My code

var inputs = document.querySelectorAll("input[type=text]")
for(i=0; i<inputs.length; i++){
	inputs[i].onclick = function(){
		inputs[i].style.width = "500px";
	}
}
<input type="text" id="sometext">

What's not working

On chrome console, I'm getting "Cannot read property 'style' of undefined" on the following line: inputs[i].style.width = "500px"; when I click on the input.

My question

How can I fix my code?

Thessalonian answered 27/12, 2015 at 16:49 Comment(6)
See the linked question for ways of handling this oft-encountered issue.Killigrew
That's because wheb user click on the input, the i var is not defined.Kodok
@Pointy: Technically it's a duplicate but in that question, a new scope is actually needed to solve it. That's not the case here.Myrnamyrobalan
@Stubborn: simply change this: inputs[i].style.width = "500px"; to this this.style.width = "500px";Myrnamyrobalan
@squint Thanks, that works!Thessalonian
@squint why isn't a new scope necessary (other than something like the availability of let)? edit oh OK, sure. Maybe that should be included in that centralized repository of answers, as this is a pretty common case.Killigrew
C
27

"Cannot read property 'style' of undefined"

The reason you're seeing this error is because the for loop has already ended and i is outside of the bounds of the inputs NodeList when the click event is fired. In other words, since the last element's index in the array is one less than the length, inputs[i] is undefined since i is equal to the length of the NodeList when the click event listener is fired.

To work around this, you could change inputs[i] to this in order to access the clicked element. In doing so, you avoid having to use i (which is what was causing the problem):

var inputs = document.querySelectorAll("input[type=text]")
for (i = 0; i < inputs.length; i++) {
  inputs[i].addEventListener('click', function() {
    this.style.width = "500px";
  });
}

Alternatively, you could use an IIFE in order to pass the value of i on each iteration:

var inputs = document.querySelectorAll("input[type=text]")
for (i = 0; i < inputs.length; i++) {
  (function(i) {
    inputs[i].addEventListener('click', function() {
      inputs[i].style.width = "500px";
    });
  })(i);
}

You could also use the .bind() method to pass the value of i:

var inputs = document.querySelectorAll("input[type=text]")
for (i = 0; i < inputs.length; i++) {
  inputs[i].addEventListener('click', function(i) {
    inputs[i].style.width = "500px";
  }.bind(this, i));
}
Caltrop answered 27/12, 2015 at 17:5 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.