Loop={true} + initialIndex leads to brief off by one error flicker

zzorba commented 5 years ago

Which OS ?

iOS

Version

1.6

react-native-swiper v1.6
react-native v0.59.8

Expected behaviour

I was debugging an issue where setting the index param briefly showed the wrong data (off by one earlier) for a brief second before a redraw happened.

In debugging, this seems to be related to the loop={true} behavior (the default), since eliminating looping causes the offset to be calculated correctly and no flicker.

I'm guessing the implementation has some kind of 'extra' item at the front to handle the left-swipe when looping, and the initialOffset is not accounting for this?

Actual behaviour

Setting index causes it to start drawing on the correct position without a flicker.

Steps to reproduce

Create a Swiper with multiple items, and loop={true}
Set the initial index.
Observe that it is briefly off by one before correcting itself.

ArrayZoneYour commented 5 years ago

@zzorba Can you reproduce it on react-native@0.59.4 ? IOS bug may be related to ScrollView, https://github.com/facebook/react-native/issues/24826#issuecomment-495570736 Android issue will be tracked here: https://github.com/react-native-community/react-native-viewpager/issues/34

zzorba commented 5 years ago

I rebuilt with RN 0.59.4 and it seems to still happen on simulator.
Unlike the gif, the 'flash' of the wrong content only happens on initial draw -- once you start interacting with it is correct and there's no issues. Removing the loop={false} from my Swiper props causes it to work as expected (initial draw at the correct position).

My guess is it's the following code in onLayout that is the cause.

    if (this.state.total > 1) {
      let setup = this.state.index
      if (this.props.loop) {
        setup++
      }
      offset[this.state.dir] =
        this.state.dir === 'y' ? height * setup : width * setup
    }

The code in initState doesn't seem to account for the looping. The render method for android does seem to adjust based on loop, initialPage={this.props.loop ? this.state.index + 1 : this.state.index}? I think this would explain the issue, initially being off by one and then fixing itself when onLayout fires.

ArrayZoneYour commented 5 years ago

Can you provide the minimal example code and the environment ? I can't reproduce it on my ios simulator

react-native info

info 
  React Native Environment Info:
    System:
      OS: macOS 10.14.4
      CPU: (12) x64 Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
      Memory: 159.74 MB / 16.00 GB
      Shell: 5.3 - /bin/zsh
    Binaries:
      Node: 11.11.0 - ~/.nvm/versions/node/v11.11.0/bin/node
      Yarn: 1.9.4 - /usr/local/bin/yarn
      npm: 6.9.0 - ~/.nvm/versions/node/v11.11.0/bin/npm
      Watchman: 4.9.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 12.2, macOS 10.14, tvOS 12.2, watchOS 5.2
      Android SDK:
        API Levels: 23, 24, 26, 27, 28
        Build Tools: 23.0.1, 26.0.3, 27.0.3, 28.0.3
    IDEs:
      Android Studio: 3.2 AI-181.5540.7.32.5056338
      Xcode: 10.2.1/10E1001 - /usr/bin/xcodebuild
    npmPackages:
      react: 16.8.3 => 16.8.3 
      react-native: 0.59.9 => 0.59.9

my App.js

import React, { useState } from 'react'
import Swiper from 'react-native-swiper'

import { View, StyleSheet, Text, TouchableWithoutFeedback } from 'react-native'
const styles = StyleSheet.create({
  // wrapper: { flex: 1 },
  wrapper: {},
  slide1: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#9DD6EB'
  },
  slide2: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#97CAE5'
  },
  slide3: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#92BBD9'
  },
  text: {
    color: '#fff',
    fontSize: 30,
    fontWeight: 'bold'
  }
})

const SwiperComponent = () => {
  const [autoPlay, setAutoPlay] = useState(true)
  const stop = () => setAutoPlay(false)
  const start = () => setAutoPlay(true)
  return (
    <Swiper
      style={styles.wrapper}
      autoplay={autoPlay}
      loop={true}
      autoplayTimeout={4}
      // index={0}
      index={3}
      // bounces={false}
    >
      <View style={styles.slide1}>
        <TouchableWithoutFeedback onPress={() => {}}>
          <Text style={styles.text}>1</Text>
        </TouchableWithoutFeedback>
      </View>
      <View style={styles.slide2}>
        {/* <TouchableWithoutFeedback onPress={() => this.props.navigation.pop()}> */}
        <Text style={styles.text}>2</Text>
        {/* </TouchableWithoutFeedback> */}
      </View>
      <View style={styles.slide3}>
        <TouchableWithoutFeedback onPress={() => {}}>
          <Text style={styles.text}>3</Text>
        </TouchableWithoutFeedback>
      </View>
      <View style={styles.slide3}>
        <TouchableWithoutFeedback onPress={() => {}}>
          <Text style={styles.text}>4</Text>
        </TouchableWithoutFeedback>
      </View>
    </Swiper>
  )
}

export default SwiperComponent

Screen Recording 2019-07-02 at 1 11 03 AM 2019-07-02 01_24_33

zzorba commented 5 years ago

Yep, I think I have a repro here. My project has a very large number of items, when I moved from the 10 static ones to a couple hundred invoked with a key, it shows up with the wrong number, and then quickly corrects itself.

import React, { useState } from 'react'
import Swiper from 'react-native-swiper'
import { map, range } from 'lodash';

import { View, StyleSheet, Text, TouchableWithoutFeedback } from 'react-native'
const styles = StyleSheet.create({
  // wrapper: { flex: 1 },
  wrapper: {},
  slide1: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#9DD6EB'
  },
  slide2: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#97CAE5'
  },
  slide3: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#92BBD9'
  },
  text: {
    color: '#fff',
    fontSize: 30,
    fontWeight: 'bold'
  }
})

const SwiperComponent = () => {
  const [autoPlay, setAutoPlay] = useState(true)
  const stop = () => setAutoPlay(false)
  const start = () => setAutoPlay(true)
  return (
    <Swiper
      style={styles.wrapper}
      loop={true}
      loadMinimal
      loadMinimalSize={3}
      index={1}
      // bounces={false}
    >
    { map(range(0, 200), id => (
      <View style={styles.slide1} key={id}>
        <TouchableWithoutFeedback onPress={() => {}}>
          <Text style={styles.text}>{id}</Text>
        </TouchableWithoutFeedback>
      </View>
    ))}
    </Swiper>
  )
}

export default SwiperComponent

ArrayZoneYour commented 5 years ago

Can you provide a gif and more environment info? I still cannot reproduce the flicker with the code above (；′⌒`)