kenwheeler / slick

the last carousel you'll ever need
kenwheeler.github.io/slick
MIT License
28.51k stars 5.88k forks source link

slickGoTo problem when not enough slides for slidesToShow depending on slidesToScroll #1006

Open lmcorreia opened 9 years ago

lmcorreia commented 9 years ago

I'm experiencing an odd behavior:

It seems that slick will refuse to the last slides if there are not enough slides to fill a viewport of "slidesToShow" width. This behavior also changes with the value of "slidesToScroll".

If you play around with slidesToScroll or simply add enough slides, the problem goes away. Here's my problem on a JSFiddle, along with a working 12 slides slick: http://jsfiddle.net/lcorreia/xr3k0fjy/

Also some experiments with 8 slides: slidesToShow:3 - http://jsfiddle.net/lcorreia/wat0aqcf/ slidesToShow:4 - http://jsfiddle.net/lcorreia/L4h8mqbe/

I'm gonna try to study and pinpoint the exact fault, all help is appreciated.

UPDATE 1:

This problem occurs because the "getNavigableIndexes()" method only returns [0, 4]: (slick.js line 641). This only allows slick to scroll slides [4, 5, 6, 7] into view, leaving slide[8] hidden to the right.

This contrasts with the behavior we get using the navigation arrows. With slides [0, 1, 2, 3] in view, if you push the next arrow once you'll get [4, 5, 6, 7] and if you push it once again you'll get [5, 6, 7, 8] - this is the desired behavior.

I've tried hard-coding the navigableIndexes to [0, 4, 5] and the goTo(8) works as it should but it then breaks the previous arrow navigation. You push it once it shows [1, 2, 3, 4] and it won't scroll back to slide[0]. What it should do is scroll to [4, 5, 6, 7] when pushed once and [0, 1, 2, 3] when pushed twice.

UPDATE 2:

This seems to solve my problem:

    Slick.prototype.getNavigableIndexes = function() {

        var _ = this, breakPoint = 0, counter = 0, indexes = [], max;

        if(_.options.infinite === false) {
            max = _.slideCount - _.options.slidesToShow + 1;
            if (_.options.centerMode === true) max = _.slideCount;
        } else {
            breakPoint = _.slideCount * -1;
            counter = _.slideCount * -1;
            max = _.slideCount * 2;
        }

        while (breakPoint < max){
            indexes.push(breakPoint);
            breakPoint = counter + _.options.slidesToScroll;
            counter += _.options.slidesToScroll <= _.options.slidesToShow ? _.options.slidesToScroll  : _.options.slidesToShow;
        }

        if(_.slideCount % _.options.slidesToScroll !== 0) indexes.push(breakPoint);

        return indexes;

    };

I only added the line: if(_.slideCount % _.options.slidesToScroll !== 0) indexes.push(breakPoint);

I haven't tested it properly but it seems to work ok in all my sliders. Both slickGoTo() and arrow navigation work as expected.

UPDATE 3:

The previous fix still had a few issues with a few select cases. I then thought about it a bit more and I think I arrived at something that makes a bit more sense:

    Slick.prototype.getNavigableIndexes = function() {

        var _ = this, breakPoint = 0, counter = 0, indexes = [], max;

        if(_.options.infinite === false) {
            max = _.slideCount - _.options.slidesToShow + 1;
            if (_.options.centerMode === true) max = _.slideCount;
        } else {
            breakPoint = _.slideCount * -1;
            counter = _.slideCount * -1;
            max = _.slideCount * 2;
        }

        while (breakPoint < max){
            indexes.push(breakPoint);
            breakPoint = counter + _.options.slidesToScroll;
            counter += _.options.slidesToScroll <= _.options.slidesToShow ? _.options.slidesToScroll  : _.options.slidesToShow;
        }

        if(indexes.indexOf(_.slideCount-_.options.slidesToShow) === -1) indexes.push(_.slideCount-_.options.slidesToShow);

        return indexes;

    };

As you can see, the fix is now:

 if(indexes.indexOf(_.slideCount-_.options.slidesToShow) === -1) indexes.push(_.slideCount-_.options.slidesToShow);

And it makes sense. What was missing in this "indexes" array was the last slide that can be placed on the first position of the viewport (left) placing the absolute last slide on the last position of the viewport (right). For example, in a 9 slides slick with 4 slidesToShow (my case), this would be slide[9-4], i.e., slide[5].

That way, we have 3 possible sets of slides to be shown in the viewport: [0, 1, 2, 3], [4, 5, 6, 7] and now [5, 6, 7, 8].

I haven't looked at a better way to apply the code, I think it would look nicer if we'd change the while loop or its preconditions but this seems to be the definitive fix.

PS: Please ignore the commit below, I was just testing to find a fix...

themikeb commented 9 years ago

Thanks much! I just ran into this issue as well in 1.4.1.

However, that does not cover all cases. slidesToShow is 3, slidesToScroll is 2 and total slides are 6, the last slide will not be displayed. Set slidesToScroll to 3 or 1 and it works.

lmcorreia commented 9 years ago

It may need some adjustments but @kenwheeler must be busy so don't expect a patch just yet. I would test it myself and make a pull request for a fix but sadly I'm a bit tied up at the moment.

sooroos commented 7 years ago

@lmcorreia thx, u saved my day. I had the same problem in 1.6

pravinweb commented 7 years ago

@lmcorreia Thanks much I am facing same issue

onstottj commented 7 years ago

Any update on a fix for this?

AMiketta commented 7 years ago

@lmcorreia thx, u saved my day to...

stevenbrown-85 commented 7 years ago

Thanks @lmcorreia! - this resolved my issue in 1.8.0 with slickGoTo and infinite set to false

Frohteloss commented 5 years ago

This is still an issue for me in v1.9 with slickGoTo and infinite set to false. If the number of remaining items in the list is less than the slidesToScroll setting, nothing happens when you hit next. It should navigate to the end slide. Same with in the beginning of the list too.

perrette commented 2 years ago

Hi, I was wondering how best to implement this hack? (since I am still seeing this issue in latest version 1.8.1). The Slick class is not defined in the global namespace.