Mila Samdub
Mila Samdub

Reputation: 11

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

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!

Upvotes: 1

Views: 123

Answers (2)

Freyja
Freyja

Reputation: 40914

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');
}

Upvotes: 2

T.J. Crowder
T.J. Crowder

Reputation: 1075527

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.

Upvotes: 5

Related Questions