Jquery using $this in a for loop in an each loop

Question

Here's my code for a basic jquery image slider. The problem is that I want to have many sliders on one page, where each slider has a different number of images. Each slider has the class .portfolio-img-container and each image .portfolio-img.

Basic html setup is as follows:

<div class="portfolio-item"><div class="portfolio-img-container"><img class="portfolio-img"><img class="portfolio-img"></div></div>

<div class="portfolio-item"><div class="portfolio-img-container"><img class="portfolio-img"><img class="portfolio-img"></div></div>

And javascript:

 $.each($('.portfolio-img-container'), function(){


        var currentIndex = 0,
            images = $(this).children('.portfolio-img'),
            imageAmt = images.length;


        function cycleImages() {
            var image = $(this).children('.portfolio-img').eq(currentIndex);
            images.hide();
            image.css('display','block');
        }


        images.click( function() {
            currentIndex += 1;
            if ( currentIndex > imageAmt -1 ) {
                currentIndex = 0;

            }

            cycleImages();
        });


    });

My problem comes up in the function cycleImages(). I'm calling this function on a click on any image. However, it's not working: the image gets hidden, but "display: block" isn't applied to any image. I've deduced through using devtools that my problem is with $(this). The variable image keeps coming up undefined. If I change $(this) to simply $('.portfolio-img'), it selects every .portfolio-img in every .portfolio-img-container, which is not what I want. Can anyone suggest a way to select only the portfolio-imgs in the current .portfolio-img-container?

Thanks!

You could set relevant context using: `cycleImages.call(this);` and better would be to set logic regarding `currentIndex` inside `cycleImages()` method and then just bind it using: `images.click(cycleImages);` — A. Wolff, Sep 30 '16 at 09:48

score 5 · Answer 1 · answered Sep 30 '16 at 09:42

this within cycleImages is the global object (I'm assuming you're not using strict mode) because of the way you've called it.

Probably best to wrap this once, remember it to a variable, and use that, since cycleImages will close over it:

$.each($('.portfolio-img-container'), function() {
    var $this = $(this);                                                 // ***

    var currentIndex = 0,
        images = $this.children('.portfolio-img'),                       // ***
        imageAmt = images.length;

    function cycleImages() {
        var image = $this.children('.portfolio-img').eq(currentIndex);   // ***
        images.hide();
        image.css('display', 'block');
    }

    images.click(function() {
        currentIndex += 1;
        if (currentIndex > imageAmt - 1) {
            currentIndex = 0;
        }

        cycleImages();
    });
});

Side note:

$.each($('.portfolio-img-container'), function() { /* ... */ });

can more simply and idiomatically be written:

$('.portfolio-img-container').each(function() { /* ... */ });

Side note 2:

In ES2015 and above (which you can use with transpiling today), you could use an arrow function, since arrow functions close over this just like other functions close over variables.

score 2 · Answer 2 · edited May 23 '17 at 10:27

2

You can't just refer to this inside of an inner function (see this answer for a lot more detailed explanations):

var self = this; // put alias of `this` outside function
function cycleImages() {
  // refer to this alias instead inside the function
  var image = $(self).children('.portfolio-img').eq(currentIndex);
  images.hide();
  image.css('display','block');
}

edited May 23 '17 at 10:27

Community

1
1

answered Sep 30 '16 at 09:42

Frxstrem

38,761
9
79
119

Jquery using $this in a for loop in an each loop

2 Answers2