akiran / react-slick

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

Problem in rendering Slider with dynamic children #504

Closed chochihim closed 7 years ago

chochihim commented 8 years ago

If I render Slider as below:

<Slider {...settings}>
    {slides.map((slide) => (
        <div key={slide.index}>{slide.index}</div>
    ))}
</Slider>

nextProps.children.length in componentWillReceiveProps of class InnerSlider will always returns 1 which breaks the sliding logic.

Does Slider not expect dynamic children?

akiran commented 8 years ago

Can you reproduce the issue with this jsfiddle and provide link https://jsfiddle.net/kirana/20bumb4g/

chochihim commented 8 years ago

Sure. Here's the implementation of my described problem: https://jsfiddle.net/8qLhy7do/1/

If options slidesToShow and slidesToScroll are set to 1, it works fine: https://jsfiddle.net/8qLhy7do/2/

walston commented 8 years ago

Hey chochihim, it looks like you're not returning your <div> inside map()

try:

<Slider {...settings}>
    {slides.map((slide) => (
        return (<div key={slide.index}>{slide.index}</div>);
    ))}
</Slider>
chochihim commented 8 years ago

@walston no, mine is valid shorthand

In my jsfiddle example, the initial rendering is fine but not after clicking 'Add slide'.

anonmily commented 8 years ago

+1 Having this issue too. On version 0.14.5 for react-slick.

roik7 commented 8 years ago

+1

zhujunel commented 8 years ago

version 15.3.2 Cannot read property 'getBoundingClientRect' of null

hackhat commented 8 years ago

+1 same here. basically this line fails return elem.getBoundingClientRect().height || elem.offsetHeight; because elem is null. It comes from slickList.querySelector('[data-index="0"]').

szimek commented 8 years ago

@hackhat I had exactly the same issue and fixed by adding data-index attribute to my slide elements, more or less like this:

<Slider {...settings}>
  {slides.map((slide, index) => (
    <div data-index={index} key={index}>{index}</div>
  ))}
</Slider>

I've just started adding this library to my project, so I'm not sure if it won't cause some other issues later on.

hackhat commented 8 years ago

@szimek That's the same thing I did, but something doesn't feel right about it.

ShuaiShuaiguo commented 8 years ago

@hackhat you can try this: `<Slider {...settings}>

{slides.map((slide, index) => (
{index}
))}

`

hackhat commented 8 years ago

@ShuaiShuaiguo You mean removing the data-index={index} from the example above (see below code)? I don't think will work, because react-slick needs the data-index attribute.

<Slider {...settings}>
  {slides.map((slide, index) => (
    <div data-index={index} key={index}>{index}</div>
  ))}
</Slider>
akiran commented 8 years ago

Which version of react-slick are you using ?

I don't see this issue with latest release. Check this demo https://jsfiddle.net/8qLhy7do/6/

hackhat commented 8 years ago

@akiran I got the issue with "version": "0.14.5"

akiran commented 8 years ago

@hackhat Can provide a jsfiddle to replicate your issue ?

anonmily commented 8 years ago

@akiran I was able to reproduce the issue here:

https://jsfiddle.net/17xt8pxw/2/

It seems like if we start out with no children (e.g. this.state.slides = []), then there is a problem. When there is at least one children before the initialization of the slick slider (e.g. this.state.slides = [1]), then it still works.

Edit: Also, with further investigation, seems like the slick-track element remains empty.

ShuaiShuaiguo commented 8 years ago

@hackhat no, it needs a certain DOM element, just wraps the dynamic data by any DOM elements, not one of element.it works to me.

samplefrequency commented 8 years ago

+1 This is definitely a bug with 14.5. I pretty much have the same set up as the OP but utilizing a Redux store. My store initializes as empty then is populated later. React-slick shouldn't even be initializing with an empty array.

akiran commented 8 years ago

@samplefrequency Agreed. I will try to fix it soon

andricicezar commented 8 years ago

