oliviertassinari / react-swipeable-views

A React component for swipeable views. :snowflake:
https://react-swipeable-views.com/
MIT License
4.46k stars 480 forks source link

Change index on transition end instead of on touch end #175

Open vincentsels opened 7 years ago

vincentsels commented 7 years ago

This is a follow-up to #171. A new onTransitionEnd callback was added to allow delaying any cpu-intensive rendering, certainly in scenarios where views are virtually rendered through the slideRenderer callback, until after the swipe animation is completed, to avoid a jerky animation.

However, slideRenderer will still be called 'on touch end', along with updating the previous & current index on the state, so this doesn't change anything to that scenario.

A possible alternative solution I see would be to be able to configure whether you want the component's current & previous index state to be changed on touch end or on transition end - along with calling both the slideRendered to render the next slide, and trigger a callback. So in this scenario; you'd only have one callback; but the moment at which it will be called will be different.

Some more background info on why I'm using the callback in my situation (maybe I'm taking a wrong approach): I would like the selected view to be set by my application state, and reflected in the application's route. Concretely: swiping through the views should, through a callback, update some 'selected view' property my store, which in turn would update the route and delegate this 'selected view' state to the component which renders the swipeable views. Because the new 'selected view' would be the same as its internally 'selected view', no re-render would be necessary.

oliviertassinari commented 7 years ago

@vincentsels Thanks for raising this issue. That's a very good starting point to work on some further improvements for the virtualized HoC.

be able to configure whether you want the component's current & previous index state to be changed on touch end or on transition end

There is one missing information that we have to take into account. The virtualized HoC is increasing the displayed window by 1 when users are swiping forward at the onTouchEnd callback. This allows users to swipe as fast as possible. For some reason, there is a limitation for doing it when swiping backward (#152). I plan to fix the issue and to have the same behavior when swiping backward.

However, you are right, there is room for improvement. E.g. on the Demo8 of the docs, slideRenderer is called 7+6 times when swiping forward and 6+6 times when swiping backward.

I'm not sure what the best strategy is. We have two areas of improvement:

vincentsels commented 7 years ago

Good point about being able to swipe fast, before animation ends. Hadn't thought about that at all :/.

Not sure if I fully understand those two dimensions you refer to though.

I suppose ideally we should render a configurable number of 'excess' views on animation end, and only render a view ad-hoc during an animation when it was not yet rendered but would appear inside the viewport.

Alternatively, you could work with some sort of placeholder which gets displayed for unrendered views in the viewport, and always only start actually rendering them on animation end. This seems like a really big change though.

steezeburger commented 7 years ago

@vincentsels curious if you ever got any type of solution working.

I have the exact same use case and have tried all kinds of crazy stuff like grabbing prop values from my swipeable components, setting slideIndex on my component (not in state because updating state in slideRenderer causes issues), and using that in onTransitionEnd to grab the current card by index and use the cards data-prop to grab my currentCategory then calling a redux action in onTransitionEnd, but the indices don't seem to be lining up properly.

vincentsels commented 7 years ago

@steezeburger: No - I had to switch back to onChangeIndex.

Bobgy commented 6 years ago

Got into the same problem. I did something similar to @steezeburger has described, as setting a slideIndex on my own component, and managed to get it working mostly fine with some minor issues, but I don't yet have time to tidy up the code and share it. just FYI that's feasible.

edit: add some context of my use-case, I have a tab that shows current index. If I change index at onTouchEnd, the tab has to re-render and it's slow enough to cause a jank on the transition animation.

so instead, I want the index change happening at transition end, so that the re-render would happen when no animation is going on. That makes the jank not noticeable.