Reputation: 11
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
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
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