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?

Stubborn
  • 995
  • 4
  • 17
  • 30
  • See the linked question for ways of handling this oft-encountered issue. – Pointy Dec 27 '15 at 16:51
  • That's because wheb user click on the input, the `i` var is not defined. – Mosh Feu Dec 27 '15 at 16:53
  • @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. –  Dec 27 '15 at 16:57
  • 3
    @Stubborn: simply change this: `inputs[i].style.width = "500px";` to this `this.style.width = "500px";` –  Dec 27 '15 at 16:58
  • 1
    @squint Thanks, that works! – Stubborn Dec 27 '15 at 16:59
  • @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. – Pointy Dec 27 '15 at 17:00

1 Answers1

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));
}
Josh Crozier
  • 233,099
  • 56
  • 391
  • 304