yaodingyd / react-flickity-component

A React.js component for using @desandro's Flickity
314 stars 51 forks source link

Issue when adding/removing cells from Flickity #14

Closed Kinark closed 6 years ago

Kinark commented 6 years ago

I'm trying to add cells dynamically by mapping a state object and remapping after an ajax call ends:

export default class Reviews extends Component {
   constructor(props) {
      super(props);
      this.state = {
         reviewjson: [
            {
               id: 0,
               name: 'Loading...'
            }
         ]
      };
   }
   componentDidMount() {
      getJSON(this.props.dataURL, (err, data) => {
         if (err !== null) {
            alert('error');
            }
         } else {
            this.setState({reviewjson: data.reviews});
         }
      });
   }
   componentDidUpdate() {
      console.log('foi');
   }
   render() {
      const flickityOptions = {
         cellSelector: '.review',
      }
      return (
         <div className="carousel-holder">
            <Flickity options={flickityOptions} reloadOnUpdate >
               {this.state.reviewjson.map(i => (
                  <Cell className="review" key={i.id} name={i.name} />
               ))}
            </Flickity>
         </div>
      )
   }
}

However, what I get rendered after the ajax call is something like this:

<div class="flickity-enabled is-draggable" tabindex="0">
   <div class="flickity-viewport" style="height: 175px; touch-action: pan-y;">
      <div class="flickity-slider" style="left: 0px; transform: translateX(27.43%);">
         <Cell class="review">...</Cell>
      </div>
   </div>
   <Cell class="review">...</Cell>
   <Cell class="review">...</Cell>
   <Cell class="review">...</Cell>
   <Cell class="review">...</Cell>
   <Cell class="review">...</Cell>
</div>

I mean, if I have more cells in the remap than I had before, they're added outside the viewport and the slider. And if I have less cells in the ajax call than I had before, I got an error:

Uncaught DOMException: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.

Is it a bug or am I doing something wrong?

theolampert commented 6 years ago

hey @Kinark I'll take a look into this and get back to you.

theolampert commented 6 years ago

@Kinark So I've been thinking of some ways to allow this package to use dynamic children but haven't come up with anything super useful. Flickity expects cells to be added and removed with the .append / .insert and .remove methods. It also expects these to be native DOM elements not react elements so the children can't be passed to these methods internally without first being converted to DOM elements. So my thoughts are either it internally needs to create the DOM elements from children and pass them to the flickity methods or alternatively I just expose these methods as props on the component and let end users handle this. Both options don't sound super appealing. Any ideas? / thoughts?

Ismael commented 6 years ago

Forcing the user to manage the elements "by hand" IMO breaks the beauty of React. I'd expect the component to "just work". Whatever black magic is required should be contained in the component.

theolampert commented 6 years ago

@Ismael yeah I tend to agree, if I put up a test branch would you be willing to help out testing this?

Ismael commented 6 years ago

Sure

92arpitgoyal commented 6 years ago

@theolampert I can also help you test this, are you working on this?

theolampert commented 6 years ago

@92arpitgoyal @Ismael I'm hoping #16 will solve this and a few other issues.

yaodingyd commented 6 years ago

@Kinark 2.0.0 release fixes your problem. See demo