Only add infinite carousel if needed

The “problem” is that the back and forward arrows are added via the infiniteCarousel.js file. We don't know exactly how many slides there are. But I think that this is an important change to the layout which should be applied by default and not as an option. The following will do the trick and additionally will result in lesser HTML.

The following will do the trick you want:

// $Id: infiniteCarousel.js,v 1.1 2010/05/10 12:54:58 manuelgarcia Exp $

/**
 * Plugin written by the great jqueryfordesigners.com
 * http://jqueryfordesigners.com/jquery-infinite-carousel/
 */
$.fn.infiniteCarousel = function () {

    function repeat(str, num) {
        return new Array( num + 1 ).join( str );
    }

    return this.each(function () {

        var $wrapper = $('> div', this).css('overflow', 'hidden'),
            $slider = $wrapper.find('> ul'),
            $items = $slider.find('> li'),
            $single = $items.filter(':first'),

            singleWidth = $single.outerWidth(),
            visible = Math.ceil($wrapper.innerWidth() / singleWidth), // note: doesn't include padding or border
            currentPage = 1,
            pages = Math.ceil($items.length / visible);

        // Check if all thumbs are already visible. If so we do not have to add the arrows.
        if ($items.length > visible) {
          // 1. Pad so that 'visible' number will always be seen, otherwise create empty items
          if (($items.length % visible) != 0) {
              $slider.append(repeat('<li class="empty" />', visible - ($items.length % visible)));
              $items = $slider.find('> li');
          }

          // 2. Top and tail the list with 'visible' number of items, top has the last section, and tail has the first
          $items.filter(':first').before($items.slice(- visible).clone().addClass('cloned'));
          $items.filter(':last').after($items.slice(0, visible).clone().addClass('cloned'));
          $items = $slider.find('> li'); // reselect

          // 3. Set the left position to the first 'real' item
          $wrapper.scrollLeft(singleWidth * visible);

          // 4. paging function
          function gotoPage(page) {
              var dir = page < currentPage ? -1 : 1,
                  n = Math.abs(currentPage - page),
                  left = singleWidth * dir * visible * n;

              $wrapper.filter(':not(:animated)').animate({
                  scrollLeft : '+=' + left
              }, 500, function () {
                  if (page == 0) {
                      $wrapper.scrollLeft(singleWidth * visible * pages);
                      page = pages;
                  } else if (page > pages) {
                      $wrapper.scrollLeft(singleWidth * visible);
                      // reset back to start position
                      page = 1;
                  }

                  currentPage = page;
              });

              return false;
          }

          $wrapper.after('<a class="arrow back" title="'+ Drupal.t('Previous page') +'">'+ Drupal.t('Previous page') +'</a><a class="arrow forward" title="'+ Drupal.t('Next page') +'">'+ Drupal.t('Next page') +'</a>');

          // 5. Bind to the forward and back buttons
          $('a.back', this).click(function () {
              return gotoPage(currentPage - 1);
          });

          $('a.forward', this).click(function () {
              return gotoPage(currentPage + 1);
          });

          // create a public interface to move to a specific page
          $(this).bind('goto', function (event, page) {
              gotoPage(page);
          });
        }
    });
};

You're welcome but I can't see the problem you described on the webpage you linked. But I can imagine what you mean. Again the problem is the way the thumbnails are generated in combination with the width of the thumbnail section. To solve this I (we) would have to know how to reproduce this particular problem. Additionally I think you should open a new issue for this.

Changed status to “needs review” because the above code solves the issue.

Fleshgrinder, can you provide a patch so I can actually see the changes you made, and if so apply them. On the cvs instructions you can find copy and paste code to work on this project easily =) (on the tabs on any project actualy).

I have taken a different approach than yours: I'm checking within galleryformatter.js to see whether or not we need to add the infinite carousel at all, and if so we do add it. Otherwise we don't call the plugin at all.

I believe my approach is a bit cleaner, since there is no need to call infiniteCarousel()

Guys, please test out the patch and report back, let's get this in before the next release.

Sorry for the late reply. You're right, I should really set up everything for creating patches and gain CVS access — but there are so many things to do all day. ;-)

Your approach is much better then mine and is working great, tested it with several constellations. In my opinion this patch should go into the module because the infinite carousel is really unnecessary if there aren't enough pictures.

Regards

Thanks for the review Fleshgrinder, committed.

I'm thinking that perhaps we should make the infinte carrousel optional, but that's for another issue.

BTW, there's no need for cvs access for just checking out a project from cvs as annonymous and provide patches ;)

Sure, I just meant that I should make myself familiar with that whole Patch creation thing. Diff is only available on Unix machines, currently I only have a Win7 box here. But maybe Winmerge can do the same for me.

Ow right... windows haven't used it in years (ubuntu here), I'm sure theres a way I guess.

BTW, 6.x-1.0-rc3 should be coming out soon.