akiran / react-slick

React carousel component
http://react-slick.neostack.com/
MIT License
11.72k stars 2.1k forks source link

variableWidth on mobile device break the slider #1789

Open roux1max opened 4 years ago

roux1max commented 4 years ago

Hey guys ! Thanks for this amazing plugin, it really rocks !

There seems to be a problem in the way the width of the slick-track div is calculated on mobile. This causes the slider to completly break on the first change of slide because the translation is also wrongly calculated.

My settings :

{
    infinite: true,
    centerMode: true,
    slidesToShow: 1,
    slidesToScroll: 1,
    variableWidth: true,
    draggable: false,
    speed: 1000
}

This issue can be replicated when you switch to mobile view on https://codesandbox.io/s/react-slick-playground-1huh6.

The funny thing here is that if you just change the setting slidesToShow to 0, a javascript error is displayed but the width is rightly calculated!

Anyway, let me know if you find anything!

roux1max commented 4 years ago

Hey!

Just wanted to known if anyone saw this and has any idea. Otherwise I can try to investigate this myself and maybe create PR or at least post what I found.

Thanks

akiran commented 4 years ago

@roux1max Please send a PR

roux1max commented 4 years ago

Ok, I identified the problem: in the innerSliderUtils.js there is a method called getTrackCSS that is called to refresh all the props and state variables of the slider. This method does not take into account the flag variableWidth so every time the state of the component is updated, the width of the Track component is calculated but only by doing this : getTotalSlides(spec) * spec.slideWidth;.

It only worked in desktop mode for me because the slideWidth is initialized with the initial size of the list (which is the width of its container), and my slide were not wider than the width of the container (i.e the viewport in my case). So the calculated width for the Track component was just enormous (way bigger than the sum of my slides).

But in mobile view, all my slides are wider than the screen so the previous calculation did not work.

I think we have to use the same approach than the one in ssrInit to calculate the width in the getTrackCSS method. I am just a bit afraid of the impact in term of performance.

@akiran let me know what you think :)

roux1max commented 4 years ago

Hey ! Any update on that PR :) ?

davidc05 commented 4 years ago

Any update on this issue?

roux1max commented 4 years ago

Still waiting on my PR to be merged

phal0r commented 4 years ago

+1 for feedback

AlexKotov commented 4 years ago

+1

elishaterada commented 4 years ago

+1

It works well on the desktop where the width of the slider is defined as part of layout but fails on mobile where I let the slider take up 100% of the available width. If there is no fix via code I'd love to know if anyone came up with a workaround.

roux1max commented 4 years ago

@elishaterada as I said in the thread, I just used the value slidesToShow: 0 when in mobile and it bypasses the calculation of the width.

phal0r commented 4 years ago

@roux1max I tried to set slidesToShow: 0, but it leads to

Warning: Infinity is an invalid value for the width

Then it is broken for Desktop and Mobile unfortunately.

bymoe commented 4 years ago

Any news?

rapritchard commented 3 years ago

This still seems to be a bug, anyone found a workaround? I tried slidesToShow: 0 but that just broke react-slick even more.

phal0r commented 3 years ago

Not a real workaround, but I dropped react-slick completely in my project and switched to https://swiperjs.com/, which works perfectly well for me.

oscarsidebo commented 3 years ago

@phal0r did you get variableWidth to work with swiper js?

phal0r commented 3 years ago

@oscarsidebo This is my Swiper integration:

<Swiper
          spaceBetween={15}
          slidesPerView="auto"
          centeredSlides={true}
          centeredSlidesBounds={false}
          loop={slides.length > 1}
          navigation
        >
          {slides}
        </Swiper>

The slides look like so:

<SwiperSlide style={{ width: 'auto' }} zoom={true}>
          <img src={url} className={classes.pictures} />
</SwiperSlide>

This works on mobile/desktop with variable width, centering the active slide and an infinite loop.

akshatdizayn commented 11 months ago

I have also the same issue with variableWidth but in my case by giving {adaptiveHeight: true} works completely fine.

const settings = { dots: false, arrows: false, speed: 750, slidesToShow: 1, variableWidth: true, adaptiveHeight: true, centerMode: true, slidesToScroll: 1, };