0

I'm learning JavaScript. And now I want to make a typical image slider that when the page loads automatically starts, and if one of 2 buttons (forward and backward) are clicked it stops the automatic slider and just goes to where we click the buttons.

But the code that I'm testing doesn't have the buttons working. I have even used jQuery attempting to make it work, but no success. I think that I have made a mess, but this is the code:

$(function() {
  var slideIndex = 0;
  var i;
  var s = document.getElementsByClassName("slide");
  var stopslider = false;
  slider();
  
  $(".change[n]").click(function(){
    var slideIndex = 1;
    var stopslider = true;
    $( "slide1" ).removeClass( "slide" ).addClass( "1slide" );
    $( "slide2" ).removeClass( "slide" ).addClass( "1slide" );
    $( "slide3" ).removeClass( "slide" ).addClass( "1slide" );
    $( "slide4" ).removeClass( "slide" ).addClass( "1slide" );
    var s = document.getElementsByClassName("1slide");
    slideslide1Index += n;
    if (n > s.length) {
      slideIndex = 1
    } 
    if (n < 1) {
      slideIndex = s.length
    } ;
    for (i = 0; i < s.length; i++) {
      s[i].style.display = "none"; 
    }
    s[slideIndex-1].style.display = "block"; 
  });
  
  function slider() {
    for (i = 0; i < s.length; i++) {
      s[i].style.display = "none"; 
    }
    slideIndex++;
    if (slideIndex > s.length) {
      slideIndex = 1
    }
    s[slideIndex-1].style.display = "block";
    setTimeout(slider, 4000);
    if (stopslider= true) {
      clearTimeout(slider);
    } 
  }
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<a href="imagem1"><img id="slide1" class="slide" src="imagens/imagemcentral1.jpg"></a>
<a href="imagem2"><img id="slide2" class="slide" src="imagens/imagemcentral2.jpg"></a>
<a href="imagem3"><img id="slide3" class="slide" src="imagens/imagemcentral3.jpg"></a>
<a href="imagem4"><img id="slide4" class="slide" src="imagens/imagemcentral4.jpg"></a>

<button id="previous" class="change" n="-1">previous</button>
<button id="latter" class="change" n="+1">latter</button>
myfunkyside
  • 3,890
  • 1
  • 17
  • 32
Adato
  • 423
  • 4
  • 20

2 Answers2

1

(For an answer using the OP's original code, see my second answer.)

Okay.. so I may have gone a little overboard:)

The thing is, when I just started out I did exactly the same as you did, with a bunch of variables to store the progress and current position etc.

But really, all that isn't necessary, and JavaScript is quite unique in that way because of the 'physical' DOM structure that is already present on the page and can be referenced instead of using a dedicated variable. They can in many cases function as your 'variables', and in many cases are preferred over (global) variables.

ANYWAY, so I pretty much changed your whole code, also renamed probably everything in the process, stripped out unnecessary elements like the <a> tags. You might have a need for them, but I wanted to make the code as clean as possible so it's easier to understand:

Read the comments in the code to understand what's going on.
Ignore the CSS, it's only for styling. All the functionality is in the JS.

PURE JS:

window.onload = function() {
  function changeSlide(d, id) {
   var slides = document.getElementById(id).children[0];
    for (var i=0,count=slides.children.length; i<count; ++i) {
      if (slides.children[i].className.indexOf("active") != -1) {var slide=slides.children[i]; break;} //store the current slide
    }
    if (d=="next") {
      if (slides.lastElementChild.className.indexOf("active") != -1) {
        slides.firstElementChild.className += " active"; //loop around and activate the first slide
      } else {slide.nextElementSibling.className += " active";} //activate the next slide
    } else { //prev
      if (slides.firstElementChild.className.indexOf("active") != -1) {
        slides.lastElementChild.className += " active"; //loop around and activate the last slide
      } else {slide.previousElementSibling.className += " active";} //activate the prev slide
    }
    slide.className = slide.className.replace(" active",""); //deactivate the current slide
  }
  
//AUTOMATIC LOOP------------------------------
 function timer(id){setTimeout(function(){if (document.getElementById(id).className.indexOf("auto") != -1) {changeSlide("next",id); timer(id);}},4000);}
  for (var i=0,s=document.getElementsByClassName("slider"),count=s.length; i<count; ++i) {timer(s[i].id);} //create a timer-loop for every slider
  
//EVENT-HANDLERS------------------------------
  for (var i=0,s=document.getElementsByClassName("slider"),count=s.length; i<count; ++i) {
    s[i].onmousedown = function(){return false;}; //otherwise sometimes the space next to the slider gets selected on navigation
    //click-handlers for prev/next buttons
    for (var j=0,c=s[i].children.length; j<c; ++j) {
      if (s[i].children[j].className.indexOf("nav") != -1) {
        s[i].children[j].onclick = function() {
         var s = document.getElementById(this.parentNode.id);
          if (s.className.indexOf("auto") != -1) {s.className = s.className.replace(" auto","");} //stop the automatic loop
          changeSlide(this.className.split(" ")[1],this.parentNode.id); //prev/next slide ('this.className.split(" ")[1]' returns the second classname)
        };
      }
    }
  }
};
.slider {display:inline-block; position:relative; width:240px; height:135px; background:#000;}
.slider .slides {width:100%; height:100%;}
.slider .slides .slide {float:left; width:0; height:100%; background:center center/contain no-repeat; transition:width 0.5s ease 0s;}
.slider .slides .slide.active {width:100%;}

.slider .nav {position:absolute; top:0; width:15%; min-width:44px; height:100%; background:#888; opacity:0; cursor:pointer;}
.slider:hover .nav {opacity:0.3;}
.slider .nav.prev {left:0;}
.slider .nav.next {right:0;}
.slider .nav .btn {position:absolute; top:0; bottom:0; width:34px; height:34px; margin:auto 0; box-sizing:border-box; border:2px solid #ddd; border-radius:999px; background:#555; opacity:0.7; visibility:hidden;}
.slider .nav.prev .btn {left:5px;}
.slider .nav.next .btn {right:5px;}
.slider:hover .nav .btn {visibility:visible;}
.slider:hover .nav .btn:hover {border-color:#fff; opacity:1;}

.slider .nav .btn .arrow {position:absolute; top:0; bottom:0; width:0; height:0; margin:auto 0; border:10px solid transparent;}
.slider .nav.prev .btn .arrow {right:calc(50% - 3px); border-right-color:#ddd;}
.slider .nav.next .btn .arrow {left:calc(50% - 3px); border-left-color:#ddd;}
.slider .nav.prev .btn:hover .arrow {border-right-color:#fff;}
.slider .nav.next .btn:hover .arrow {border-left-color:#fff;}
<div class="slider auto" id="slider1">
  <div class="slides">
    <div class="slide active" style="background-image:url('https://placeimg.com/320/180/arch');"></div>
    <div class="slide" style="background-image:url('https://placeimg.com/640/250/animals');"></div>
    <div class="slide" style="background-image:url('https://placeimg.com/720/450/tech/sepia');"></div>
    <div class="slide" style="background-image:url('https://placeimg.com/480/480/nature');"></div>
  </div>
  <div class="nav prev"><div class="btn"><div class="arrow"></div></div></div>
  <div class="nav next"><div class="btn"><div class="arrow"></div></div></div>
</div>

<div class="slider auto" id="slider2">
  <div class="slides">
    <div class="slide active" style="background-image:url('https://placeimg.com/320/480/nature');"></div>
    <div class="slide" style="background-image:url('https://placeimg.com/640/480/people/sepia');"></div>
    <div class="slide" style="background-image:url('https://placeimg.com/640/180/tech');"></div>
    <div class="slide" style="background-image:url('https://placeimg.com/640/360/arch/sepia');"></div>
  </div>
  <div class="nav prev"><div class="btn"><div class="arrow"></div></div></div>
  <div class="nav next"><div class="btn"><div class="arrow"></div></div></div>
</div>
jsfiddle: https://jsfiddle.net/9pommada/9/
  • I made it so that you can add multiple sliders to your page and they will all work independently from each other. I did that by removing (almost) all ids and working only with classes.
    The only ids that are left are on the .sliders, I couldn't get it to work in pure JS without them.
    If you want no ids at all, check out the jQuery version below.
  • The for-loop may look a little weird to you. Basically, instead of doing the more familiar version for (var i=0; i<slides.children.length; i++) {, I declare an extra variable in the first part of the for-loop, so that the length isn't calculated on every iteration. The result is the following for (var i=0,count=slides.children.length; i<count; ++i) {. Using ++i has a subtle difference over i++, but used like this there is no practical difference.
  • I said all the functionality is in the JS, that's not entirely true. There are some things in CSS that have a great impact on the way the slider behaves. Although it is mostly fancy transition, no too long ago you would have had to code that in JavaScript too, so it is definitely part of the functionality I think.
    One is the transition:width 0.5s ease 0s;. Together with float:left; it makes the slides really slide, instead of just static replacement.
    The other is the background:center center/contain no-repeat;. This makes that the images are always nicely centered and scaled to fit the container. If the image is too wide it gets vertically centered, if it's too tall it gets horizontally centered. That's all CSS.
  • Another important point you may overlook at first, is that for the point above (about the image centering and scaling) to work, you need to replace your <img/> tags by <div>s, and on those <div>s set the images as background-images.
    I did it in HTML here, this is usually considered bad practice, but if you would move these to CSS, you would have to give every slide either a unique class or an id for that alone.

JQUERY:

$(function() {
  function changeSlide(d, slides) {
    var slide = $(slides).children(".slide.active")[0]; //store the current slide
    if (d=="next") {
      if ($(slides).children(".slide").last().hasClass("active")) {
        $(slides).children(".slide").first().addClass("active"); //loop around and activate the first slide
      } else {$(slide).next().addClass("active");} //activate the next slide
    } else { //prev
      if ($(slides).children(".slide").first().hasClass("active")) {
        $(slides).children(".slide").last().addClass("active"); //loop around and activate the last slide
      } else {$(slide).prev().addClass("active");} //activate the prev slide
    }
    $(slide).removeClass("active"); //deactivate the current slide
  }
  
//AUTOMATIC LOOP------------------------------
 function timer(slides){setTimeout(function(){if ($(slides).parent().hasClass("auto")) {changeSlide("next",slides); timer(slides);}},4000);}
  $(".slider").each(function(){timer($(this).children(".slides")[0]);}); //create a timer-loop for every slider
  
//EVENT-HANDLERS------------------------------
  $(".slider").on("mousedown",function(){return false;}); //otherwise sometimes the space next to the slider gets selected on navigation
  $(".slider .nav").click(function(){
    if ($(this).closest(".slider").hasClass("auto")) {$(this).closest(".slider").removeClass("auto");}; //stop the automatic loop
    changeSlide(this.className.split(" ")[1],$(this).siblings(".slides")[0]); //prev/next slide
  });
});
.slider {display:inline-block; position:relative; width:240px; height:135px; background:#000;}
.slider .slides {width:100%; height:100%;}
.slider .slides .slide {float:left; width:0; height:100%; background:center center/contain no-repeat; transition:width 0.5s ease 0s;}
.slider .slides .slide.active {width:100%;}

.slider .nav {position:absolute; top:0; width:15%; min-width:44px; height:100%; background:#ddd; opacity:0; cursor:pointer;}
.slider:hover .nav {opacity:0.3;}
.slider .nav.prev {left:0;}
.slider .nav.next {right:0;}
.slider .nav .btn {position:absolute; top:0; bottom:0; width:34px; height:34px; margin:auto 0; box-sizing:border-box; border:2px solid #ddd; border-radius:999px; background:#555; opacity:0.7; visibility:hidden;}
.slider .nav.prev .btn {left:5px;}
.slider .nav.next .btn {right:5px;}
.slider:hover .nav .btn {visibility:visible;}
.slider:hover .nav:hover .btn {opacity:1;}

.slider .nav .btn .arrow {position:absolute; top:0; bottom:0; width:0; height:0; margin:auto 0; border:10px solid transparent;}
.slider .nav.prev .btn .arrow {right:calc(50% - 3px); border-right-color:#ddd;}
.slider .nav.next .btn .arrow {left:calc(50% - 3px); border-left-color:#ddd;}
.slider .nav.prev:hover .btn .arrow {border-right-color:#fff;}
.slider .nav.next:hover .btn .arrow {border-left-color:#fff;}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<div class="slider auto">
  <div class="slides">
    <div class="slide active" style="background-image:url('https://placeimg.com/320/180/arch');"></div>
    <div class="slide" style="background-image:url('https://placeimg.com/640/250/animals');"></div>
    <div class="slide" style="background-image:url('https://placeimg.com/720/450/tech/sepia');"></div>
    <div class="slide" style="background-image:url('https://placeimg.com/480/480/nature');"></div>
  </div>
  <div class="nav prev"><div class="btn"><div class="arrow"></div></div></div>
  <div class="nav next"><div class="btn"><div class="arrow"></div></div></div>
</div>

<div class="slider auto">
  <div class="slides">
    <div class="slide active" style="background-image:url('https://placeimg.com/320/480/nature');"></div>
    <div class="slide" style="background-image:url('https://placeimg.com/640/480/people/sepia');"></div>
    <div class="slide" style="background-image:url('https://placeimg.com/640/180/tech');"></div>
    <div class="slide" style="background-image:url('https://placeimg.com/640/360/arch/sepia');"></div>
  </div>
  <div class="nav prev"><div class="btn"><div class="arrow"></div></div></div>
  <div class="nav next"><div class="btn"><div class="arrow"></div></div></div>
</div>
jsfiddle: https://jsfiddle.net/nwbx5g02/11/
  • Because we can work with jQuery objects, we can eliminate all ids, which makes it more flexible to implement on other sites.

Which one you choose probably comes mostly down to preference. Personally I prefer jQuery, 'cause it just makes life so much easier and I'm already using it for everything else anyway. But when I just started out I was really suspicious of it, and coding in pure JS certainly helped me get a good understanding of what I was doing. Anyways, hope I could help, if you have any questions about the code just leave 'em in the comments and I'll try to answer them.

myfunkyside
  • 3,890
  • 1
  • 17
  • 32
1

I had another look at my first answer and realized that may entirely not be what you're looking for.
So here I'll try to stay as true to your original code as possible.

See the comments below the code snippet for explanation.

window.onload = function() {
  var slideIndex = 0;
  var stopslider = false;
  slider();
  
  function change(n) {
    stopslider = true;
    var s = document.getElementsByClassName("slide");
    slideIndex += n;
    if (slideIndex > s.length) {
      slideIndex = 1;
    } else if (slideIndex < 1) {
      slideIndex = s.length;
    }
    for (var i=0; i<s.length; i++) {
      s[i].style.display = "none";
    }
    s[slideIndex-1].style.display = "block";
  }
  
  function slider() {
    if (stopslider == false) {
      var s = document.getElementsByClassName("slide");
      for (var i=0; i<s.length; i++) {
        s[i].style.display = "none";
      }
      slideIndex++;
      if (slideIndex > s.length) {
        slideIndex = 1;
      }
      s[slideIndex-1].style.display = "block";
      setTimeout(slider, 4000);
    }
  }
  
//CLICK-HANDLERS-------------------------
  document.getElementById("previous").onclick = function(){change(-1);};
  document.getElementById("latter").onclick = function(){change(+1);};
};
<a class="slide" href="imagem1"><img src="https://placeimg.com/200/150/animals"></a>
<a class="slide" href="imagem2"><img src="https://placeimg.com/200/150/arch"></a>
<a class="slide" href="imagem3"><img src="https://placeimg.com/200/150/people"></a>
<a class="slide" href="imagem4"><img src="https://placeimg.com/200/150/tech"></a>

<button id="previous" class="change">previous</button>
<button id="latter" class="change">latter</button>
jsfiddle: https://jsfiddle.net/Lkeaejkn/
  • Since you stated that originally you were coding in pure JS, I reverted all the jQuery back.
    So $(function() { becomes window.onload = function() {.
  • I removed global variable var s = document.getElementsByClassName("slide");. You could use it, but since you also already declare it inside one function, and you generally want to keep your global variables to a minimum (link1, link2), I chose to remove the global one.
    I also removed var i;, same reason. It's better to declare them inside the for-loop.
  • <button class="change" n="+1"> and ".change[n]" are not something I am familiar with, in either HTML, pure JS or jQuery. I don't think it's possible (someone correct me if I'm wrong).
    You can't just make up your own HTML attributes. Well, you kinda can, using data-* attribute. You could use them, and if you want to, check out the second code snippet below.
    In this snippet, I chose another way using onclick-handlers inside your JS.
    (It is considered better practice to keep all your HTML and JS separated, so as far as that goes this option might be better. But for this option you do need to give the buttons ids - which you already had. Using data-attributes, those ids are not necessary.)
    In your JS, change $(".change[n]").click(function(){ to a regular function: function change(n) {, where n holds a value of either +1 or -1 that determines the direction of the slider.
    At the bottom of your code I added the two click-handlers, which both call function change(n), and send along parameter n, either with value -1 or +1.
  • Inside function change(n), I removed var slideIndex = 1; because this would override/interfere with the global variable var slideIndex you had already declared at the start of the script.
    Unfortunately, this variable has to stay global, otherwise you would reset the value every time you call the function change(n), and the slide would never change. This is one of the reasons I created my first answer, to avoid global variables. If you're interested, check that one out.
    1. I removed var from var stopslider = true;. The var creates a new variable that is only visible inside the function (so not global), but in this case we do need to reference the global var stopslider, because that is the variable that is checked inside the function slider() to determine if the auto-slider should be stopped.
    2. I removed all the $( "slide1" ).removeClass( "slide" ).addClass( "1slide" );. I don't know what you were trying there, but it's not necessary. (Also, jQuery selectors for id need a # in front of the id, so: $("#slide1")).
      I also went ahead and removed all the ids from the slides in HTML, because they are not necessary and it can save you a lot of pain when you want to insert more slides later on.
    3. I also removed the 1 from var s = document.getElementsByClassName("1slide");, don't know what you were doing there either.
      I also moved class="slide" from the <img/>s to the <a>s, because otherwise the images would be hidden, but the <a>s of all images would still be visible, which is not what your want.
      Your actually don't need the <a>s for the images or the slider itself, but you may have a specific purpose for them, so I'll leave them in.
    4. I changed slideslide1Index += n; to slideIndex += n;, so it references the global variable.
  • In all your if-clauses you check for n: if (n > s.length) {, but you should check for slideIndex. Remember, n only ever has a value of either -1 or +1. Global slideIndex holds the value of the current slide.
  • I combined your two if-clauses into an if-else: } else if (slideIndex < 1) {. Because only ever one of the two clauses can be true, it inefficient to run them both each time change(n) is called. And this also directly shows (to the programmer who reads the code) that these two clauses belong together, are part of one bigger check.
  • You had a typo: } ;, I removed it.
  • As I said earlier it's better to declare i inside the for-loop, so that becomes: for (var i=0; i<s.length; i++) { (same in function slider()).
  • Inside function slider(), because I removed the global var s = document.getElementsByClassName("slide");, I had to add that line at the top of this function, just like in change(n).
  • In the if-clause condition I changed if (stopslider= true) { to if (stopslider == true) {.
    Because = is already used to assign value, in programming we use == (double =) to check if two values are equal to each other.
  • In order to be able to clear a timeout, you unfortunately first need to store a global reference to that timer when you set it, so: timer = setTimeout(slider,4000);, where timer is a global var that you set at the start of your script.
    Fortunately, we don't actually need to clear the timer. Because setTimeout() only runs once and after that just stops, we just need to not set it again.
    In order to do that, just put all the code from function slider() inside the if-clause that you used around clearTimeout(), only changing true to false: if (stopslider == false) {.
    (If you put it only around setTimeout(), the slider will still change automatically one more time after you clicked a button for the first time.)

I changed the src of your images, just so we can actually see them change.

Code snippet using data- attribute:

This snippet is based on the previous snippet, with a few changes.

window.onload = function() {
  var slideIndex = 0;
  var stopslider = false;
  slider();
  
  function slider() {
    if (stopslider == false) {
      var s = document.getElementsByClassName("slide");
      for (var i=0; i<s.length; i++) {
        s[i].style.display = "none";
      }
      slideIndex++;
      if (slideIndex > s.length) {
        slideIndex = 1;
      }
      s[slideIndex-1].style.display = "block";
      setTimeout(slider, 4000);
    }
  }
  
//CLICK-HANDLERS-------------------------
  (function() {
    var c = document.getElementsByClassName("change");
    for (var i=0; i<c.length; i++) {
      c[i].onclick = function() {
        stopslider = true;
        var n = parseInt(this.getAttribute("data-n"));
        var s = document.getElementsByClassName("slide");
        slideIndex += n;
        if (slideIndex > s.length) {
          slideIndex = 1;
        } else if (slideIndex < 1) {
          slideIndex = s.length;
        }
        for (var i=0; i<s.length; i++) {
          s[i].style.display = "none";
        }
        s[slideIndex-1].style.display = "block";
      };
    }
  })();
};
<a class="slide" href="imagem1"><img src="https://placeimg.com/200/150/animals"></a>
<a class="slide" href="imagem2"><img src="https://placeimg.com/200/150/arch"></a>
<a class="slide" href="imagem3"><img src="https://placeimg.com/200/150/people"></a>
<a class="slide" href="imagem4"><img src="https://placeimg.com/200/150/tech"></a>

<button class="change" data-n="-1">previous</button>
<button class="change" data-n="+1">latter</button>
jsfiddle: https://jsfiddle.net/Lkeaejkn/2/
  • in HTML, I added data-n="-1" and data-n="+1" to the respective buttons, and removed the ids from the buttons as they are no longer necessary. Read about the data-* attribute
  • In JS, I removed the old id-based click-handlers, and instead changed the whole function change(n) to an anonymous click-handler for the buttons.
  • I wrapped click-handlers in an self-executing anonymous function to prevent any more global variables: (function() { ... })();.
  • Then I use the same principle as you use for the slides, but now to iterate over the buttons.
    And then for each button, I bind the actual click-handler: c[i].onclick = function() {.
  • Inside the click-handler is use var n = parseInt(this.getAttribute("data-n")); to get the n-value from the respective button.
    Because the data from the data-n attribute is in text format, parseInt() is needed to convert the text value to an integer.
myfunkyside
  • 3,890
  • 1
  • 17
  • 32
  • Wow. Thanks a lot for your effort, myfunkside. Thats a lot of info right here. Now, my problem is solved. Thanks !!! – Adato Jul 02 '17 at 16:31