views:

82

answers:

6

I have what i thought was a simple javascript / jquery function (fade out of one div, fade into another... loop until it reaches a maximum and then start back from the begining. The problem i have though is that to fadein the next div i need to increment the global counter. Doing this increments double increments it because i'm assuming the local variable i've created maintains the same reference to the global variable.

The code sample below should explain a little easier. Can anyone spot what i'm doing wrong?

var current_index = 1;

$(document).ready(function() {
    $(function() {
        setInterval("selectNextStep()", 3000);
    });
});

function selectNextStep() {
    $("#step_"+current_index).fadeOut('slow', function() {
        var next = current_index;
        next = next + 1;
        $("#step_"+next).fadeIn('slow', function() {
            if (current_index == 4) current_index = 1;
            else current_index ++;
        });
    });
}
+1  A: 

isn't $(function() {}); the same as $(document).ready(function(){}), so you are initializing selectNextStep twice (hence the double increment)?

James Connell
No, that's not the issue, the double wrap is unnecessary yes, but will not cause code to be executed twice. You can test it here: http://jsfiddle.net/nick_craver/tQ6bP/1/
Nick Craver
I was just looking for the low hanging fruit. Jsfiddle is great, thanks for that.
James Connell
+1  A: 

Try this. Simplifies things a little. Increments (and resets if needed) the current_index before the next fadeIn().

Example: http://jsfiddle.net/r7BFR/

var current_index = 1;

function selectNextStep() {
    $("#step_" + current_index).fadeOut('slow', function() {
        current_index++;
        if (current_index > 4) current_index = 1;
        $("#step_" + current_index).fadeIn('slow');
    });
}

$(document).ready(function() {
    setInterval(selectNextStep, 3000);
});

EDIT: Added example, and fixed my misspelling (camelCase) of current_index.

Here's an alternate way of doing the increment:

current_index = (current_index % 4) + 1;
patrick dw
+2  A: 

I do not see any double increment the way your code is..

the problem is that the next variable goes beyond the 4 value that seems to be the limit, and trying to fadein an element that does not exist. so the code that resets the currentIndex never executes..

try adding if (next > 4 ) next = 1; after increasing the next variable

Example at http://jsfiddle.net/5zeUF/

Gaby
A: 

Try this, slightly different approach but does what you need it to do I believe, also you can add more steps without modifying the script and doesn't pollute the global namespace (window)

[HTML]

​<div class="step defaultStep">One</div>
<div class="step">Two</div>
<div class="step">Three</div>
<div class="step">Four</div>
<div class="step">Five</div>

[CSS]

.step { display: none; }
.defaultStep { display: block; }​

[JS]

$( function() {
    var steps = $( ".step" );
    var interval = setInterval( function( ) {
        var current = $( ".step" ).filter( ":visible" ), next;
        if( current.next( ).length !== 0 ) {
            next = current.next( );
        } else {
            next = steps.eq(0);
        }
        current.fadeOut( "slow", function( ) {
             next.fadeIn( "slow" );  
        } );
    }, 3000);
} );
Stuie Wakefield
Regards the problem with copying a global variable into the local scope, in your code the local "next" variable takes on the global "current_index" variable's value and not its pointer so you are safe to modify the "next" variable. Only direct assignments to "current_index" will modify the global variable. Your if statement has a semi colon before the else, remove that first. I cannot see any other place in which "current_index" is being assigned a value, the callback could be getting called twice for some reason.
Stuie Wakefield
A: 

Maybe you also want to have a look at the cycle plugin for jquery. There you can actually achieve such nice transitions. I think with a little work this would ease up everything.

http://jquery.malsup.com/cycle/

Regarding your code snippet. I think you can enhance it a little in this way:

$(document).ready(function() {
    var current_index = 0;

    window.setInterval(function() {
        $("#step_"+ current_index).fadeOut('slow', function() {
            $("#step_" + (current_index + 1)).fadeIn('slow', function() {
                current_index = (current_index + 1) % 4;
            });
        });
    }, 3000);
});

This should do the exact same work. As the interval function closes over the current_index variable it should be valid inside the function. Sorry, if you're not a fan of all these closures but I rather preferr passing the function I want to execute directly to the setInterval function, than defining it anywhere else.

P.S. Be aware that the changes I introduced imply that your #step_ IDs start at 0.

philgiese
+2  A: 

I think you're ending up with race conditions due to the interval trying to fade things in and the callbacks trying to fade things out. For this setup it makes more sense to let the fade callbacks start the next round.

Also using a 0-based index makes the math easier.

var current_index = 0; // zero indexes makes math easier

$(document).ready(function () {
    $(function () {
      // use timeout instead of interval, the fading callbacks will 
      // keep the process going
        setTimeout(selectNextStep, 3000);
    });
});

function selectNextStep() {

  // +1 to adapt index to element id
    $("#step_" + (current_index + 1)).fadeOut('slow', function () {

        var next = current_index + 1;

       // keeps index in range of 0-3
        next = next % 4; // assuming you have 4 elements?
        current_index = (current_index + 1) % 4; 

      // callback will start the next iteration
        $("#step_" + (next + 1)).fadeIn('slow', function () {
            setTimeout(selectNextStep, 3000);
        });

    });
}

demo: http://jsbin.com/exufu

lincolnk