7

I'm building a custom slider on Javascript , and I want that every time the user clicks on a div of the slider, the slider should stop for X seconds.

My code is:

$(document).ready(function () {
    var ciclo;
    var index_slide = 1;

    function startSlidercicle() {
        ciclo = setInterval( function() {
            // Slider code goes here
        }, 3000); 
    }

    //Here I start the slider animation
    startSlidercicle();

    //When the user clicks on a div called 'slide', stop the cycle and start again the animation cycle
    $('.slide').on('click', function() {
        clearInterval(ciclo);
        setTimeout(startSlidercicle(), 3000);
    });
});

But the problem is that everytime I click and stop the slider, the cycle starts faster and faster. How can I fix it?

cfs
  • 10,610
  • 3
  • 30
  • 43
  • 2
    You should be using `setTimeout(startSlidercicle, 3000)` (no parentheses after `startSlidercicle`). But that shouldn't be causing what you describe... – bfavaretto Aug 22 '13 at 22:09
  • I can only reproduce that if I click the .slide div multiple times (see http://jsfiddle.net/XdMHz/). You should add a guard against that, because it creates "leftover" timers you can't reference/clear. – bfavaretto Aug 22 '13 at 22:14
  • I've read some questions about the same topic, but I don't have clear how to prevent or close those multiple timers –  Aug 22 '13 at 22:24
  • 1
    One way that you can prevent the multiple timers is basically the same way you currently clear your interval: create a new variable `timerId` (or whatever you want to call it) then in your click handler add `clearTimeout(timerId)` and then `timerId = setTimeout(...`. – nnnnnn Aug 22 '13 at 22:38
  • There is no other setTimeout or setInterval in the missing slider code? Can't seem to recreate the issue with what you've got here. – Jesse Kernaghan Aug 22 '13 at 22:57

2 Answers2

6

Instead of:

clearInterval(ciclo);
setTimeout(startSlidercicle(), 3000);

or:

clearInterval(ciclo);
setTimeout(startSlidercicle, 3000);

I changed the code to be:

clearInterval(ciclo);
startSlidercicle();

And now the slider just works fine. I think that, in the first two proposals, every time I click on the div, a new function is created, "overlapping" over the existing cycle and, thus, it looks like the slider speeds up, but its just one cycle starting over another.

  • Already tested with Firefox 23, IE 9, Opera 12+ and Chrome 29 on Windows 7 and still works fine. –  Aug 23 '13 at 09:18
  • 1
    My conclusion is that `clearInterval(ciclo)` ends the `setInterval` cycle inside `startSlidercicle()`, so, there is no remaining code inside the function and exits, then jumps to the next line where I call a new `startSlidercicle()`, and if I click many times, I'm just finishing and starting new `setInterval` methods –  Aug 23 '13 at 09:25
2

You need to change this:

clearInterval(ciclo);
setTimeout(startSlidercicle(), 3000);

to this:

clearInterval(ciclo);
setTimeout(startSlidercicle, 3000);

In your existing code, you are calling startSlidercirle immediately and it is not waiting until the setTimeout() fires because you have the () after the function name. That means to execute it immediately and pass the result of executing that to setTimeout(). You want to just pass the function reference to setTimeout() which is done by just having the name of the function with no () after it. This is a common mistake.

jfriend00
  • 683,504
  • 96
  • 985
  • 979