alexbrillant / react-native-deck-swiper

tinder like react-native deck swiper
ISC License
1.56k stars 465 forks source link

Button on card triggers setState, but does not re-render #192

Closed baophamtd closed 5 years ago

baophamtd commented 5 years ago

Hello everyone,

I have used this plugin for my project and really like it. Thank you for your work. I'm trying to implement couple transparent buttons on top of the card to go back and forth in an image array for each card (much like Instagram does it). The button does get called. However, the setState in the call does not make the UI to re-render. I have looked up what might cause it and used the solutions from other answers to trigger re-render and they work fine, but within the Card, it does not work for some reasons. Here are my codes:

return (
      <View style={{ flex: 1 , backgroundColor: '#efede6', flexDirection: 'column'}}>
        <View style={{ flex: 1 ,backgroundColor: 'blue'}}>

        </View>
        <View style={{ flex: 8 , backgroundColor: 'red'}}>
          <Swiper
              ref="deck"
              cards={this.state.restaurants}
              renderCard={(card) => {
                return(
                  <ImageBackground
                    style = {styles.card}
                    source = {{uri:card.photos[this.state.currentImageIndex]}}
                    borderRadius = {20}
                  >
                  {this.renderImageIndexes()};
                  <View
                    style = {styles.imageControllerButtonContainer}
                  >
                    <TouchableOpacity
                      style = {styles.imageControllerButton}
                      onPressOut = {()=>this.previousImage()}/>
                    <TouchableOpacity
                      style = {styles.imageControllerButton}
                      onPressOut = {()=>this.nextImage()}
                      hitSlop={{ top: 12, left: 36, bottom: 0, right: 0 }}/>
                  </View>
                  </ImageBackground>
                )
              }}
              onSwipedLeft={(cardIndex) => {
                this.previousRestaurant(cardIndex);
              }}
              onSwipedRight={(cardIndex) => {
                this.nextRestaurant(cardIndex);
              }}
              onSwipedAll={() => {console.log('onSwipedAll')}}
              cardIndex={0}
              backgroundColor={'#efede6'}
              showSecondCard = {true}
              stackSize= {this.state.restaurants.length}
              disableLeftSwipe = {this.state.firstRestaurant}
              goBackToPreviousCardOnSwipeLeft = {true}>

            </Swiper>
        </View>

      </View>
);

And here is the call to setState:

nextImage(){
    console.log("Get called!!!");
    if(this.state.currentImageIndex < 10){
      this.setState({
        currentImageIndex: this.state.currentImageIndex + 1,
      });
      console.log(this.state.currentImageIndex)
    }
  }

  previousImage(){
    if(this.state.currentImageIndex > 0){
      this.setState((state, props) => ({
        currentImageIndex: this.state.currentImageIndex - 1,
      }));
    }
  }

Appreciate any help

webraptor commented 5 years ago

Same issue as https://github.com/alexbrillant/react-native-deck-swiper/issues/189 Already answered there.

In short, you're modifying the state of the parent component that contains the swiper, not the properties that the swiper receives. Therefor, after the initial render the swiper has no clue that something's changed in order to trigger a re-render.

If you want any re-renders to occur, look into the shouldComponentUpdate method to understand when swiper will re-render.

With respect to your code, in order to make it work, since the image carousel is part of the card, the index of the image shown should be part of the card. In your case, that's within each object you have in the restaurants array in the state. This means that currentImageIndex should be an attribute of a restaurant, and depending of the card index in the deck, when tapping next/previous you would increment / decrement currentImageIndex of the restaurant[cardIndex] object in your state.

baophamtd commented 5 years ago

Same issue as #189 Already answered there.

In short, you're modifying the state of the parent component that contains the swiper, not the properties that the swiper receives. Therefor, after the initial render the swiper has no clue that something's changed in order to trigger a re-render.

If you want any re-renders to occur, look into the shouldComponentUpdate method to understand when swiper will re-render.

With respect to your code, in order to make it work, since the image carousel is part of the card, the index of the image shown should be part of the card. In your case, that's within each object you have in the restaurants array in the state. This means that currentImageIndex should be an attribute of a restaurant, and depending of the card index in the deck, when tapping next/previous you would increment / decrement currentImageIndex of the restaurant[cardIndex] object in your state.

Thank you for your response. From what I understand, the swiper only re-renders if properties cardIndex or cards change. Is this correct? I've tried it in couple ways. With your suggestion, I have added a currentImage attribute to my restaurant object. Thus, in order to trigger the re-render, I have modified the cards as follows:

nextImage(){
    let tempRestaurant = this.state.restaurants[this.state.currentRestaurantIndex];
    tempRestaurant.currentImage = tempRestaurant.photos[this.state.currentImageIndex + 1].url;
    console.log(tempRestaurant);
    let tempRestaurants = this.state.restaurants.slice();
    tempRestaurants[this.state.currentRestaurantIndex] = tempRestaurant;
    if(this.state.currentImageIndex < 10){
      this.setState({
        currentImageIndex: this.state.currentImageIndex + 1,
        restaurants: tempRestaurants,
      });
    }
  }

