Slider module and extending it further

Question

I created this module, and I'm wondering if I did a good job. How would an experienced JavaScript developer improve this simple slider module further? Like, let's say that you want to use this module on 3 sliders on the same page.

Here is a jsFiddle.

Module

var sliderTest = (function(){
    /*:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
    ✚   private variables
    :::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::*/    
    var expoElement = document.querySelector(".test")
    var expoCanvas = expoElement.querySelector(".canvas")
    var expoSlides = expoCanvas.querySelectorAll(".slide")
    var controlPrev = expoElement.querySelector(".control.prev")
    var controlNext = expoElement.querySelector(".control.next")
    /*:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
    ✚   private functions
    :::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::*/   
    function whereIsActive(){
       var y = {}
       for(var i = 0; i < expoSlides.length; i++){
          if(expoSlides[i].classList.contains("active")){
              y.current = i;
              if(expoSlides[i+1]){
                  y.next = i+1;
              } else {
                  y.next = 0;
              }

              if(expoSlides[i-1]){
                  y.prev = i-1;
              } else {
                  y.prev = expoSlides.length-1;
              }                   
          }       
      }   
      return y
   }

   function removeActiveClasses(){
       for(var i = 0; i < expoSlides.length; i++){
           expoSlides[i].classList.remove("active")
       }       
   }   
    /*:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
    ✚   public methodes
    :::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::*/

        var publicMethod = {
            nextSlide:function(ev){
               ev.preventDefault();
               var nextIndex = whereIsActive().next;
               removeActiveClasses();
               expoSlides[nextIndex].classList.add("active");          
       },

       prevSlide:function(ev){
           ev.preventDefault();
           var prevIndex = whereIsActive().prev;

           removeActiveClasses();
           expoSlides[prevIndex].classList.add("active")
      },
      sendInTheClone:function(){alert(whereIsActive().current)}
  }
    /*:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
    ✚   event listners
    :::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::*/

            controlNext.addEventListener("click", publicMethod.nextSlide)
            controlPrev.addEventListener("click", publicMethod.prevSlide)    
    /*:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
    ✚   return
    :::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::*/

        return publicMethod;
})();

HTML

<div class="test">

    <div class="canvas">
        <div class="slide ">1</div>
        <div class="slide ">2</div>     
        <div class="slide active">3</div>
        <div class="slide ">4</div>
    </div>

    <a class="control prev" href="" data-control="prev">prev</a> <a class="control next" href="" data-control="next">next</a>

</div>

Answer 1

document.querySelector

While it is a great function to use, you are using in the wrong situations.

For every time you use it, you are getting an array of elements with a class that you passed in to the function. Why not just use document.getElementsByClassName?

Sure, it takes a little bit longer to type but it's faster.

This about it: the querySelector function has to read the argument passed in and, based on it's syntax ('.' for classes, '#' for ids, and a few more), scan the document in a way unique to each type of identification (id, class, etc).

Wouldn't it be easier for the function to know that it was going to look for one class and for one class only?

Semicolons

They aren't entirely necessary, and the interpreter probably adds them in for you, but your code looks like JavaScript when you put them in (at least in my opinion).

HTML indentation

Your divs are perfectly indented, but when you go down to your as, you start to try and clump them on to one line.

Originally, when I was looking at the a tag(s), I thought it was one really long a tag.

JavaScript indentation

For the most part, it's really good. But there is one section that is bothering me: your public methods section.

I actually had to post this in to JSFiddle and click "Tidy up" to understand what was going on there.

I believe the problem is the closing bracket of nextSlide; it is too far back, and it looks like it is closing publicMethod.

Built in output

In the sendInTheClone method of publicMethod, it uses the function alert.

Not everyone using this module may want an alert popping up on their screen; maybe they want it written to the DOM, or maybe they use custom pop-up boxes.

(Excuse this section ("Built in output") if I misunderstood your sendInTheClone function)

Good things:

1. Contrary to what people have been commenting, I think that the big comments going across your code to split it up is a good idea. In my opinion, it helps with legibility.

2. The use of publicMethod. It's a pretty good idea; I like it.