7
votes

I have something a little bit more complicated. I have five li's and a class "active".

Only one of li's has class "active". That one will have the color red.

When i click the "Next" link the class "active" will be added to the next li.

When i click the "Prev" link the class "active" will be added to the previous li.

When i click on a li, the class "active" will be added only to the clicked li.

Those thinks work, but there is a problem with the loop() function.

I need the class "active" to move automatically from one li to the next. It works, but things are going crazy when i click next-link, prev-link or a li.

All those 3 elements: loop, next-prev links and click on a li should work perfect together.

If i click the next-link, and the class "active" is transfered from the second li to the third li,i need the "for" statement to continue from the third li to move the class and continue the cycle.

I could try to take that "i" in the for from nr-1 (the li what was clicked) not from 0, for(var i=nr-1; i < functions.length; i++), but after the first cycle of class "active" it should start from 0 again, but it starts from the nr-1,which is normal.

The loop works, the problem appears when i click a li or the prev-next link.

$(document).ready(function () {

  var f1 = function () {
    $(".1").addClass("active").siblings("li").removeClass("active")
  };
  f2 = function () {
    $(".2").addClass("active").siblings("li").removeClass("active")
  };
  f3 = function () {
    $(".3").addClass("active").siblings("li").removeClass("active")
  };
  f4 = function () {
    $(".4").addClass("active").siblings("li").removeClass("active")
  };
  f5 = function () {
    $(".5").addClass("active").siblings("li").removeClass("active")
  };

  var functions = [f1, f2, f3, f4, f5];
  var y = functions.length;

  var loop = function () {
    for (var i = 0; i < functions.length; i++) {
      (function (i) {
        setTimeout(function () {
          functions[i]();
        }, 1000 * i);
      })(i);
    }
    setTimeout(loop, (1000 * (functions.length)));
  }
  
  loop();

  $("ol li ").click(function () {
    var nr = $(this).data("number");
    functions[nr - 1]();
  });


  $(".flex-next").click(function () {
    var nr = $("ol").find("li.active").data("number");

    if (nr == y) {
      functions[0]();
    } else {
      functions[nr]();
    }

  });


  $(".flex-prev").click(function () {
    var nr = $("ol").find("li.active").data("number");
    //y = functions.length;
    if (nr == 1) {
      functions[y - 1]();
    } else {
      functions[nr - 2]();
    }

  });

});
@import url(http://reset5.googlecode.com/hg/reset.min.css);
ol li {
  width:50px;
  height:50px;
  background-color:blue;
  float:left;
  margin:10px;
}
ul li a {
  text-decoration: none;
  width:100px;
}
.active {
  background-color:red;
}
<script src="http://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<ol>
  <li data-number="1" class="1 active ">
    <a href="#"></a>	
  </li>
  <li data-number="2" class="2">
    <a href="#"></a> 
  </li>
  <li data-number="3" class="3">
    <a href="#"></a> 
  </li>
  <li data-number="4" class="4">
    <a href="#"></a> 
  </li>
  <li data-number="5" class="5">
    <a href="#"></a> 
  </li>
</ol>
<ul class="flex-direction-nav">
  <li>
    <a class="flex-prev" href="javascript:void(0);">Previous</a>
  </li>
  <li>
    <a class="flex-next" href="javascript:void(0);">Next</a>
  </li>
</ul>

jsfiddle here

5
one thing i noticed right away is that var f1 is defined as a variable inside document.ready, but you seperate all the f2,f3,f4,f5 with a ";" instead of a "," , so f2,f3,f4,f5 gets defined as global variables! Just a heads up, either define multiple variables with the shorthand like "var f1, f2, f3;" or individually like "var f1; var f2; var f3;" =) - NachoDawg
So it sounds like you need to stop the timeout when something is clicked and restart it from that point. See developer.mozilla.org/en-US/docs/Web/API/WindowTimers/…. Also, I think that loop() method looks pretty fishy. Looks like you're setting 5 different setTimeouts in that for loop. I'd also think setInterval() might be more appropriate here. - Jeff

