search

richardscarrott / jquery-ui-carousel

jQuery RS Carousel is a responsive and touch-enabled carousel written on top of jQuery and the jQuery UI Widget Factory providing a full and familiar API in less than 2.6kB minified and gzipped.
http://richardscarrott.github.io/jquery-ui-carousel/
192 stars 56 forks source link

restore active items #58

Closed greatseth closed 11 years ago

greatseth commented 11 years ago

I am not sure if this is a bug or feature request.

I started using the 0.11.1 tag. Things were swell until I got to styling active items. I noticed a bug where the active item class was never removed once set.

I went to fork the project with the ultimate goal of fixing this presumably simple bug. I was surprised to find the latest code had removed the active item behavior entirely. This seems strange to me as that feature seemed like an obvious one.

So, FWIW, I have pushed a branch which restores this feature and fixes the bug which inspired this journey.

https://github.com/greatseth/jquery-ui-carousel/commit/6df9ff9af7e28975e376241dffe806d651569880

If you would not apply this change, I'd love clarity on why if for no other reason than to determine if my team needs to fork or consider alternatives.

Thanks!

richardscarrott commented 11 years ago

That functionality has never been part of this carousel, it's something that can easily be added via the existing before or after callbacks but sounds useful enough to include by default, your fork doesn't add the className to every item in the page but if you want to send a pull request I'll merge it in and adjust it so rs-carousel-item-active is applied to all items in the current page with this.getPage().addClass(activeClass).

Cheers

greatseth commented 11 years ago

Well, I am thoroughly confused as to where this feature came from in the code I have, then. I am almost certain the code I am using is the 0.11.1 tag from your repo. Downloading that version again, I do not see the code. Was this project in another repo at some point perhaps? I am very curious about this detail now. The code I refer to is clearly a version of this project, with the same name, tree contents, extension scripts, etc.

I would appreciate your thoughts on this mystery, but either way, I'll try to revisit this soon and adjust the patch.

richardscarrott commented 11 years ago

It's always been in this (currently incorrectly named) repo. Which version did you have as I'm pretty confident this functionality hasn't existed previously, perhaps you originally downloaded somebody else's a fork of the code, either way I'll add this in if you send a pull request or alternatively I'll look into adding it when I next get some time.