leecade / react-native-swiper

The best Swiper component for React Native.
MIT License
10.42k stars 2.34k forks source link

Entire window shifts when updating from state change #578

Open Nickersoft opened 7 years ago

Nickersoft commented 7 years ago

Which OS ?

iOS

Version

Which versions are you using:

Expected behaviour

Updating the state and the subsequent re-rendering it causes shouldn't cause any of the existing slider layout to change (current index, positioning, etc.)

Actual behaviour

Updating the state when the swiper is not fullscreen causes the content and slides inside the swiper to shift drastically. Here's an example (all I'm doing is setting a timeout to change the text from "Hello Swiper" to "Hi Swiper" via setState() and it causes everything to go out of whack):

2017-09-19 12_28_42

How to reproduce it>

To help us, please fork this component, modify one example in examples folder to reproduce your issue and include link here.

Steps to reproduce

  1. Add the swiper to a view
  2. Make its parent view less than 100% in height
  3. Update any state object that would force the swiper or view to re-render

Any help would be greatly appreciated.. I'm trying to use this swiper in an actionsheet/modal at the bottom of the screen (about 500pt in height) and any re-rendering causes the entire layout to get messed up.

acrocat commented 7 years ago

I'm seeing the same issue on iOS, haven't tested on Android.

I also noticed the issue when hot-reloading the app by updating any file which doesn't contain the swiper. If you hot-reload the app by editing the file containing the swiper it corrects the issue.

I made nasty workaround to update the height of the swiper by 1 pixel when I change the state of my scene. I guess this forces the Swiper to re-render? Works in my case anyway.

Nickersoft commented 7 years ago

Hm, I'll have to try that. I would try my hand at a fix myself but the index.js file is so long I wouldn't know where to start. It's a peculiar issue to say the least.

Nickersoft commented 7 years ago

@acrocat So I actually figured out the code responsible for this issue. What's basically happening is the default dimensions the swiper uses is the default device width and height. This is set in the method initState(), which initializes the initial state of the component. Then, when the swiper is actually initialized, onLayout() is called, which computes the correct width and height of the parent container and sets it in the state. This is why when the slider first shows up it looks fine. Then, when the component needs to update componentWillReceiveProps() is called for some reason, which re-runs the original state init code. This resets the dimensions to that of the device, causing everything to go out of whack.

TL;DR seeing the width and height are already being set via the onLayout() call, I'd say it's safe to delete lines 228 and 229 which look like this:

initState.width = props.width || width
initState.height = props.height || height

I've tested this fix and it seems to work. My guess is your hack worked because adjusting the height was forcing onLayout() to be called a second time, correcting the dimensions. I can go ahead and throw this in a PR but my concern is I'm not sure it'll ever get merged (this repo doesn't seem super active), so I thought I'd post the fix here in case anyone else is impatient like me and just wants to fix it themselves.

timothykibler commented 7 years ago

Deleting those lines didn't seem to fix it for me :(

Nickersoft commented 7 years ago

Gah sorry to hear that @timothykibler :( I would try comparing the dimensions set in initState() to the ones calculated in onLayout(). For me, I noticed initState() was setting the width to be that of the device, while onLayout() was calculating a width much less than that. Which device/OS have you been testing on?

timothykibler commented 7 years ago

Tested on SE and 7, seems to have the same issue just showing in different ways. For example, on 7 initState() is calculating a height much larger than what onLayout() sets it to. Tested by just hardcoding a height into initState() just to see what would happen and it looks like it's almost working except that the initial index is wrong. The weird part is that swiping to the next index and then touching a button that changes state will either work as expected or cause the layout to change.

Touching a button on the initial, wrong index (it's actually the second component) works fine as well as subsequently swiping to the next indices + state changes, so I suppose I could just rearrange the components inside the Swiper.

Nickersoft commented 7 years ago

I wound up just forking this project, committing my "hack", and importing that in my package.json over the official repo. I wish the author would fix this.. it's a very troubling and annoying issue to have to work around.

AliceB08 commented 7 years ago

I had the same problem on my project, I added height={'100%'} in the props of Swiper:

screen shot 2017-09-27 at 15 02 06

it did the trick!

acrocat commented 7 years ago

@AliceB08 works perfectly, thanks

sylvain75 commented 7 years ago

@AliceB08 . I had an issue with the position of my arrow buttons and your solution works for me. Thanks

jketcham commented 7 years ago

I was having this issue too when rendered with both different width and height settings than the device's width and height. @Nickersoft's solution worked, but it also looks like pull request https://github.com/leecade/react-native-swiper/pull/548 fixed this. Might be able to close this now?

arribbar commented 7 years ago

https://github.com/leecade/react-native-swiper/pull/548 should fixed this issue. Can you try again with the version 1.5.13 ?

timothykibler commented 7 years ago

Not the author but tested with latest version and no dice.

@AliceB08 fix did not work either; however, setting loop={false} somehow fixed the issue for me.

core433 commented 7 years ago

I was using the Swiper in a view with a custom width and height which are smaller than the window size. Hardcoding width={myWidth} and height={myHeight} on the Swiper fixed my issue.

KonstantinGreat commented 5 years ago

set needed width and height <Swiper showsButtons={false} showsPagination={false} height={300} width={width}> {this.innerSliderList()} </Swiper> works for me

asadkaleememumba commented 3 years ago

@arribbar I am still getting this issue in version 1.6.0. Is there any permanent fix other than width and height? because that's not working for me.

lannisterks commented 3 years ago

Same issue here! I've updated the minimum version of Gradle of 16 to 21 and it crashed everything with Swiper in my App. I've try to downgrade the version of Swiper to 1.5.5 and I had problem with PagerView. With this I changed the index.js of the Swiper to correct directory and problem continue.

lannisterks commented 3 years ago

Works for me https://github.com/leecade/react-native-swiper/issues/1215#issuecomment-747454159