Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes autorotate double-delay on first and last slides (and mutliple-carousels-per-page issue) #58

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adamnorwood
Copy link

The existing autorotate code appears to cause the first and last slide to wait for twice as long as the others before moving (i.e. if the data-autorotate value is set to '3000', the first and last slides will actually pause for 6000ms). I've set up a demo of this here: http://adamnorwood.com/junk/dynamic-carousel-fix/ Or am I alone in seeing this?

The revised code in this pull request drops the multiple setInterval() calls in favor of a single setInterval() with a "direction" flag that can keep track of whether the carousel is moving forward or backward through the slides with each function call. Does that seem like a reasonable approach?

Also, at least with the demo that I linked above, I noticed a separate issue where multiple carousels on the same page that are set to different autorotate intervals would influence each other (the forward or backward movement would be applied to all of the carousels instead of the single carousel, if that makes sense). This pull request fixes that as well, but I haven't fully diagnosed why...correcting scope?

In any case, thanks for the awesome carousel plugin, I've been using it happily aside from this relatively minor glitch! Also, this is my first pull request here on GitHub, so apologies if I haven't done it correctly – any feedback would be appreciated :)

The earlier autorotate code appears to cause the first and last slide to wait for twice as long as the others before moving (i.e. if the data-autorotate value is set to '3000', the first and last slides will actually pause for 6000ms).

This code change also caches the $slider object and appears to fix an issue where the direction of movement for one carousel can affect the movement of other carousels on the page.
@markadrake
Copy link

Totally recommend merging this to the script. I saw the delay in Firefox this morning and used the code @adamnorwood provided above to fix it.

@ncompass
Copy link

ncompass commented Nov 8, 2012

Nice fix - to clarify: I replaced the 'data-autorotate' function on line 366 this this code:

    $slidewrap.filter('[data-autorotate]').each(function() {
        var auto,
            $el         = $(this),
            speed       = $el.attr('data-autorotate'),
            $slider     = $el.find(opt.slider),
            slidenum    = $el.find(opt.slide).length,
            forward     = true,
            autoAdvance = function() {
                var active   = -( $slider.getPercentage() / 100 ) + 1;

                switch ( active ) {
                    case slidenum:
                    forward = false;
                    break;
                    case 1:
                    forward = true;
                    break;
                }

            $slider.trigger("nextprev", { dir: (forward) ? 'next' : 'prev' });
        };

And it worked perfectly. Thank you

@MaryLiz
Copy link

MaryLiz commented Feb 8, 2013

Thanks so much! I used your revised plugin code and it fixed this issue for me as well. Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants