jsor / jcarousel

Riding carousels with jQuery.
https://sorgalla.com/jcarousel/
MIT License
1.99k stars 736 forks source link

Duplication of LI elements when wrap: 'circular' #14

Closed JonMcL closed 14 years ago

JonMcL commented 14 years ago

So I'm loving jCarousel so far and I'm planning to utilize it on a site I'm currently working on. I'm particularly interesting in the vertical mode with a 'circular' wrap, but I'm noticing something I think is odd. I just want to make sure I'm not doing something to cause it....

When the vertical carousel reaches the bottom of the

  • list, it adds the first
  • to the bottom of the list so that it scrolls into place. That is working fine, but the
  • is left at the top of the list. If you repeat through the list multiple times, the entire list is duplicated over and over again. After 10 complete vertical cycles, you've got 10 times the number of
  • elements you start with. Is this by design? Or is there something that might be setting it off?

    I would have assumed that the first

  • gets pulled off the top of the list and then moved to the bottom (and vice versa when you're going the other direction) rather than duplicated. My issue is that I have complicated tooltip-type popups tied to the
  • elements and it seems to get confused as to which
  • triggered the popup. They frequently popup adjacent to a
  • that has long since scrolled out of view.

    Thanks!

    ..jon

  • jsor commented 14 years ago

    Hi,

    yes, this was already reported in #5. You should look at this issue, i posted a workaround there.

    JonMcL commented 14 years ago

    Thanks for the quick reply and sorry I didn't search through the messages more carefully to find that other issue.

    The solution you give works as far as keeping the size of the list down to a small amount, but it doesn't solve the issue of duplicating list items. I end up with a list of the original items plus duplicates for each item that is currently visible. So if I have 3 items visible, I end up with size()+3 with the extra 3 either having negative indexes and coming before the original list or positive indexes and coming after the original list.

    It did get me working on a solution. This could probably be a lot more compact and eloquent, and it will need modifications to work with other configurations.. and it's a huge hack .. but it seems to be doing the job so far for me...

    itemVisibleOutCallback: {onAfterAnimation: function(carousel, item, i, state, evt) { switch (state) { case 'next': delPos = carousel.last - carousel.size(); break; case 'prev': delPos = carousel.first + carousel.size(); } if (carousel.get(delPos).length == 1) { carousel.remove(delPos); switch (state) { case 'next': carousel.first--; carousel.last--; break; case 'prev': carousel.first++; carousel.last++; } } // resequence the indexes var $container = carousel.container; var newIndex = 0; var currentIndex; $container.find('li').each(function() { newIndex++; $li = $(this); currentIndex = $li.attr('jcarouselindex'); $li.removeClass('jcarousel-item-'+currentIndex); $li.removeClass('jcarousel-item-'+currentIndex+'-vertical'); $li.attr('jcarouselindex', newIndex); $li.addClass('jcarousel-item-'+newIndex); $li.addClass('jcarousel-item-'+newIndex+'-vertical'); }); }}

    jsor commented 14 years ago

    Hi, yeah, you're right. It doesn't solve your problem completely. The issue sounds more trivial than it is actually.

    In your code, you're re-assigninig the index for each item. This works in your case, but could break other userland code.

    But its on my todo list and i'll try to find a solution.

    Jan

    JonMcL commented 14 years ago

    Hi Jan,

    Just an FYI -- I had originally just removed 'old' duplicate LI's from the far end of the list and left the indexes intact. This worked fine for one complete cycle through the list. Unfortunately, when you go beyond the first cycle, jCarousel's LI duplication seems to look for the original LI elements under the original index. So even though the list has moved to indexes like -7 through 0 (for an 8 item list), the duplication code goes to look for indexes 1 through 8 when it tries to add more LI elements at position -8. So it ends up adding empty LI elements. The re-indexing was just my hacked method of solving that issue.

    I'm good for now :) .. but yes, some folks might like having the functionality built in incase they have event bound to the LI elements like I do.

    Amazing work on the plugin! And very well coded!

    ..jon

    JonMcL commented 14 years ago

    Just an update. I unfortunately had to abandon jCarousel and instead switch to jQuery Tools' Scrollable (which had another problem I needed to fix .. see http://flowplayer.org/tools/forum/35/40506 ). The issue is that I'm also using Tools' Tooltip plugin, and when jCarousel clones the elements that have tooltips attached to them, the trigger event gets cloned correctly but it appears that Tooltip get confused and tries to attach the popup container to the original element instead of the displayed clone. Scrollable actually clones element during initialization and when the clone scrolls into position, it snaps the list back to the right position to display the original. (or something like that) Once everything is cloned, I initialize Tooltip and it attaches triggers to the originals and the clones.