5 Answers

1
votes

Modified your loop() a bit.

$(document).ready(function () {

  var f1 = function () {
    $(".1").addClass("active").siblings("li").removeClass("active")
  };
  f2 = function () {
    $(".2").addClass("active").siblings("li").removeClass("active")
  };
  f3 = function () {
    $(".3").addClass("active").siblings("li").removeClass("active")
  };
  f4 = function () {
    $(".4").addClass("active").siblings("li").removeClass("active")
  };
  f5 = function () {
    $(".5").addClass("active").siblings("li").removeClass("active")
  };

  var functions = [f1, f2, f3, f4, f5];
  var y = functions.length;
  var myInterval = null;
  var loop = function (goToFirst) {
    if(goToFirst == true)
      f1();
    myInterval = setInterval(function(){
      $(".flex-next").click();
    },1000);
  }
  
  loop(true);

  $("ol li ").click(function () {
    var nr = $(this).data("number");
    functions[nr - 1]();
  });


  $(".flex-next").click(function () {
    var nr = $("ol").find("li.active").data("number");
    clearInterval(myInterval);
    if (nr == y) {
      functions[0]();
    } else {
      functions[nr]();
    }
    loop(false);
  });


  $(".flex-prev").click(function () {
    var nr = $("ol").find("li.active").data("number");
    clearInterval(myInterval);
    if (nr == 1) {
      functions[y - 1]();
    } else {
      functions[nr - 2]();
    }
    loop(false);
  });

});
@import url(http://reset5.googlecode.com/hg/reset.min.css);
ol li {
  width:50px;
  height:50px;
  background-color:blue;
  float:left;
  margin:10px;
}
ul li a {
  text-decoration: none;
  width:100px;
}
.active {
  background-color:red;
}
<script src="http://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<ol>
  <li data-number="1" class="1 active ">
    <a href="#"></a>	
  </li>
  <li data-number="2" class="2">
    <a href="#"></a> 
  </li>
  <li data-number="3" class="3">
    <a href="#"></a> 
  </li>
  <li data-number="4" class="4">
    <a href="#"></a> 
  </li>
  <li data-number="5" class="5">
    <a href="#"></a> 
  </li>
</ol>
<ul class="flex-direction-nav">
  <li>
    <a class="flex-prev" href="javascript:void(0);">Previous</a>
  </li>
  <li>
    <a class="flex-next" href="javascript:void(0);">Next</a>
  </li>
</ul>
0
votes

As you admit, the for-loop seems to not fit the job. Instead, let's just make our own loop that isn't depending on a i

var interval= 5000;

function navLoop() {

    (function loopCheck() {

        var $thisLi = $(".myList").find(".active")

        if($thisLi.next('li').length) { //check to see if "this" isn't the last element
             $thisLi.removeClass("active")
             $thisLi.next('li').addClass("active");

        } else { //If "this" is last element, just remove class "active" and add it to the first li
            $(".myList li").removeClass("active")
            $(".myList li").first().addClass("active")
        }


        setTimeout(loopCheck, interval)

    }());

}

Now you don't need a function for each individual list element, just call the navLoop() once and let it work on its own. navLoop() doesn't care what element is currently "active" by the user's choice, it will just try to keep the loop going from where the "active" element currently is.

0
votes

I've changed your js:

$(document).ready(function(){

    var parent = $('ol'),
        items = parent.find('li'),
        itemsLength = items.length,
        currentTime;
    var navItem = function(way){
        var nextItem, currentItem;
        items.each(function(i){
            if($(this).hasClass('active')){
                currentItem = $(this);
                if(way == 'next')
                nextItem = (i+1==itemsLength) ? $(items[0]) : $(items[i+1]);
                else nextItem = (i==0) ? $(items[itemsLength-1]) : $(items[i-1]);
            }
        });
        currentItem.removeClass('active');
        nextItem.addClass("active").siblings("li");
        currentTime = setTimeout(function(){navItem('next')}, 1000);
    }
    navItem('next');
    $('.flex-prev').click(function(){
        clearTimeout(currentTime);
        navItem('prev');
    });

    $('.flex-next').click(function(){
        clearTimeout(currentTime);
        navItem('next');
    });
    items.click(function(){
        clearTimeout(currentTime);
        var newCurrent = $(this).prev('li')
        items.removeClass('active');
        if(newCurrent.length != 0)
        $(this).prev('li').addClass("active").siblings("li");
        else
        $(items[itemsLength-1]).addClass("active").siblings("li");
        navItem('next');
    });

});

You can check it in jsfiddle

0
votes

HTML

<ul>
<li>1</li>
<li>2</li>
<li>3</li>
<li>4</li>
<li>5</li>
</ul>
<div id="next">NEXT</div>
<div id="prev">PREV</div>

JS

addActive = function(){
    $('.active').removeClass('active');
    $(this).addClass('active');
}

loop = function(){
    var li = $('.active');
    $('.active').removeClass('active');
    if($( li ).next().length == 0){
        $(" li:first-child ").addClass('active');
    }
    else
    $( li ).next().addClass('active');
}

nextClick = function(){
    var li = $('.active');
    $('.active').removeClass('active');
    if($( li ).next().length == 0){
        $(" li:first-child ").addClass('active');
    }
    else
    $( li ).next().addClass('active');
}

prevClick = function(){
    var li = $('.active');
    $('.active').removeClass('active');
    if($( li ).prev().length == 0){
        $(" li:last-child ").addClass('active');
    }
    else
    $( li ).prev().addClass('active');
}
$(document).ready(function(){
    //LOOP
    setInterval(loop, 2000);
    //LI CLICK
    $('li').click(addActive);
    //NEXT CLICK
    $('#next').click(nextClick);
    //PREV CLICK
    $('#prev').click(prevClick);
});

Just use this it would work fine

0
votes

I think that if you simplify a bit your initial code, then everything will be easier. Basically you want automate the "next" sequence. It means to me that you have to improve that part as first thing.

I refactored your code as follow:

$(function(){

  // we move the logic of `next` and `prev` in separate,
  // stand alone, functions in order to be reusable.
  function next() {
    // we remove the `active` class from the active element,
    // and we're searching for the next one
    var li = $("ol li.active").removeClass("active").next("li");

    // adding `active` class to the next one, or to the `first` in case
    // there is no next element.
    (li.length ? li : $("ol li").first()).addClass("active");  
  }

  function prev() {
    // we remove the `active` class from the active element,
    // and we're searching for the previous one
    var li = $("ol li.active").removeClass("active").prev("li");

    // adding `active` class to the previous one, or to the `first`
    // in case there is no previous element
    (li.length ? li : $("ol li").last()).addClass("active");  
  }

  // Here we add the logic when we click on `li` elements.
  $("ol").on("click", "li", function(){
   $(this).addClass("active").siblings().removeClass("active");
  });

  // We bound the `next` and `previous` functions to the links
  $(".flex-next").click(next);
  $(".flex-prev").click(prev);

  // At this point, we can easily call both `next` and `prev` functions
  // also from intervals or timeouts
  setInterval(next, 1000);
});

This is working because both next and prev functions doesn't depends by anything else than the DOM itself. So, in order to understand what is the "active" element, it search for the element with active class: it doesn't really matter which one is it, if it was moved by the clicking of the user, or something else. Both functions get the current active element, and move the class to the next / previous element – if possible, otherwise to the first / last one.

You could also use timeout, instead of interval, and adjust the time of the next iteration based on the user's click – or cancel it.

Hope it helps.