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 functionality for cyclic sliding #14

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Sigma-90
Copy link

@Sigma-90 Sigma-90 commented May 9, 2013

I added some functionality to enable cyclic sliding by adding the new "transitionType" option "cycle". With this, when the last slide is reached and the "next" link is clicked, it no longer "rewinds" to the first slide but instead it moves the first slide to the end of the list and then slides to it. Of course the navigation will also be modified accordingly and it works in both directions. That way it is possible to silde in one direction infinitely. I think this is not only very handy but more or less neccesssary when implementing the slider for mobile devices and making use of swipe-events, because the user expects the slider to move in the same direction he moved it with his finger.

@getdave
Copy link
Owner

getdave commented Jun 11, 2013

@Sigma-90 looks really good thanks.

Question: did you remove a lot of comments or something? Seems to be a lot of removals? Am I ok to assume this has been tested on mobile devices?

@Sigma-90
Copy link
Author

@getdave As for the comments, check my replies to your code comments. As for mobile devices: Yes, I conducted mobile tests, but as I described in another reply, I used hammer.js for gesture detection because of the overlay in front of the slideshow that blocked the original swipe functionality, so I can only confirm that it works with hammer. I will have to test it again after I cleaned it all up and removed those dependencies.

@getdave
Copy link
Owner

getdave commented Jun 11, 2013

@Sigma-90 No problem. Thanks for replying. Let me know if/when you make the updates. I'm trying to keep the core script free of dependencies so I'd like to avoid hammer.js.

@getdave
Copy link
Owner

getdave commented Aug 9, 2013

@Sigma-90 Just checking whether you might consider making any further contributions to this pull request? I like your contributions but can only merge once fix have been made and dependencies removed. Much appreciated.

@Sigma-90
Copy link
Author

Sigma-90 commented Aug 9, 2013

I already did the cleanup work, just didn't find the time to put it through proper testing yet, very busy at work at the moment. As soon as I've verified that everything's working smoothly I will update the repo and drop you a note.

@Sigma-90
Copy link
Author

Cleanup completed... Finally. Yesterday evening I managed to find some time to run a last test to see if everything works. It did, so it was about time to commit everything, which I did just now. All external dependencies are out and the slides cycle just fine. Please note that the CSS file had to be changed as well in order to make it work correctly.

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.

3 participants