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

Added lazyLoad option 'anticipated' to preload {n} images #2456

Merged
merged 2 commits into from
Oct 26, 2016

Conversation

Rendez
Copy link
Contributor

@Rendez Rendez commented Aug 6, 2016

Description

So that the user experience is better, the images should be loaded on demand, but anticipated, meaning, that the slider will always be one step ahead of the user, by pre-loading the 1 next and 1 previous image so that the loading is seamless, without ever making the user aware that the image was loading. On the other hand, this represents an improvement over loading all the images progressively, as the amount of data to load might not be worth causing also a poor experience, specially on mobile devices with slow connections.

Demo

http://jsfiddle.net/zeL1kmo3/12/

Process

There are a few edge cases in which lazyLoad === 'anticipated' might not work as expected, they are rooted on the nature of the current implementation, some bugs and limitations:

  1. If slidesToScroll > 1, going from index 0 to -2 and index 8 to 10 only contains one cloned item, so scrolling 2 shows an empty image during the animation This can be see in line https://github.com/kenwheeler/slick/blob/master/slick/slick.js#L2034, as slidesToShow is used where slidesToScroll should be the amount of clones to set-up.
  2. If slidesToScroll > 1 and the number of images is an odd number. Similar problem to 1.
  3. When fade == true, the rangeEnd seems to be +1 count too much, this is either related to another bug or an implementation feature I am not aware of.

Credits to @AlaaSarhan for his collaboration bringing this feature to life.

@tomhicz
Copy link

tomhicz commented Aug 12, 2016

Thanks for this - Very useful.
I'm using it on a site in development and it seems to be working well so far. (not using slidesToScroll > 1 or fade.)

@rafalf
Copy link

rafalf commented Sep 6, 2016

@kenwheeler @Rendez why is that not present in the CDN-ed version of slick.js - Version: 1.6.0
e.g. https://cdnjs.cloudflare.com/ajax/libs/slick-carousel/1.6.0/slick.js

@Rendez
Copy link
Contributor Author

Rendez commented Sep 7, 2016

@rafalf I have no rights to merge this PR.

@rafalf
Copy link

rafalf commented Sep 10, 2016

@Rendez Thanks! we added these bits and pieces to 1.6.0 and stopped using cdn but cloudfront instead and working like a charm.

@leroy0211
Copy link

Please merge this as soon as possible!

@kenwheeler kenwheeler merged commit 95e1be8 into kenwheeler:master Oct 26, 2016
@kenwheeler
Copy link
Owner

thanks @Rendez

jacobarriola added a commit to jacobarriola/slick that referenced this pull request Nov 6, 2017
Found this merge in kenwheeler#2456 but I don't see the documentation updated. Had to dig a while, but luckily ran into the PR ✨
@swinggraphics
Copy link

Can this feature be added to the documentation?

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.

6 participants