0

I am trying to make a gallery.

I succeeded in all the steps except the final one when clicking on a thumbnail would change a text in a paragraph into another text that I put in an array.

While I'm inside a for-loop, I want the text of the index 2 to show when I click the image of the index 2. I seem to have followed the right syntax, but still the paragraph shows "undefined" as a result. However when I choose an index, like whichever picture I click, I want to show the text of the index 3, it works.

Here is my simplified code that should allow me to show a text while clicking on a paragraph. It would be awesome if you guys didn't use jQuery. Thank you ALL!

<!DOCTYPE html>
<html>
<body>

<p class="demo">1</p>
<p class="demo">2</p>
<p class="demo">3</p>

    <script>
        var cars = ["Saab", "Volvo", "BMW"];
        var para = document.getElementsByClassName("demo");
        for (k=0;k<cars.length;k++){
          para[k].onclick=function(){
            this.innerHTML = cars[k];
          }
        }
    </script>

</body>
</html>
Steve Mitcham
  • 5,268
  • 1
  • 28
  • 56
Chihab
  • 403
  • 5
  • 11
  • Problem is probably that the index `k` isn't getting preserved between loops in your `for` loop. You need to use a closure. – Matthew Dec 08 '15 at 15:56

5 Answers5

4

This is an issue with javascript's scoping. Your variable k, note how you increase it every time. When onclick gets called by a click event, k will have 3 as its value regardless of what image you clicked on.

You can try this workaround (note I cleaned up your code a little bit)

<!DOCTYPE html>
<html>
<body>

<p class="demo">1</p>
<p class="demo">2</p>
<p class="demo">3</p>

    <script>
    var cars = ["Saab", "Volvo", "BMW"];
    var para = document.getElementsByClassName("demo");

    for (k = 0; k < cars.length; k++) {
      (function (index) {
        para[index].onclick = function() {
          this.innerHTML = cars[index];
        }
      }(k));
    }
</script>

</body>
</html>

In this case, a separate copy is made of k for every value it takes on, this copy is available inside the function as "index".

For more information about the specific problem, refer to this post: JavaScript closure inside loops – simple practical example

Community
  • 1
  • 1
Azeirah
  • 6,176
  • 6
  • 25
  • 44
  • Perfect! thank you. that's what I'm looking for! I'll try to decrypt this code although I'm a beginner and I don't really understand the part where you put the function between parentheses. – Chihab Dec 09 '15 at 16:12
  • YESSSSS!!!!! I got it now! and was able to apply it on my real project (the small code I put in the question was just as simple demo to try the technique on.) which code I will attach in this comment (if possible, otherwise I will send it via email if interested. Thank you ALL again!!! – Chihab Dec 09 '15 at 16:31
3

Use a closure:

<script>
  var cars = ["Saab", "Volvo", "BMW"];
  var para = document.getElementsByClassName("demo");
  for (var k = 0; k < cars.length; k++) {
    (function (k) {
      para[k].onclick = function () {
        this.innerHTML = cars[k];
      }
    }(k));
  }
</script>

Fiddle: http://jsbin.com/zezuhitige

Reason:

The value will always be the last value of the loop. So you will always get k = 3 when the click happens, k value is executed at the time of click, which is 3. Closures prevent that, by executing the value right when it is assigned.

Praveen Kumar Purushothaman
  • 164,888
  • 24
  • 203
  • 252
0

A more flexible way of doing it would be something like this:

<div id="container">
    <p class="demo" data-car="Saab">1</p>
    <p class="demo" data-car="Volvo">2</p>
    <p class="demo" data-car="BMW">3</p>
</div>

JS:

document.getElementById('container').onclick = function(e) {
    e = e || window.event;
    var target = e.srcElement || e.target;
    while( target != this && !target.className.match(/\bdemo\b/)) {
        target = target.parentNode;
    }
    if( target != this) {
        // target is now one of the .demo elements
        target.innerHTML = target.getAttribute("data-car");
    }
};

This puts the data itself in with the element that it refers to, and only attaches a single event listener instead of one per car - in bigger applications, the less event handlers you have, the better!

Niet the Dark Absol
  • 320,036
  • 81
  • 464
  • 592
0

Look into JS closures and try the following:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Closures

document.addEventListener("DOMContentLoaded", function(event) { 
        var cars = ["Saab", "Volvo", "BMW"];
        var para = document.getElementsByClassName("demo");
        for (k=0;k<cars.length;k++){
          var clojureCar = cars[k];
          
          para[k].onclick=function(){
           
            this.innerHTML = clojureCar;
          }
        }
});
<!DOCTYPE html>
<html>
<body>

<p class="demo">1</p>
<p class="demo">2</p>
<p class="demo">3</p>

    <script>
        var cars = ["Saab", "Volvo", "BMW"];
        var para = document.getElementsByClassName("demo");
        for (k=0;k<cars.length;k++){
          para[k].onclick=function(){
            this.innerHTML = cars[k];
          }
        }
    </script>

</body>
</html>
Jorge Zuverza
  • 885
  • 8
  • 26
0

I think its a good practice to add an event listener like this. And because your click event was executed after your loop is done so its always 3 which is undefined.

var cars = ["Saab", "Volvo", "BMW"];
var para = document.getElementsByClassName("demo");
for (k=0;k<cars.length;k++){
  var div = para[k];
  (function (i) {
    div.addEventListener('click', function() { this.innerHTML = cars[i]; });
  })(k);
}
Harry Bomrah
  • 1,658
  • 1
  • 11
  • 14