+1, setting manually data-index solved the problem.

samplefrequency commented 8 years ago

@andricicezar Still won't fix the issue if the array starts off empty and is later populated via AJAX or from a Redux store.

dvislov commented 8 years ago

Still not working :/

akiran commented 8 years ago

As a quick workaround, you can mount slider only if you have slides.

{slides.length > 0 ? <Slider>{slides}</Slider> : null }
dvislov commented 8 years ago

@akiran I solve this problem almost like you: {this.state.loading ? <Loader> : <Slider>{slides}</Slider> }

Thanks for react-slick!

ypyang237 commented 7 years ago

@akiran {slides.length > 0 ? <Slider>{slides}</Slider> : null } didn't work for me.

Any updates?

samplefrequency commented 7 years ago

Replace 'slides' with the name of the array that contains the slide items. For example, instead of slides.length use yourArray.length

On Nov 14, 2016 18:08, Pam notifications@github.com wrote:

@akiranhttps://github.com/akiran {slides.length > 0 ? {slides} : null } didn't work for me.

Any updates?

You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/akiran/react-slick/issues/504#issuecomment-260515785, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AHrj26kslUdhM1D511Y5Em8b_K9x3dGrks5q-QYegaJpZM4KMS5h.

ypyang237 commented 7 years ago

Thanks for the quick response @samplefrequency

Unfortunately it still doesn't work. Like you, my array starts off empty and is later populated via AJAX called after the Redux store has been updated (since it needs the new data from the redux store to make the call).

As you suggested, I have replaced it with my own array, and I've also confirmed the length of said array - this.renderGallery(listing).length shows 7 when rendering it directly below my images.

The below is my renderGallery handler which I call when rendering

renderGallery() {
    if (this.props.listingGalleryImages.data !== undefined) {
      const galleryAssets = this.props.listingGalleryImages.data;
      const images = galleryAssets.map((asset, index) => {
       return (
          <div data-index={index} key={index}><img key={asset.id} src={asset.url} /></div>
        );
      });
      return images;
    }
  }

And inside the render:

<div className="container" >
    {this.renderGallery(listing) !== undefined && this.renderGallery(listing).length > 0 ?      
       <Slider{...settings}>
            <div>{this.renderGallery(listing)}</div>
       </Slider> : null }
    {this.renderGallery(listing) !== undefined ? this.renderGallery(listing).length : null}  
</div>

I don't know if this is clear enough, happy to clarify further.

Btw, I'm using "react-slick": "^0.14.5", and also have the style sheet included, since initially my Isomorphic / universal React app didn't work without. https://github.com/akiran/react-slick/issues/273

ypyang237 commented 7 years ago

Solved it. Writing it here for ppl who run into this in the future :

Had help from a colleague and he pointed out that I needed only to take out the div from this line

<Slider{...settings}>
   <div>{this.renderGallery(listing)}</div>
</Slider> : null }

Apparently, if I leave it there, the Slider will pick up that one big

and count that, which means it will not render according to the other many
s within it.

Side note: I didn't even need the data-index in the .map, though the ternary is still needed for fallback purposes.

