search

bkosborne / jQuery-Waterwheel-Carousel

GNU General Public License v3.0
105 stars 69 forks source link

totalItems count may also include other elements than img #9

Open stefan-korn opened 11 years ago

stefan-korn commented 11 years ago

I tried to use Waterwheel Carousel within Drupal. I embedded the div and images directly in a page with full html chosen. I did not think about that drupal automatically inserts br-tags for new lines. So my problem appeared that right part of the carousel was not shown at all and there was also some javascript errors given. Tracking the error down finally got me to the point I realized that the total item count was nearly doubled, which then led me to the br-Tags included by Drupal. Although someone could avoid this by just removing the br-Tags, it could also be worth thinking about whether to limit the total item count to img-Tags. Therefore I changed line 34 in jquery.waterwheelCarousel.js to this:

totalItems: $(carousel).children("img").length,

instead of previously having

totalItems: $(carousel).children().length,

I think this could make sense, maybe if someone wants to add for any reason some other elements in the carousel-div.

On the other side, I do not know if other items than images can also be rotated. If this is the case the proposed change will not be right. Than maybe a class selector could do the job.

I leave it to you Brian, whether you find this useful or not.

Just wanted to give this feedback, if maybe anyone else has the same experience.

Waterwheel is really great stuff, I like it very much.

bkosborne commented 11 years ago

Thanks for the note, I'll fix this in the next release.