leecade / react-native-swiper

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

onIndexChanged not called, wrong screen rendered #503

Open ivpusic opened 7 years ago

ivpusic commented 7 years ago

Version

Which versions are you using:

Expected behavior

  1. onIndexChanged should be called when I swipe
  2. first View should be rendered as the first screen

Actual behavior

  1. not called
  2. third View (with "And simple" message) is rendered as first

Code example

export default class AppIntro extends PureComponent {

  onSwipe = (index) => {
    console.log('index changed', index);
  }

  render() {
    return (
      <Swiper style={styles.wrapper} onIndexChanged={this.onSwipe}>
        <View style={styles.slide1}>
          <Text style={styles.text}>Hello Swiper</Text>
        </View>
        <View style={styles.slide2}>
          <Text style={styles.text}>Beautiful</Text>
        </View>
        <View style={styles.slide3}>
          <Text style={styles.text}>And simple</Text>
        </View>
      </Swiper>
    );
  }
}
sethkrasnianski commented 7 years ago

I am experiencing the same issue.

adam-jr commented 7 years ago

Also experiencing this. I'm seeing onIndexChanged get called after I swipe around a bit.

kyangy commented 7 years ago

In onIndexChanged={(index) => this.setState({index})} only fires once after the first swipe. The state only changes once when it should be changing each swipe.

GertjanReynaert commented 7 years ago

I have the same issue, when debugging it seems like the internalState.offset is empty when calling updateIndex after a swipe.This causes that diff to be NaN and that causes the internal index not being updated, which subsequently causes onIndexChanged not being called.

I don't really know what the internalState.offset should be, so can't really fix the issue here, but I hope this can help the maintainers to fix the issue

sethkrasnianski commented 7 years ago

I solved this (implemented a workaround) by adding a ref to the Swiper and calling scrollBy to set the index back to its intended value.

<Swiper
  showsButtons={false}
  showsPagination={false}
  autoPlay={false}
  ref={(s: React.Element<Swiper>) => this.swiper = s}
>
// ...
</Swiper>

And in componentDidMount:

componentDidMount() {
  this.swiper.scrollBy(0);
}
stueynet commented 7 years ago

@sethkrasnianski this solved the issue of starting on the right slide but my pagination dots still don't move. Thanks for that though!

sethkrasnianski commented 7 years ago

@stueynet same... I tried passing a custom slide index with the index prop, but there's a bug where the index prop is only read on render and not if a state/props change

arribbar commented 7 years ago

Guys, sorry, it seems npm repo is not up to date with master and I do not have any rights on it. Can you try with the master branch and tell me if it is not working? Thanks guys!

sethkrasnianski commented 7 years ago

I targeted the 1.5.9 tag and wasn't able to notice any fixes. Admittedly, I didn't full clear all caches so that could have been the issue. Let me try that.

MrFlox commented 7 years ago

I've tried it on the 1.5.9 and this bug constantly appears.

arribbar commented 7 years ago

@ivpusic Can you try again with master branch of react-native-swiper and tell me whether it works ?

srameshr commented 7 years ago

@arribbar Im on 1.5.10 and it still exists. RN version is 0.47.1

ivpusic commented 7 years ago

latest release fixed it for me.

srameshr commented 7 years ago

@arribbar @ivpusic I installed from master. It still exists. The number of slides are always n-1 where n is the length of my data source and the dots indicating current slide does not switch.

milindagrawal commented 7 years ago

If onIndexChanged is not working than try to use

            onMomentumScrollEnd={(event, state) => {
              this.setState({
                selectedIndex: state.index,
              });
            }}
justinmakaila commented 7 years ago

I'm on master and still having this issue. Gonna see if I can dig anything up..

milindagrawal commented 7 years ago

onIndexChanged property was missing till the release 1.5.10. I just took the latest release and it works fine.

justinmakaila commented 7 years ago

@milindagrawal I'm still seeing the issue mentioned by @GertjanReynaert with the this.internals.offset being invalid, resulting in it never getting called

milindagrawal commented 7 years ago

Seem's like a dimension issue. Are you giving the height and width to Swiper Component ? Below is the code working for me.

          <Swiper
            key={`Some Unique Key`}
            loop={false}
            bounces={false}
            width={deviceWidth}
            height={availableHeightForSwiper}
            dotColor="grey"
            activeDotColor="orange"
            paginationStyle={customPaginationStyle}
            automaticallyAdjustContentInsets={true}
            index={this.state.selectedIndex}
            onIndexChanged={(index) => {
              this.setState({
                selectedIndex: index,
              });
            }}
          >
            {childrens}
          </Swiper>
justinmakaila commented 7 years ago

@milindagrawal that's totally what it is... i don't pass a width, but i use a horizontal swiper. my fix in the PR referenced up there (#537) was just to set a default value to that offset logic, but I guess there's something else buried in there that uses the width and height that you supply...

nixoz commented 7 years ago

Gentlemen, still experience the same issue on iOS(android is ok) React 0.48, swiper 1.5.12.

Tried all workarounds...

The last slide is shown instead of first one if loop is enabled. Disabling the loop solving the issue, but...

dktan commented 7 years ago

both onIndexChanged and onMomentumScrollEnd bot firing for me. the only handler works for me right now is onScrollBeginDrag...

dktan commented 7 years ago

seems this part in updateIndex function gives us this problem:

const diff = offset[dir] - this.internals.offset[dir]

    const step = dir === 'x' ? state.width : state.height

    let loopJump = false

    // Do nothing if offset no change.

    if (!diff) return

since this.internals.offset is set to {} in init function not {x:0, y:0}, diff is always NaN and updateIndex always return early.

atljoseph commented 6 years ago

this issue is not fixed. I am running 1.5.13 and onIndexChanged is NOT fired until the second time I load the component. annoying as hell

atljoseph commented 6 years ago

WORKAROUND / ISSUE FIX FOUND:

NO SUCCESS - included a Height/Width property as indicated above. NO SUCCESS - used onMomentumScrollEnd and no change from onIndexChanged

SUCCESS - added a "key" prop with a unique uuidV4 GUID value and problem WENT AWAY!

Hope this helps

dugginenisagar commented 6 years ago

@atljoseph Added "key" prop to the Views inside "Swiper" but still onIndexChanged not getting called. Can you please share the code which worked for you?

Michael-Benecaid commented 6 years ago

Sorry, still get the problem in ios. The last view rendered as first. "react-native": "0.44.0" "react-native-swiper": "^1.5.13" Can anyone help me?

atljoseph commented 6 years ago

Npm install uuid

Import uuidv4 from ‘uuid/v4’;

For every child in the array, add the following property: key={uuidv4()}

This is more of a react native oddity then a bug in the library.

Sent from my iPhone

On Feb 21, 2018, at 2:35 PM, Michael-Benecaid notifications@github.com wrote:

Sorry, still get the problem in ios. The last view rendered as first. "react-native": "0.44.0" "react-native-swiper": "^1.5.13" Can anyone help me?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

atljoseph commented 6 years ago

In react native, always give every item in every array unique key value

Sent from my iPhone

On Feb 21, 2018, at 2:35 PM, Michael-Benecaid notifications@github.com wrote:

Sorry, still get the problem in ios. The last view rendered as first. "react-native": "0.44.0" "react-native-swiper": "^1.5.13" Can anyone help me?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Glinkis commented 4 years ago

I'm also having this problem.

lancesnider commented 4 years ago

I know this is a really old thread, but I was having the same issue and none of the above solutions seemed to work... because it was just me being dumb. 🤦

tldr; you don't appear to be able to change the number of slides on the fly my and on first render, I had zero.

My problem:

Basically I was creating a Swiper with zero slides, then a second later updating it with multiple slides. All I had to do was wait until I had the data to render the Swiper:

slides.length > 0 && (
          <Swiper
ducsilva commented 1 year ago

I know this is a really old thread, but I was having the same issue and none of the above solutions seemed to work... because it was just me being dumb. 🤦

tldr; you don't appear to be able to change the number of slides on the fly my and on first render, I had zero.

My problem:

  • My slides were being created based on data from an API call.
  • The Slider existed before the API got its response

Basically I was creating a Swiper with zero slides, then a second later updating it with multiple slides. All I had to do was wait until I had the data to render the Swiper:

slides.length > 0 && (
          <Swiper

it's very useful. I resolved with this tutorial

Ha0ran2001 commented 2 months ago

I know this is a really old thread, but I was having the same issue and none of the above solutions seemed to work... because it was just me being dumb. 🤦

tldr; you don't appear to be able to change the number of slides on the fly my and on first render, I had zero.

My problem:

  • My slides were being created based on data from an API call.
  • The Slider existed before the API got its response

Basically I was creating a Swiper with zero slides, then a second later updating it with multiple slides. All I had to do was wait until I had the data to render the Swiper:

slides.length > 0 && (
          <Swiper

Thanks so much! Can you explain the principle of this solution?