loukmanebabakhalil / prototype-carousel

Automatically exported from code.google.com/p/prototype-carousel
0 stars 0 forks source link

circular doesnt work on visibleSlides > 1 #15

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
any suggestions how to make it work?

Original issue reported on code.google.com by ryr1...@gmail.com on 6 Nov 2009 at 6:52

GoogleCodeExporter commented 8 years ago
Hello,

There is a bug on line 195 of carousel.js:

    var nextIndex = (this.slides.length - 1 == currentIndex) ? (this.options.circular ? 0 : currentIndex) : currentIndex + 1;

Should be:

    var nextIndex = (this.slides.length - this.options.visibleSlides == currentIndex) ? (this.options.circular ? 0 : currentIndex) : currentIndex + 1;

Also, the documentation does not specify that:

1) for circular with visibleSlides > 1 the last #visibleSlides slides must be 
duplicated, not just the last one
2) the height of the "middle" div must be the same as the "carousel" div, to 
allow the scroll effect to work correctly

I have modified carousel.js accordingly and this is attached, for anyone who 
wants to try the fix.

Thanks
Jay

Original comment by james.ha...@gmail.com on 22 Jul 2010 at 11:11

Attachments:

GoogleCodeExporter commented 8 years ago
i modified the two methods next() and prev(), since jay's bugfix didn't work 
for me and also didn't include a fix for prev():

prev: function () {
    var currentIndex = this.current ? this.current._index : 0;
    if (currentIndex == 0)
    {
        if (this.options.circular)
        {
            if (this.options.effect != 'fade')
            {
                this.scroller.scrollLeft = (this.slides.length - this.options.visibleSlides) * this.slides.first().getWidth();
                this.scroller.scrollTop = (this.slides.length - this.options.visibleSlides) * this.slides.first().getHeight();
                var prevIndex = this.slides.length - this.options.visibleSlides - 1;
            }
        }
        else 
            var prevIndex = 0;
    }
    else 
        var prevIndex = currentIndex - 1;

    this.moveTo(this.slides[prevIndex]);
},

next: function () {
    var nextIndex = 1;
    if (this.current) {
        var currentIndex = this.current._index;
        if (this.slides.length - this.options.visibleSlides == currentIndex)
        {
            if (this.options.circular)
            {
                if (this.options.effect != 'fade')
                {
                    this.scroller.scrollLeft = 0;
                    this.scroller.scrollTop  = 0;
                }
            }
            else
                nextIndex = currentIndex;
        }
        else
            nextIndex = currentIndex + 1;
    }

    if (nextIndex > this.slides.length - (this.options.visibleSlides + 1)) {
        nextIndex = this.slides.length - this.options.visibleSlides;
    }       

    this.moveTo(this.slides[nextIndex]);
},

that code worked for me with four visible slides.

Original comment by chowsin...@googlemail.com on 16 Sep 2010 at 10:41

GoogleCodeExporter commented 8 years ago
chowsinwon,

Where does that code go? I am using the Carousel.js file that came in the 
download, but your code above appears to be different?

Original comment by ikj...@gmail.com on 22 Sep 2010 at 5:47

GoogleCodeExporter commented 8 years ago
the bugfix of chowsinwon work perfectly with 3 visible slides.

it's possible to include this in Carousel.js  ?

Original comment by Sylvain....@gmail.com on 27 Jan 2011 at 9:13

GoogleCodeExporter commented 8 years ago
Chowsin's fix worked, except that the carousel jumps to the second item when it 
circles back. 

Change this: 

    next: function () {
        var nextIndex = 1;

To this:

    next: function () {
        var nextIndex = 0;

Remember that arrays are zero indexed.

Original comment by jed%jedf...@gtempaccount.com on 26 Mar 2011 at 12:08

GoogleCodeExporter commented 8 years ago
THANKS GUYS!!! THIS WORKED!!!

Not sure why the heck this isn't IN the code already...

Original comment by bostonde...@hotmail.com on 5 Apr 2011 at 10:25