Unexpected order in script execution

Question

Why is it running the second script and how can I make it work like a toggle control?

<script>
    var el = 2;

    $(document).ready(function(){
        $(".rightDiv").click(function(){
            if (el == 2) {
                $(this).animate({left:'150px'});
                el = 1;
            }
        });
    });
</script>

<script>

    $(document).ready(function(){
        $(".rightDiv").click(function(){
            if (el==1) {
                $(this).animate({left:'50px'});
                el = 2;
            }
        });
    });
</script>

It's running both statements because you've attached 2 `click` callbacks. Check http://stackoverflow.com/questions/209029/best-way-to-remove-an-event-handler-in-jquery for how to remove the current event handlers before you attach new ones. — h2ooooooo, Dec 22 '12 at 01:39

score 1 · Answer 1 · answered Dec 22 '12 at 02:02

You only need one .ready()

$(document).ready(function()
{ 
  var el = false; 
  var rightDivs = $('.rightDiv');
  $(rightDivs).click(function(){ 
      if (el == false) 
      { 
        $(this).animate({left:'150px'}); 
        el = true; 
      }
      else if (el == true)
      {
         $(this).animate({left:'50px'}); 
        el = false;
      }
    }); 
});

score 0 · Answer 2 · answered Dec 22 '12 at 01:45

0

This should work fine for you:

var el = false;
$(document).ready(function() {
    $(".rightDiv").click(function() {
        $(this).stop(true).animate({left: (el ? 50 : 150) + 'px'});
        el = !el;
    });
});

jsfiddle with example

answered Dec 22 '12 at 01:45

h2ooooooo

36,580
8
61
97

score 0 · Answer 3 · answered Dec 22 '12 at 01:53

You have atteached two event handlers, so when the event happens it will run one, then the other.

As the first one will change the variable so that the condition in the second one becomes true, the code inside both if statements will run.

Put the code in the same event handler, so that you can use else to run only one of them:

<script>
  $(document).ready(function(){

    var el = 2;

    $(".rightDiv").click(function(){
      if (el == 2) {
        $(this).animate({left:'150px'});
        el = 1;
      } else {
        $(this).animate({left:'50px'});
        el = 2;
      }
    });

  });
</script>

score 0 · Answer 4 · answered Dec 22 '12 at 02:06

here's a slightly improved version of @h2ooooooo 's answer, where we ditched the global scoped variable and use element's attributes.

Basically what we are doing here is to prevent bloating the global scope by using global variables and now we are working with data directly related to the element that was pressed.

$(document).ready(function() {
    $(".rightDiv").attr("isLeft",1).click(function() {
        var pos = "50";
        if( $(this).attr("isLeft") == "1" ){
            pos = "150";
            $(this).attr("isLeft",0)
        }else{
            $(this).attr("isLeft",1);
        }
        $(this).stop(true).animate({left: pos + 'px'});
    });
});

You should definitely use `.data()` for this solution instead of `.attr()` (and besides - you shouldn't set a custom attribute without prepending `data-` to it). — h2ooooooo, Dec 23 '12 at 13:26

Unexpected order in script execution

4 Answers4