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

totalItems count may also include other elements than img #9

Open
stefan-korn opened this issue Feb 7, 2013 · 1 comment
Open

totalItems count may also include other elements than img #9

stefan-korn opened this issue Feb 7, 2013 · 1 comment

Comments

@stefan-korn
Copy link

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
Copy link
Owner

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

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

No branches or pull requests

2 participants