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

Handle all Slick events with 1.4 event model, remove deprecated callbacks on init, and pass relevant event details to callbacks #109

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

weberml2
Copy link

Breaking change-- bind to callback function in order to pass event parameters back to controller. Supersedes PR #100

@mhulse
Copy link

mhulse commented Jun 26, 2015

Solid work! Thanks for doing this.

@mhulse
Copy link

mhulse commented Jun 26, 2015

Noticed some missing parts when comparing your PR with the Slick documentation and source code:

// edgeFriction
// mobileFirst
// respondTo
// rows
// slidesPerRow
// pauseOnDotsHover

// onEdge
// onSwipe

// ??
// onDestroy
// onBreakpoint

Check both slick instantiation and the scope var, as they both need slightly different additions to the missing bits listed above.

Also, I could not find any mention of a callback named breakpoint or destroy via the Slick docs/source code.

@mhulse
Copy link

mhulse commented Jun 29, 2015

Also, I could not find any mention of a callback named breakpoint or destroy via the Slick docs/source code.

Ok, found reference to breakpoint and destroy on this page.

Looks like the docs here are not up-to-date with the GitHub repo's readme.

@weberml2
Copy link
Author

Sounds good, I'll take another crack at it, and will probably need to rev the Slick dep in situ to make sure the documentation I am using matches the current Slick

@mhulse
Copy link

mhulse commented Jun 29, 2015

Sounds good, I'll take another crack at it

Awesome! Thanks @weberml2 👍

I'll double-check your work later today (first glance, it's looking good).

rev the Slick dep in situ to make sure the documentation I am using matches the current Slick

Ahh, good point. I overlooked that in my initial comment.

So, I'm hoping we can get @vasyabigi to merge. Your fixes should help to close/fix several other issue from what I have seen/read.

@nicolas-besnard
Copy link

Is @vasyabigi still alive ? This library hasn't been updated for a will now ...

@mhulse
Copy link

mhulse commented Jul 2, 2015

@nicolas-besnard Not sure. Looks like he needs some help maintaining.

@malfborger
Copy link

initial-slide doesn't seem to work for me. It just super fly aways on first "next". See http://cl.ly/1M2c2n1b2u30 Previous slide does work though. Any idea for this?
Same for latest original vasyabigi repo

The example is using

    <slick class="slider single-item" current-index="index" responsive="breakpoints" initial-slide="1" >
      <div ng-repeat="i in [1,2,3,4,5]"><h3>{{ i }}</h3></div>
      <p>Current index: {{ index }}</p>
    </slick>

@idreeshaddad
Copy link

I've emailed @vasyabigi to check on him and see if he is still maintaining this repo. If he is not; I think the best thing would be to make a new repo and add the code to it, and not fork it, because forking just hides code on GitHub.

@avivr
Copy link

avivr commented Jul 26, 2015

@idreeshaddad have you tried this repo - https://github.com/devmark/angular-slick-carousel ?
seems like another slick angular wrapper, more maintained and up-to-date

@idreeshaddad
Copy link

@avivr Yeah I've tried it and I personally didn't like it.
This one has more potential.
There are lots of good pull requests, I sure hope this gets maintained.

@baffleinc
Copy link

SO MUCH POTENTIAL. Such a shame @vasyabigi has neglected the plugin :(

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.

9 participants