I logged the restaurant item in the array and it switches the currentImage, but no re-render happens. On the other hand, if I simply do:

if(this.state.currentImageIndex < 10){
      this.setState({
        currentImageIndex: this.state.currentImageIndex + 1,
        restaurants: this.state.restaurants.slice(0,3), //this just for testing re-render
      });
      console.log(this.state.currentImageIndex)
    }

then re-render happens.

I don't think I can use cardIndex property to trigger re-render as this is the index for the cards, not for the image carousel within the card itself.

Any suggestions on this? Thank you very much for your help.

webraptor commented 5 years ago

@baophamtd that wouldn't work and it looks really hard to maintain.

  1. your renderCard method doesn't use the cardIndex, which the swiper passes back as parameter. renderCard(card,cardIndex)
  2. your renderCard method is anonymous. You could move it a separate method
  3. when said function gets called, you'd setState({cardIndex}) so that your parent's component cardIndex coincides with whatever's on the swiper.
  4. within the swiper properties you set, replace cardIndex={0} with cardIndex={this.state. cardIndex={0}}
  5. When constructing the restaurants objects that get pushed into the this.state.restaurants array make sure you initialise a currentImageIndex with 0.
  6. Your source = {{uri:card.photos[this.state.currentImageIndex]}} would become source = {{uri:card.photos[card.currentImageIndex]}}
  7. Next/Previous image is as below, or similar:
nextImage(){
  const { restaurants, cardIndex } = this.state;
  this.setState({
    restaurants: [
      ...restaurants.slice(0, cardIndex), // if not familiar with this look for spread operator
      { 
        ...restaurants[cardIndex], 
        currentImageIndex: restaurants[cardIndex].currentImageIndex+1 
      }, 
      ...restaurants.slice(cardIndex+1, restaurants.length)
    ]
  });
}
  1. Of course, when you increment decrement stuff, you need to check that you won't go out of bounds.
baophamtd commented 5 years ago

@baophamtd that wouldn't work and it looks really hard to maintain.

  1. your renderCard method doesn't use the cardIndex, which the swiper passes back as parameter. renderCard(card,cardIndex)
  2. your renderCard method is anonymous. You could move it a separate method
  3. when said function gets called, you'd setState({cardIndex}) so that your parent's component cardIndex coincides with whatever's on the swiper.
  4. within the swiper properties you set, replace cardIndex={0} with cardIndex={this.state. cardIndex={0}}
  5. When constructing the restaurants objects that get pushed into the this.state.restaurants array make sure you initialise a currentImageIndex with 0.
  6. Your source = {{uri:card.photos[this.state.currentImageIndex]}} would become source = {{uri:card.photos[card.currentImageIndex]}}
  7. Next/Previous image is as below, or similar:
nextImage(){
  const { restaurants, cardIndex } = this.state;
  this.setState({
    restaurants: [
      ...restaurants.slice(0, cardIndex), // if not familiar with this look for spread operator
      { 
        ...restaurants[cardIndex], 
        currentImageIndex: restaurants[cardIndex].currentImageIndex+1 
      }, 
      ...restaurants.slice(cardIndex+1, restaurants.length)
    ]
  });
}
  1. Of course, when you increment decrement stuff, you need to check that you won't go out of bounds.

Thank you very much for your help. I've managed to get it to work. However, I don't think I quite understand why it works the way it does (sorry I'm newbie to react native). Especially at this block:

restaurants: [
      ...restaurants.slice(0, cardIndex), // if not familiar with this look for spread operator
      { 
        ...restaurants[cardIndex], 
        currentImageIndex: restaurants[cardIndex].currentImageIndex+1
      }, 
      ...restaurants.slice(cardIndex+1, restaurants.length)
    ]

As fas as I understand, you're reassembling the state array so that it can trigger the render. However, at this line currentImageIndex: restaurants[cardIndex].currentImageIndex+1, it would not work without it and I don't get why because between this { } should just be a restaurant object. How can you edit its currentImageIndex that way?

In addition, as you mentioned, I have moved my renderCard method out of the Swiper, but it only renders the card with this format:

renderCard = (card, cardIndex) =>{ 
          return (...)
}

Inside Swiper:
renderCard={
                this.renderCard
}

but not this format:

renderCard (card, cardIndex) { 
          return (...)
}

Inside Swiper:
renderCard={(card, cardIndex) =>{
                this.renderCard(card, cardIndex);
}}

Do you know why that is so?

webraptor commented 5 years ago

In order to understand how the restaurants array is rebuilt, as mentioned, research the js spread operator

With regards to the renderCard issue, you can either pass the function (as you did in option one), or call the function (as you did with option two). However, on your second version, this.renderCard(card, cardIndex) is part of an anonymous method and returns nothing. The renderCard method returns the (View) object but then you don't return it further within the anonymous method. The correct version two would be:

renderCard={(card, cardIndex) =>{
  return this.renderCard(card, cardIndex);
}}

If everything's clear now, please close the bug. Cheers!

baophamtd commented 5 years ago

Thanks a lot @webraptor !

codingdeep commented 3 years ago

Really life saviour solution