Making a slideshow in JavaScript/jQuery

Question

I am new to JavaScript and jQuery, and I would like to know what can be improved of the code in this slideshow.

(function ($) {
let pointer = 0;

let slides = document.getElementsByClassName('mySlide');
for (let i = 0; i < slides.length; i++) {
    slides[i].style.left = 100 * i + "%";
    $('#dots').append('<span class="dot"></span>');
}

function changeSlide() {
    $('#slider').css('left', -100 * pointer + '%');
    changeDot();
    pointer++;
    if (pointer > (slides.length - 1)) pointer = 0;
    setTimeout(changeSlide, 3000);
}

$(document).ready(function () {
    changeSlide();
});

})(jQuery);

As can be observed in the code, I'm changing the left property of the <div> inside another <div> with overflow: hidden where the images are set to position: absolute.

Answer 1

Cache DOM lookups

When iterating over the elements with class name mySlide in the for loop, there is a look-up for the dots container (i.e. $('#dots')). Bearing in mind that the example only contains four slide elements, that could lead to excess DOM queries. It is good to store references like that and then utilize the reference:

const dotsContainer = $('#dots');

//later: (e.g. in for loop)
dotsContainer.append('<span class="dot"></span>');

Consistency

Why mix native JavaScript (e.g. getting the elements with class name mySlide: document.getElementsByClassName('mySlide');) and then use jQuery later on (e.g. getting elements with class name dot: $('.dot')?

That first block to loop over the elements with class name mySlide could be re-written in jQuery style:

const dotsContainer = $('#dots'); //cache the DOM lookup for later
let slides = $('.mySlide');//document.getElementsByClassName('mySlide');
slides.each(function(i, element) { //for (let i = 0; i < slides.length; i++) {
    $(element).css({left: 100 * i + "%"});//slides[i].style.left = 100 * i + "%";
    dotsContainer.append('<span class="dot"></span>');
});

Closure simplification

Some of the closures can be simplified - for instance:

$('.dot').each(function () {
  $(this).removeClass('is-active');
});

can be simplified to:

$('.dot').removeClass('is-active');

And the same is true for the DOM-ready callback:

$(document).ready(function () {
    changeSlide();
});

Can be simplified to:

$(document).ready(changeSlide);

jQuery DOM-ready syntax:

As of jQuery 3.0, only $(handler) is recommended; the other syntaxes still work but are deprecated.¹

So that ready callback registration can be simplified to:

$(changeSlide);

Many of these tips are explained in more detail in this article from a couple years avo. It does start off by making one question how much jquery is really needed these days- a valid question. For more on that topic, check out you might not need jquery.

(function ($) {

  let pointer = 0;
  //store instead of looking up each time
	const dotsContainer = $('#dots');
  let slides = $('.mySlide');//document.getElementsByClassName('mySlide');
  slides.each(function(i, element) { //for (let i = 0; i < slides.length; i++) {
    $(element).css({left: 100 * i + "%"});//slides[i].style.left = 100 * i + "%";
    dotsContainer.append('<span class="dot"></span>');
  });

  $('.dot').each(function (i) {
    $(this).on('click', function () {
      $('#slider').css('left', -100 * i + '%');
      pointer = i;
      changeDot();
    });
  });

  function changeDot() {
    $('.dot').removeClass('is-active');
    $('.dot').eq(pointer).addClass('is-active');
  }

  function changeSlide() {
    $('#slider').css('left', -100 * pointer + '%');
    changeDot();
    pointer++;
    if (pointer > (slides.length - 1)) pointer = 0;
    setTimeout(changeSlide, 3000);
  }

  $(changeSlide);

})(jQuery);

body {
  background: #28262b;
  color: white;
  font-family: 'Roboto', serif-sans;
}

#slider-wrap {
  width: 700px;
  height: 500px;
  background: #39373a;
  border: 1px solid #17151c;
  text-align: center;
  margin: 0 auto;
  overflow: hidden;
}

img {
  width: 100%;
  height: 100%;
}

#slider {
  height: 500px;
  left: 0;
  position: relative;
  transition: all 300ms ease-in-out;
}

.mySlide {
  position: absolute;
  text-align: center;
  z-index: 100;
  top: 0;
  left: 0;
  height: 500px;
}

.photo {
  position: relative;
  width: 700px;
  height: 100%;
}

.description {
  position: absolute;
  bottom: 0;
  left: 0;
  text-align: center;
  padding: 20px 0;
  width: 100%;
  height: 100px;
  background: rgba(0,0,0,.7);
}

#dots {
  display: flex;
  align-items: center;
  justify-content: center;
  margin: 20px;
}
.dot {
  display: block;
  border: 2px solid white;
  border-radius: 50%;
  margin: 0 10px;
  width: 15px;
  height: 15px;
}
.dot:hover {
  cursor: pointer;
  background: #616161;
}

.is-active {
  background: white;
}
.dot.is-active:hover {
  background: pink;
  border-color: pink;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>
<body>
  <div id="slider-wrap">
    <div id="slider">
      <div class="mySlide">
        <div class="photo">
          <img src='https://www.fillmurray.com/300/300'/>
        </div>
        <span class="description">test1</span>
      </div>
      <div class="mySlide">
        <div class="photo">
          <img src='https://www.fillmurray.com/301/300'/>
        </div>
        <span class="description">test2</span>
      </div>
      <div class="mySlide">
        <div class="photo">
          <img src='https://www.fillmurray.com/302/300'/>
        </div>
        <span class="description">test3</span>
      </div>
       <div class="mySlide">
        <div class="photo">
          <img src='https://www.fillmurray.com/303/300'/>
        </div>
        <span class="description">test4</span>
      </div>
    </div>
  </div>
  <div id="dots"></div>
</body>

---

¹_{http://api.jquery.com/ready/}