kenwheeler / slick

the last carousel you'll ever need
kenwheeler.github.io/slick
MIT License
28.37k stars 5.88k forks source link

Slide width incorrect with slides to show greater than 2 #2167

Open jimmymorris opened 8 years ago

jimmymorris commented 8 years ago

The slide width seems to be off a few pixels when slidesToShow is set to something other than the default. I added a border to the slides to help better show that the last slide's width is seemingly being cropped a few pixels.

I'm able to resize the browser and it'll show the full slide. Browser doesn't seem to matter.

jsFiddle link: https://jsfiddle.net/headunderwater/mz8knxw1/

Steps to reproduce the problem

  1. Have the slide number be greater that the slide to show.
  2. Set the slidesToShow option to any number greater than 2
Bjoernstjerne commented 8 years ago

The width of slick-slide is rounded, If rounded up these pixel are missing.

Maybe there is another way to calculate the slide width in %.

simeydotme commented 8 years ago

I don't think so, Mazzaker. However, it's been about a year now since browsers implemented crazy floating-point pixels... Slick should adapt and at least go down to 3 points of precision, I think?

simeydotme commented 8 years ago

@jimmymorris ... can I ask why you've minified the Slick resources in your Fiddle? I can not debug and manipulate them.

jimmymorris commented 8 years ago

@simeydotme Sorry about that! Not sure why it minified, I swore I forked it from http://jsfiddle.net/simeydotme/fmo50w7n/ need me to recreate?

Bjoernstjerne commented 8 years ago

In setDimensions:

if (_.options.vertical === false && _.options.variableWidth === false) {
    _.slideWidth = Math.ceil(_.listWidth / _.options.slidesToShow);
    _.$slideTrack.width(Math.ceil((_.slideWidth * _.$slideTrack.children('.slick-slide').length)));
}

Change first .ceil to .floor -> Fiddle

But this is not better either. I can try to calculate the width in % this should be more accurate in modern browser.

jonscottclark commented 8 years ago

Because we don't support IE8 for our clients, I've forked slick to use getBoundingClientRect to get the list width, and then removed calls to Math.ceil when determining slide width. That way the pixel width of the slides is fractional and exact. Here are my changes:

https://github.com/jonscottclark/slick/commit/180c9994a43633aae9db0fa0f2286489d1cd61f2

But this means you'd have to drop support for IE8 altogether.

Or.. you could potentially use this method of simulating getBoundingClientRect: https://github.com/zeroclipboard/zeroclipboard/blob/be4573579f9dbccad4280bd06e2e72c896fa5c66/ZeroClipboard.js#L124-L155

Then, the list width would be precise, meaning that your precise-width slides (with Math.ceil removed) would also fit perfectly.

I had the exact issue as OP and these changes solved it for me in all browsers (except IE8, of course)

dankram commented 8 years ago

@jonscottclark I tried using your fork and it made it more accurate by a pixel but still showing a sliver of the next slide, as well as now showing a pixel sliver on the left as well. Also I am only showing 1 slide per as opposed to multiple which is what this bug reports.

screen shot 2016-03-03 at 6 03 32 pm
jonscottclark commented 8 years ago

@dankram Since you mentioned it I went to verify, and you're right, there are more changes that are needed. The translate3d value, called targetLeft still gets floored/ceiled, and even if it's changed in the CSS to the exact value it needs to be (some fractional value), you'll still see slivers (on retina, at least).

Here's my test run where you can see the issues happening: https://jsfiddle.net/jonscottclark/aym5aaj5/3/

Forgive me for assuming I had a legit fix — I think it worked in the situation when I needed it, and I didn't see any negative side effects, and didn't test it out rigorously enough. Hm.

dankram commented 8 years ago

No worries! If you come up with a more comprehensive solution I'd love to see it. We are launching our product in a month. :fire:

max-ch9i commented 8 years ago

I have disabled transform3d on the list element. This resolved it to me.

.slick-slider .slick-list {
    transform: none;
}

It's more of a hotfix, of course.

pdg137 commented 8 years ago

Hello, since most people here are proposing a floating-point solution, I would like to share a suggestion I made in my closed duplicate issue. The issue is with the use of Math.ceil in the line that determines slideWidth (see it here). I suggest:

artuska commented 8 years ago

I have the same problem. Please just remove Math.ceil completely from the setDimensions method. Current slide width calculation with the Math.ceil is inaccurate if Slick container is fluid.

danieltj27 commented 7 years ago

I seem to be having the same issue with a vertical slider as it shows the next slide underneath the current slider by about 3 or 4 pixels which is quite off putting when you actually notice it.

Couldn't figure out a CSS method that would fix it other than forcing the height through some sort of external JavaScript means. Any help on this?

danyj commented 7 years ago

@leggomuhgreggo we had so many issues with this , can you please explain why is Math.ceil used in setDimensions. I removed it and there are no problems with widths and borders.

mikeploeger commented 6 years ago

+1

ewessolek commented 6 years ago

+1

CptKicks commented 5 years ago

Still open?

tindl88 commented 5 years ago

+1

chadlof commented 5 years ago
  .slick-list {
      overflow: inherit;
    }

Fixed it for me!!!

alenvuletic commented 2 years ago

Bump, still having issue with this one in 2021.

safarovitch commented 2 years ago

I also experienced this problem short time ago with combination of bootstrap. The problem was putting slick as direct child of bootsrap's .row couses miscalculation of width when slide count is not equal to slidesToShow option. Writing here so someone else facing this might find it useful.

m3ltman commented 1 year ago

If I'm correct the main issue here is that usually slick-list has lower width than its child slick-slide and it's around 1px with a variable amount of digits after the comma because of the Math.ceil() rounding. The easiest workaround solution that I came up with is that all of my slide content wrapped in the child of slick-slide called for instance cmp-carousel__slide-item, I just set the width of that child to calc(100% - 1px) !important(you can play with the number of pixels here) and everything seems to be working fine with the responsive carousel layout.

See screenshots: image image

ribeiroeder commented 5 months ago

I change the lines:

if (_.options.vertical === false && _.options.variableWidth === false) {
    _.slideWidth = Math.ceil(_.listWidth / _.options.slidesToShow);
    _.$slideTrack.width(Math.ceil((_.slideWidth * _.$slideTrack.children('.slick-slide').length)));
}

for

if (_.options.vertical === false && _.options.variableWidth === false) {
    _.slideWidth = (_.listWidth / _.options.slidesToShow).toFixed(1); // Replace Math.ceil to toFixed
    _.$slideTrack.width(_.slideWidth * _.$slideTrack.children('.slick-slide').length + 1); // Remove Math.ceil and Add +1px to ensure plenty of width    
}