Code now looks like this:

 {this.props.listingGalleryImages.loaded === true &&
   <div className="container">
       {this.renderGallery(listing).length > 0 ? 
        <Slider {...settings}>{this.renderGallery(listing) </Slider> : null }
   </div>
 }
anonmily commented 7 years ago

@ypyang237 Just as a side note, instead of the ternary

this.renderGallery(listing).length > 0 ? 
        <Slider {...settings}>{this.renderGallery(listing) </Slider> : null

You could actually use && to make it shorter:

( this.renderGallery(listing).length > 0 ) && <Slider {...settings}>{this.renderGallery(listing) </Slider>
justin3737 commented 7 years ago

@szimek Thanks, this works for me! 👍

wldeveloper commented 7 years ago

@anonmily thanks i have the same problem as you , it works!

tanmynguyen commented 7 years ago

If {...settings} you set lazyLoad: true will not work. with data.length < settings.slidesToShow !!!

edwardaa7 commented 7 years ago

Doesn't work with react components

https://jsfiddle.net/8qLhy7do/21/

NamanAulakh commented 7 years ago

I also stuck in this issue, but now it's working fine for me:

<Slider
   ref={c => this.slider = c}
   {...settings}
   afterChange={this.changeState}>
   {content.map((item, index) => <p key={index}>{item}</p>)}
</Slider>

I hope it helps.

akiran commented 7 years ago

This issue is fixed in master

rolandokaizer commented 7 years ago

When will this fix be added to NPM?

nabendukarmakar commented 7 years ago

Still facing the same issue with this release -- https://github.com/akiran/react-slick/releases/tag/0.15.0

bnolens commented 7 years ago

Make sure data-index is an integer. And if your slide is a component, make sure to set the data-index attribute to the root element of your slide component.

Your slide component could look something like this:

const Slide = ({ dataIndex }) => {
    return(
        <div data-index={dataIndex} key={dataIndex}>
            <p>Slide {dataIndex}</p>
        </div>
    )
}

This fixed it for me.

niklasp commented 5 years ago

How is it possible to do what @ypyang237 did first? I need a slider that has that setup

<Slider{...settings}>
   <div><div className="inner">{this.renderGallery(listing)}</div></div>
</Slider> : null }

And slick only to pick up like .inner as slides. In original slick-slider there is the slide attribute, which tells slick from which selectors to create the slides. It seems not implemented in react-slick?

hpachy commented 3 years ago

Hello I still have the same problem... what the hell is going on... look at the width size...

my code :

<div>
      <h2>Custom Arrows</h2>
      <Slider {...settings}>
       <div>
          <h3>1</h3>
        </div>
        <div>
          <h3>2</h3>
        </div>
        <div>
          <h3>3</h3>
        </div>
        <div>
          <h3>4</h3>
        </div>
        <div>
          <h3>5</h3>
        </div>
        <div>
          <h3>6</h3>
        </div> 
      </Slider>
    </div>
Capture d’écran, le 2020-12-29 à 17 08 03
kevin700brands commented 3 years ago

I still have a problem at rendering dynamic children.

My config:

<Slider {...settings}>
{data.map((item, idx) => <div data-index={idx} key={idx}>{item}</div>)}
</Slider>

I've tried all tricks and tips and "solutions" above but nothing seem to work.

"react-slick": "^0.28.1"

Xperience0501 commented 3 years ago

@kevin700brands You might have missed importing the CSS files:

just try adding the two lines of code in ur app.js file:

import "slick-carousel/slick/slick.css";
import "slick-carousel/slick/slick-theme.css";
fws-nenadknezev commented 1 year ago

I am using version 0.29.0 and still have issues regarding broken slider. Initially everything is fine, when i load the page, but when i move to another page and return, all my data are there, I put log to track changes, but my slider is not visible... Can anyone relate to this behavior? I put all the changes mentioned here bit it does not work...

My Settings object is:

const settings: Settings = { dots: false, infinite: false, speed: 1000, slidesToShow: 1, slidesToScroll: 1, arrows: true, prevArrow:

, nextArrow:
, };

My component code is:

return (

    <div className="map-slider-container">

        <div className="map-slider">

            <div className="map-slider__content">

                <Slider {...settings} ref={sliderRef}>
                    {propertyCards?.map((card, index) => {
                        return (
                            <div data-index={index} key={index} className="map__card">
                                <div className="map__card-img">
                                    <Image
                                        src={card?.image! ?? CardImage}
                                        width={200}
                                        height={200}
                                        alt="image"
                                    />
                                </div>
                                .
                                .
                                .
fws-nenadknezev commented 1 year ago

When I removed first container div, slider now works, its not broken, but my .css is now... Why is this happening?