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

Edit cards and update index #228

Open
pierregoutheraud opened this issue Feb 22, 2019 · 10 comments
Open

Edit cards and update index #228

pierregoutheraud opened this issue Feb 22, 2019 · 10 comments

Comments

@pierregoutheraud
Copy link

pierregoutheraud commented Feb 22, 2019

I am following on #153 because there is something not clear to me about cardIndex prop.
Passing cardIndex re-renders the cards but it does not seem to fix the index problem.

My swiper initializes with a list [1, 2, 3].

state = { 
  list: [1, 2, 3] 
}

As soon as I swiped card 1, I am removing it from the list.
(In my app, this is needed and is done through redux state)

handleSwiped = () => {
  this.setState(prevState => ({ list: prevState.list.slice(1) }));
}

Causing a re-render of the swiper with the updated list [2, 3].

The thing is that after the first swipe and the re-render, the swiper displays 3 instead of 2.

This is normal because the internal current card index incremented from 0 to 1, but I would like to get around that.
So I basically need the current card index to stay at 0 after the re-render and I do not see how this is possible with cardIndex prop... I tried setting it at 0 but it does not change anything.

An obvious solution is just not mutating the cards prop.
Another solution is setting a key={list.length} to the Swiper so that I re-mount the Swiper each time I swipe... which is not great.
Another solution is to prevent re-rendering of the swiper, so rendering once at the start, removing stuff from the cards and not re-rendering the swiper. The problem with that is that onSwiped only gives you back an index so you would have to manage the difference between the swiper index and your updated cards.
Would be cool to have cardIndex and cardData returned from onSwiped.

So how would you make that work ?

@pierregoutheraud pierregoutheraud changed the title Edit cards prop and update index Edit cards and update index Feb 22, 2019
@lightninglu10
Copy link

@pierregoutheraud don't remove it from the list. when you swipe, just move the card index

@pierregoutheraud
Copy link
Author

pierregoutheraud commented Feb 23, 2019

@lightninglu10 Unfortunately it is not that easy and it is what I meant by "An obvious solution is just not mutating the cards prop." Also, moving the card index is done internally by the swiper.

I am showing here an example but in my app the cards are stored in a redux state and I need to keep track of the swiped left cards so that next time it initializes it does not display the previously swiped left cards.

@matthewfbenjamin
Copy link

@pierregoutheraud Did you find a solution to this?

@webraptor
Copy link
Contributor

@pierregoutheraud @matthewfbenjamin
There's no reason why you should remove a swiped card from the list.
The swiper does a lot of work so you don't have to. Simply modifying the props after each swipe will degrade your performance because the swiper will re-render if the cards prop changes.

I fail to see the reason why you would do what you're describing. Especially if your swiper is configured with infinite false.

@matthewfbenjamin
Copy link

In my scenario what's happening is that I have two screens where users can make the same edits. One of the screens is the Swiper and the other is a list view. They both perform the same actions but I need parity across them. So when a user makes a change on the ListView and switches the tab back to the Swiper they should see the same array. This would be easy, but the way a tab navigator works, in react-navigation, is that it doesn't re-mount the component. So instead of reseting the Swiper, I'm setting a local array to feed into the Swiper that's resetonFocus.

willFocus = () => {
        const state = {cardIndex: 0};
        if (this.props.reduxCards.length !== this.state.cards.length) {
          state.cards = this.props.subscriptions
        }
        this.setState(state)
  }

I wonder if a good feature would be to feed an array of indexes to skip over since the Swiper already has a setCardIndex function.

Some psuedocode would be:

const skipArr = [ 1, 3, 5 ]
if (skipArr.includes(currentIdx)) setCardIndex(currentIdx + 1)

So instead of changing the array that's fed into the Swiper the ListView (in my case, but could be any other screen that affects the array for other people), I'd keep track of a list of edited objects. I guess I could do this on my side also but using the jumpToCardIndex method provided but I'd imagine that would look fairly jumpy.

@webraptor
Copy link
Contributor

@matthewfbenjamin
We have the same in our app. The proper way to fix this is to make sure that you base your list view / swiper on the same store / state. Changing it in either place will have the same results. What you're describing is adding more code to achieve a goal when you should remove code and let RN do its thing.

What are you using for state management?

@matthewfbenjamin
Copy link

We use redux and we are using the same state across the two views. I think the issue is that when you edit an item in the array (swipe right on the card) that will change the status of the item from New to Used (or whatever it is). On that change, the item gets moved to a new Used item array and is removed from the New item array.

@webraptor
Copy link
Contributor

If the item in the array is moved all-together the swiper will re-render and start from the beginning and you shouldn't have any issues.

@matthewfbenjamin
Copy link

Yeah, it is moved, but that goes against the issue that was opened. I'm splicing one array and pushing to another array. The spliced array is the one being shown, though.

@webraptor
Copy link
Contributor

That's because it's not really an issue. What @pierregoutheraud was describing is intended behaviour, not a bug within the swiper.

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

4 participants