I have to implement an image-Carousel.
Because I'll reuse the snippet within other projects which might contain all kinds of external libraries I used an immediately invoked function expression.
Could my code be improved?
All Hints appreciated.
(function (imgPaths, imgTag, prevButton, nextButton) {
var length = imgPaths.length;
var i = 0;
function setImageSrc () {
// Using alt-attribute for simplicity.
// In production the src-attribute is used.
imgTag.setAttribute('alt', imgPaths[i]);
}
setImageSrc();
prevButton.addEventListener('click', function() {
i = ((i - 1) + length) % length;
setImageSrc();
});
nextButton.addEventListener('click', function() {
i = (i + 1) % length;
setImageSrc();
});
})( [ 'imgs/first', 'imgs/second', 'imgs/third',
'imgs/fourth', 'imgs/fifth' ],
document.querySelector('.display img'),
document.querySelector('.prev'),
document.querySelector('.next') );<div class="wrap">
<div class="display">
<img src="#" alt="" />
</div>
<button class="prev"> << </button>
<button class="next"> >> </button>
</div>1 Answer 1
A carousel is an enhancement, a different way of displaying the same data. Without the carousel effect, a carousel is just a list of images. If JS was turned off or for some reason the script blew up, one would still expect to see the images, albeit in a broken layout.
In this case, I would format it to become a list.
<div class="my-carousel">
<ul class="my-carousel__deck">
<li class="my-carousel__slide"><img src="img1.jpg" /></li>
<li class="my-carousel__slide"><img src="img2.jpg" /></li>
<li class="my-carousel__slide"><img src="img3.jpg" /></li>
</ul>
<div class="my-carousel__controls">
<button type="button" class="my-carousel__control my-carousel__control--prev"> << </button>
<button type="button" class="my-carousel__control my-carousel__control--next"> >> </button>
</div>
</div>
In addition, I just popped in a few classes for styling. You can, by default, hide the buttons and show all images. When JS runs, it shows the controls and hides all but one image.
.my-carousel{...} # Widget style
.my-carousel__deck{...} # Deck
.my-carousel__slide{...} # Default slide style
.my-carousel__slide--hidden{...} # Styles when a slide is hidden
.my-carousel__controls{...} # Default controls styles
.my-carousel__controls--active{...} # Controls when active
Your carousel looks simple, just showing one image, no sliding effect etc. We can improve that even further. Since we moved out the image sources away from the JS, we can focus on just the effect.
;(function(images, controls, prevButton, nextButton){
const imageHiddenClass = 'my-carousel__slide--hidden';
const controlsActiveClass = 'my-carousel__controls--active';
let currentSlide = 0;
function setActiveSlide(index){
// Hide all images
images.forEach(image => image.classList.add(imageHiddenClass));
// But then show the one we want shown
images[index].classList.remove(imageHiddenClass);
}
prevButton.addEventListener('click', function() {
currentSlide = (currentSlide - 1) % images.length;
setActiveSlide(currentSlide);
});
nextButton.addEventListener('click', function() {
currentSlide = (currentSlide + 1) % images.length;
setActiveSlide(currentSlide);
});
// Show the controls that are hidden by default
controls.classList.add(controlsActiveClass);
// Set the first slide as active
setActiveSlide(currentSlide);
})(
Array.prototype.slice.call(document.querySelectorAll('.my-carousel__slide')),
document.querySelector('.carousel__controls'),
document.querySelector('.carousel__control--prev'),
document.querySelector('.carousel__control--next')
);
In the above, the script is a bit longer but it takes an alternate approach. The major difference is that instead of changing srcs of the image, since the images are all in the markup, what the script does now is toggle the my-carousel__slide--hidden class, adding it to hide images not in focus.