Wilto / Dynamic-Carousel

A carousel plugin built for responsive layouts. There are many like it, but this one is mine.
matmarquis.com/carousel/
472 stars 112 forks source link

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

Open adamnorwood opened 12 years ago

adamnorwood commented 12 years ago

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 :)

markadrake commented 12 years ago

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 commented 11 years ago

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 commented 11 years ago

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