Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue when adding/removing cells from Flickity #14

Closed
Kinark opened this issue Nov 28, 2017 · 8 comments
Closed

Issue when adding/removing cells from Flickity #14

Kinark opened this issue Nov 28, 2017 · 8 comments

Comments

@Kinark
Copy link

Kinark commented Nov 28, 2017

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?

@Kinark Kinark changed the title Issue when adding cells to Flickity Issue when adding/removing cells from Flickity Nov 28, 2017
@theolampert
Copy link
Collaborator

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

@theolampert
Copy link
Collaborator

@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
Copy link

Ismael commented Dec 4, 2017

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
Copy link
Collaborator

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

@Ismael
Copy link

Ismael commented Dec 5, 2017 via email

@92arpitgoyal
Copy link

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

@theolampert
Copy link
Collaborator

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

@yaodingyd
Copy link
Owner

@Kinark 2.0.0 release fixes your problem. See demo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants