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

Add / Remove 'active' class from slides #26

Closed
wants to merge 4 commits into from
Closed

Add / Remove 'active' class from slides #26

wants to merge 4 commits into from

Conversation

andrewmclagan
Copy link

Add / Remove 'active' class from slides. Developers can now scope CSS animations and specific styles to active slides.

Add / Remove 'active' class from slides. Developers can now scope CSS animations and specific styles to active slides.
@andrewmclagan
Copy link
Author

I have submitted the above pull request. can you please review and integrate?? or discuss here... awesome!

the DOM for controls and breadcrumbs is no longer generated when they are set to false or 'none' respectively.
Added a callback function that is called after a slidr is created. receives the slider object _
@bchanx
Copy link
Owner

bchanx commented Mar 24, 2014

Hey @andrewmclagan,

Thanks for the PR! Some comments:

  • In general, I like the active class addition, but it's a little too generic. I'm going to go with slidr-active instead.
  • The callback after Slidr creation is fine, but I'm going to make some code styling changes to make it more fluent with the rest of the code. (i.e default the creation callback to NOOP, so we don't need type check when calling).

@bchanx
Copy link
Owner

bchanx commented Mar 24, 2014

Actually @andrewmclagan, is there a reason why you're passing the internal Slidr _ data struct to the callback? Most of those properties are meant for internal library usage and shouldn't really be exposed outside of the library.

It would make more sense to pass the callback.meta() params back, with probably null/empty values for out. The meta params limit the data exposed, but it has a reference to the slidr element for dom manipulation. Thoughts?

@bchanx
Copy link
Owner

bchanx commented Sep 18, 2020

Closing due to inactivity

@bchanx bchanx closed this Sep 18, 2020
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.

None yet

2 participants