Accessible360 / accessible-slick

the last (accessible) carousel you'll ever need.
https://accessible360.github.io/accessible-slick
MIT License
259 stars 47 forks source link

"refresh" method deletes the slides when reinitialising the slider. #38

Open mitchell-ionata opened 3 years ago

mitchell-ionata commented 3 years ago

I'm responsively unslicking my slider and with Slick v1.8.1 I was able to $('.my-slider').slick('refresh'); to reinitialise the slider, however with this fork, it does attempt to reinitialise the slider but all the slides are removed and the .slick-track is completely empty.

jasonwebb commented 3 years ago

Weird! Can you post a code sample demonstrating this issue in action so we can troubleshoot it?

mitchell-ionata commented 3 years ago

Weird! Can you post a code sample demonstrating this issue in action so we can troubleshoot it?

Sure thing, here's a codepen: https://codepen.io/Mitchell-ionata/pen/QWvBEWg Press the "Unslick" button to first run the unslick method on the slider, then the slides disappear with the "Re-slick" button, which uses the refresh method.

jasonwebb commented 3 years ago

Thanks for putting that together! I can confirm that this is an issue that seems to be present in accessible-slick, but not the original Slick library. But I haven't been able to figure out the cause of it yet.

There haven't been any changes made to either the unslick or refresh functionality directly, so whatever is going on seems like it must be a side effect to some unrelated changes.

So far I've found that the unslick option is really just an alias for the destroy() method, and the destroy() method is already called by the refresh() method. So in your example, when you "unslick" the carousel and try to do a refresh, the carousel gets destroyed twice. When the second destroy() call is made inside the refresh method, the slides get removed from the DOM entirely. I think I've narrowed down the problem down to this line of code in the cleanUpRows() method, but I understand yet what is going on. Seems like there is some hand-wavy jQuery magic going on that is making the issue really tricky to trace.

I'm also seeing that the refresh() method is not officially documented or demonstrated in the original library's README or docpage, so the intended purpose and proper usage is a bit unclear to me. As a short term solution, I wonder if you can get the results you want by re-initializing Slick entirely instead of trying to use the refresh option, like the snippet below. But since I do really want this package to be a true drop-in replacement for Slick Slider, I want to resolve this issue regardless - I'm just not sure how long that'll take.

$(".reslick").on("click", function() {
  $(".my-slider").slick({
    infinite: true,
    slidesToShow: 3,
    slidesToScroll: 1,
    arrows: true,
    dots: false,
    centerMode: true,
  });
});

If you, or anyone else, has any ideas for what could be going on here, please let me know!

mitchell-ionata commented 3 years ago

Thanks for looking into it! Yeah I just happened to stumble upon the refresh method when looking into how to easily reinitialise my slider after it has been unslicked so it's definitely possible how I'm using it is not intended.

I'll have a poke around myself if I find the time, cheers!

rmcveigh commented 3 years ago

FWIW, I've run into this issue as well. It looks like this could be related to an issue on slick.js where the refresh with filtering was removing the slides. That issue was resolve in 1.9.1.

fri3ndly commented 3 years ago

I'm experiencing the same issue. Using NPM i'm getting v1.81 latest. Just had a look at the git repo and it looks like the v1.9.0 was reverted - anyone have any idea how to fix this issue?

zotsf commented 2 years ago

It seems this issue is documented here: https://github.com/kenwheeler/slick/issues/3308#issuecomment-668850879