jsor / jcarousel

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

scrolling at beginning or end of circular goes to wrong position #506

Closed DiskHandler closed 10 years ago

DiskHandler commented 11 years ago

On my page I have several picture galleries. When clicking on the carousel I'd like for the selected image to be the center image. When using the scroll function more than 6 from the beginning or end of the image list, everything works fine. When I'm close to either end, the scroll function scrolls well away from the selected image and gets worse the more you click until you reload the page.

I also am adjusting the number of images shows depending on the size of the browser window. Again, far from either wnd, it works fine. Close to the end it scrolls to an unrelated image. You can see these here:

Working: http://ssbbs.dyndns.org/panic/gtest.asp?WGal=14&Page=20

Not Working http://ssbbs.dyndns.org/panic/gtest.asp?WGal=14&Page=2

Any help with this would be GREAT!!!!

Many thanks for a great carousel!

DiskHandler commented 11 years ago

This appears to be related. Any gallery I use that has more than 137 images, the carousel goes wild when choosing image 138 or higher.

Great work so far, BTW.

jsor commented 11 years ago

Looks like you calculations are wrong somewhere. Note, that the order of the items change in circular mode once you have reached the end of the carousel because the items are swapped around.

DiskHandler commented 11 years ago

I'm not sure what you mean by the items are swapped around. I'm just calling what I want to have as my first image to be shown. I've set up an alert to show the image number that is being called as the first image in the display and I'm taking in to account the end and beginning in my calculations. Once I call the image to be set as the first image, if it is near the end, the carousel acts weird. My alert shows that it isn't my calculations.

I'm using: $('#jcarousel1').jcarousel('scroll', NewStrt);

where NewStrt is the variable that is to be the first image shown in the carousel. Try it again with the alert and you will see what I mean.

Also, try this one:

http://ssbbs.dyndns.org/panic/gtest.asp?WGal=2&Page=149

It is supposed to put the first image as 149 but instead it puts image 6 as the first one. It is because it is after image 138. With image 138 or less, it puts the correct image as the first image.

DiskHandler commented 11 years ago

Sorry - I meant that with image 149 being called from the URL that image 146 would be the first one displayed but it is displaying image 6 as the first image. Again: Submitting an image number in the URL of 138 or less it works fine until an image higher than 138 is chosen.

DiskHandler commented 11 years ago

Also, calling directly image 140 in a carousel currently that has 328 images, the carousel doesn't show image 141 and up.

http://ssbbs.dyndns.org/panic/gtest.asp?WGal=2&Page=140

DiskHandler commented 11 years ago

Although, if you call say image 136 using the URL above, just using the next page at the bottom, it shows all the images loaded, but selecting past image 138 from the carousel it will start acting up there as well.

jsor commented 11 years ago

Looks like the 20000px width of <ul> isn't wide enough for all the items. You must ensure that all items fit in there. I also saw, that you're changing the width of the root element after initialization. Make sure to call the reload method afterwards.

DiskHandler commented 11 years ago

Is this 20000px width imposed by (UL) or jCarousel?

DiskHandler commented 11 years ago

Nevermind - I found what I was looking for in the CSS and increased the (UL) width. The weird limitation of 138 images has been fixed. Now to figure out the issue with the jumping at the beginning/end that I'm experiencing.

jsor commented 11 years ago

Thats what i meant in my first comment. I haven't looked in detail, but it seems you're assuming that the index of each item stays the same. But if you reach the end of the carousel in circular mode, jCarousel moves the items from the beginning to the end. So, the index changes. You should try to not move relatively but by item, e.g.:

$('.jcarousel li').on('click', function() {
    var current = $(this);

    // Now determine the element, we assume 5 items are visible
    var target = el.prev().prev();

    $('.jcarousel').jcarousel('scroll', target);
});
DiskHandler commented 11 years ago

I couldn't get that code to work, but thanks to your explanation of what was going on, I was able to change my code from scrolling to an index to scrolling to the target via an offset. As long as I'm keeping track of the offset when hitting the next/prev buttons, it works as expected. Thanks for clarifying this for me! I now can spend my time skinning my carousel!

Awesome job by the way!

DiskHandler commented 11 years ago

Ok, here is a question - if I start at say image 10, and I start going backwards and the carousel wraps, what happens to the index at that point? Does it go into negatives or what?

jsor commented 11 years ago

I realized, my example doesn't work with circular carousels. I push a little experimental plugin which can center an item: https://github.com/jsor/jcarousel/blob/center-plugin/src/center.js

Include the file after the jCarousel file and use it like:

$('.jcarousel').on('click', 'li', function() {
    $('.jcarousel').jcarousel('center', this);
});

Let me know if that works for you.

DiskHandler commented 11 years ago

This may be exactly what I need. I got it working using a way to keep up with the index, but with this, I can probably reduce my code count quite a bit and clean a lot of things up!

Thanks!!

DiskHandler commented 11 years ago

Any suggestions on recentering the carousel when resizing my page? I had it mostly working with my code, but using your example above worked better for my general navigation of the carousel that I used it instead - and of course, my code will no longer center on document resize.

THANKS in advance!

jsor commented 11 years ago

Keep track of the currently centered item.

$('.jcarousel')
    .on('click', 'li', function() {
        $('.jcarousel').jcarousel('items').removeClass('centered');
        $(this).addClass('centered');

        $('.jcarousel').jcarousel('center', this);
    })
    // This gets triggered on window resize
    .on('reloadend.jcarousel', function() {
        var centered = $('.jcarousel').jcarousel('items').find('.centered');
        $('.jcarousel').jcarousel('center', centered);
    });

Note: I released version 0.3.0-rc.1 yesterday which includes a BC break regarding event names. If you switch to this version, replace reloadend.jcarousel with jcarousel:reloadend.

DiskHandler commented 11 years ago

I tried that and it doesn't seem to work. Is there another plugin that I need? I really hate bothering you about this! Back in the day, I could code a C=64 like nobody's business, but I'm still a newb when it comes to javascript.

Your help on this so far has been invaluable!

jsor commented 10 years ago

Try to move the code from my last comment down after $('.jcarousel').jcarousel('center', (139), false);:

$(document).ready(function() {

  winSet();
  $('.jcarousel').jcarousel('center', (139), false);

$('.jcarousel')
    .on('click', 'li', function() {
        $('.jcarousel').jcarousel('items').removeClass('centered');
        $(this).addClass('centered');

        $('.jcarousel').jcarousel('center', this);
    })
    // This gets triggered on window resize
    .on('reloadend.jcarousel', function() {
        var centered = $('.jcarousel').jcarousel('items').find('.centered');
        $('.jcarousel').jcarousel('center', centered);
    });
});
DiskHandler commented 10 years ago

I placed it where you suggested and it still doesn't work when resizing. I'm still hopeful that we can get this figured out.