0

I want to fade in 4 div boxes, one by one. In css they have the opacity = 0.

Here my JavaScript code:

function fadeIn() {
    var box = new Array(
        document.getElementById('skill1'),
        document.getElementById('skill2'),
        document.getElementById('skill3'),
        document.getElementById('skill4')
    );

    var pagePosition = window.pageYOffset;

    if (pagePosition >= 1000) {
        for (var i = 0; i < box.length; i++) {
            setTimeout(function(i) {
                box[i].style.opacity = "1";
            }, i * 500);
        }
    }
}

Well, the function has to start if you scroll the page to the position 1000px and called in the body-tag:

Without the setTimeout it works, but with this function the console says:

Uncaught TypeError: Cannot read property 'style' of undefined

I'm a beginner and want to understand JS, so please don't provide an answer using jQuery.

Qantas 94 Heavy
  • 15,750
  • 31
  • 68
  • 83
Keyla
  • 3
  • 2
  • 2
    Welcome to [so]. It appears your question can be answered here: [**JavaScript closure inside loops – simple practical example**](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example). Please let me know if it solves your issue. Thanks! – Qantas 94 Heavy Jun 04 '15 at 13:06

2 Answers2

4

By the time your your timeout runs, the loop has finished processing, so i will always be the last iteration. You need a closure:

for(var i = 0; i < box.length; i++) {
  (function(index) {
    setTimeout(function() {
        box[index].style.opacity = "1";
    }, index*500);
  })(i)
}
tymeJV
  • 103,943
  • 14
  • 161
  • 157
  • 1
    @JamesThorpe -- It'll work..but I'll change it for clarity. Edit: Oh - the `timeout` function, yeah, that was wrong :D – tymeJV Jun 04 '15 at 13:06
  • @JamesThorpe I think a different variable name would make the operation clearer, but there's actually two `i` variables in tyme's original version - when it enters the function scope, it discards the outside `i` naming to just use `i` as a local argument. – Katana314 Jun 04 '15 at 13:08
  • Also worth noting that `i` would actually be one more than the number of items in the `box` array, which is what was causing the `undefined` error. – James Thorpe Jun 04 '15 at 13:08
  • @Katana314 exactly, which would again have been `undefined` as it wouldn't be passed by the timeout – James Thorpe Jun 04 '15 at 13:09
  • Why would `i` be one more? I thought `undefined` was coming from the `i` in the function being passed to the `timeout` – tymeJV Jun 04 '15 at 13:10
  • @tymeJV Ah yes, that too. Without the `i` in the timeout function, it would be using the `i` from the outer scope, which the for loop would have incremented to beyond the end of the array, so `box[i]` would be undefined. There's too much wrong here :) – James Thorpe Jun 04 '15 at 13:11
  • Actually, Thorpe appears correct; I hadn't noticed `i` was being passed as an argument again inside the setTimeout scope. I was referring to the first function. The example looks fine now. – Katana314 Jun 04 '15 at 13:11
  • `i` would not increment beyond the array! The array length is `4` - `i` starts at `0` - up by `1` until not less than `4`. `4 < 4 == false` – tymeJV Jun 04 '15 at 13:17
  • If the length is 4, the highest index is 3, ie `box[0]` to `box[3]` are fine, but after the loop is finished (and by the time the timeout ran), `i` would be 4, and `box[4]` is `undefined` – James Thorpe Jun 04 '15 at 13:25
3

The problem is the scope. When anonymous function executes inside timeout i variable has the last value of the i in the iteration. There are two solutions:

1) Use an IIFE:

for (var i = 0; i < box.length; i++) {
  (function (i) {
    setTimeout(function (i) {
      box[i].style.opacity = "1";
    }, i * 500);
  })(i);
}

for (var i = 0; i < 5; i++) {
  (function(i) {
    setTimeout(function() {
      console.log(i);//prints out 0 1 2 3 4
    }, i * 500)
  })(i);
}

2) Using let:

 for (let i = 0; i < box.length; i++) {

    setTimeout(function (i) {
        box[i].style.opacity = "1";
    }, i * 500);
}

"use strict";
for (let i = 0; i < 5; i++) {
  setTimeout(function() {
    console.log(i)
  }, i * 500)
}

The let statement declares a block scope local variable, optionally initializing it to a value.

Keep in mind let is feature fo ecmaScript 6.

Alex Char
  • 32,879
  • 9
  • 49
  • 70
  • Even if `set i` is an ES6 feature I haven't read yet, I think you linked to the wrong MDN article; it's detailing a `Set` object – Katana314 Jun 04 '15 at 13:13