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

Android vertical swiper #643

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tudorilisoi
Copy link

@tudorilisoi tudorilisoi commented Nov 3, 2017

Is it a bugfix ?

Is it a new feature ?

No
It's a bug fix for nested vertical swiper not being displayed on Android (with horizontal={false} ).

Describe what you've done:

I included a vertical view pager for Android

How to test it ?

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

const Dimensions = require('Dimensions');
const window = Dimensions.get('window');

const justification = {
  flex: 1,
  justifyContent: 'center',
  alignItems: 'center',
}
const justification2 = {
  flex: 1,
  alignItems: 'center',
}
const styles = {
  wrapper: {
    backgroundColor: 'transparent'
  },
  innerSwiper: {
    width: window.width,
    height: window.height / 3,
    marginTop: 60,
    backgroundColor: '#92BBD9'
  },
  slide1: {
    ...justification,
    backgroundColor: '#9DD6EB'
  },
  slide2: {
    ...justification,
    backgroundColor: '#97CAE5'
  },
  slide3: {
    ...justification,
    backgroundColor: '#92BBD9'
  },
  slide4: {
    ...justification2,
    backgroundColor: '#D6EB9D'
  },
  slide5: {
    ...justification,
    backgroundColor: '#CAE597'
  },
  slide6: {
    ...justification,
    backgroundColor: '#BBD992'
  },
  text: {
    color: '#fff',
    fontSize: 30,
    fontWeight: 'bold'
  }
}

export default () =>
  <Swiper style={styles.wrapper} showsButtons={true} horizontal={false}>

    <View style={styles.slide4}>
      <View style={styles.innerSwiper}>
        <Swiper style={styles.wrapper} showsButtons={true} >

          <View style={styles.slide1}>
            <Text style={styles.text}>Horiz Panel 1</Text>
          </View>

          <View style={styles.slide2}>
            <Text style={styles.text}>Horiz Panel 2</Text>
          </View>

          <View style={styles.slide3}>
            <Text style={styles.text}>Horiz Panel 3</Text>
          </View>

        </Swiper>
      </View>
      <Text style={styles.text}>Vertical Panel 1</Text>
    </View>
    <View style={styles.slide5}>
      <Text style={styles.text}>Vertical Panel 2</Text>
    </View>
    <View style={styles.slide6}>
      <Text style={styles.text}>Vertical Panel 3</Text>
    </View>
  </Swiper>

@robblovell
Copy link

robblovell commented Nov 3, 2017

Using the code at the bottom as a test, there seems to be a number of problems (some not introduced here, but revealed as a result of the fix):

  1. On android, there is a problem with the pager buttons, touching a vertical scroller pager button gives there error:
_this.scrollView[animated ? 'setPage' : 'setPageWithoutAnimation'] is not a function. 
(In '_this.scrollView[animated ? 'setPage' : 'setPageWithoutAnimation'](diff)', 
'_this.scrollView[animated ? 'setPage' : 'setPageWithoutAnimation']' is undefined)
scrollBy
    index.js:467:91
onPress
    index.js:600:72
...
  1. The vertical page buttons are in the wrong location. This is a problem on both ios and android and not introduced by this fix. Again, this is not a problem with this merge request, but it might be nice to fix it in a follow up request (perhaps there is one that I haven't seen).

  2. There seems to be a font problem in line 94 of index.js for Android:

  buttonText: {
    fontSize: 50,
    color: '#007aff',
    fontFamily: 'Arial'
  }
gives the error _on load_ in Android: 
console.error: "fontFamily 'Arial' is not a system font and has not been loaded through Expo.Font.loadAsync.

- If you intended to use a system font, make sure you typed the name correctly and that it is supported by your device operating system.

- If this is a custom font, be sure to load it with Expo.Font.loadAsync."
newConsoleFunc
    AppEntry.bundle?platform=android&dev=true&minify=false:1760:21
error
This may be an error in my setup, but I am using a unmodified android pixel phone simulator for the test.

Removing the _fontFamily: 'Arial'_ removes this error. Perhaps there is a better fix out there:
var buttonText = {
  fontSize: 50,
  color: '#007aff',
  fontFamily: 'Roboto'
}
if (Platform.OS === 'ios') {
  buttonText = {
    fontSize: 50,
    color: '#007aff',
    fontFamily: 'Arial'
  }
}

Note:

An alternative to this code, ultimately, is to use ScrollView for both Android and ios. I realize that ScrollView is not complete for android yet (snap doesn't work for vertical sliding). This fix can work in the interim until the ScrollView for Android fix is complete (I see that there is activity toward getting this fixed within the next month).

Opinion:

I would move forward with this pull request once the first problem above is addressed and fixe problems 2 and 3 in separate pull requests.

The code used for testing:

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

const Dimensions = require('Dimensions');
const window = Dimensions.get('window');

const justification = {
  flex: 1,
  justifyContent: 'center',
  alignItems: 'center',
}
const justification2 = {
  flex: 1,
  alignItems: 'center',
}
const styles = {
  wrapper: {
    backgroundColor: 'transparent'
  },
  innerSwiper: {
    width: window.width,
    height: window.height / 3,
    marginTop: 60,
    backgroundColor: '#92BBD9'
  },
  slide1: {
    ...justification,
    backgroundColor: '#9DD6EB'
  },
  slide2: {
    ...justification,
    backgroundColor: '#97CAE5'
  },
  slide3: {
    ...justification,
    backgroundColor: '#92BBD9'
  },
  slide4: {
    ...justification2,
    backgroundColor: '#D6EB9D'
  },
  slide5: {
    ...justification,
    backgroundColor: '#CAE597'
  },
  slide6: {
    ...justification,
    backgroundColor: '#BBD992'
  },
  text: {
    color: '#fff',
    fontSize: 30,
    fontWeight: 'bold'
  }
}

export default () =>
  <Swiper style={styles.wrapper} showsButtons={true} horizontal={false}>

    <View style={styles.slide4}>
      <View style={styles.innerSwiper}>
        <Swiper style={styles.wrapper} showsButtons={true} >

          <View style={styles.slide1}>
            <Text style={styles.text}>Horiz Panel 1</Text>
          </View>

          <View style={styles.slide2}>
            <Text style={styles.text}>Horiz Panel 2</Text>
          </View>

          <View style={styles.slide3}>
            <Text style={styles.text}>Horiz Panel 3</Text>
          </View>

        </Swiper>
      </View>
      <Text style={styles.text}>Vertical Panel 1</Text>
    </View>
    <View style={styles.slide5}>
      <Text style={styles.text}>Vertical Panel 2</Text>
    </View>
    <View style={styles.slide6}>
      <Text style={styles.text}>Vertical Panel 3</Text>
    </View>
  </Swiper>

@robblovell
Copy link

Related to #477 too.

@tudorilisoi
Copy link
Author

I (shamefully) pasted the example, should have come up with a minimalist one.
Thank you so much for taking the time to test it.
Bottom line, this does work better, but indeed has issues with the buttons and the bullets. I thought some people would like to use it this month :) i edited the req. to include your fixed test code

@robblovell
Copy link

robblovell commented Nov 3, 2017

Other issues:

  1. It appears that sometimes the render method for the vertical panels either don't get called correctly or sub views don't get rendered.
  2. The vertical position dots don't get updated for vertical panels.

@robblovell
Copy link

I agree with you that this is a vast improvement over not having anything at all!

@robblovell
Copy link

robblovell commented Nov 3, 2017

I have found that commenting out the ios specific statements in the code does produce a working application, but with one problem: On Android, the vertical scroll doesn't snap to page breaks. Everything else seems to work.

I noticed that the fix for ScrollView snap on Android is almost merged into the main branch of react-native. Perhaps there is a workaround for the snap problem on Android in the interim?

@alonle
Copy link

alonle commented Nov 5, 2017

Hi, I have tested it on my project and got an issue with loadMinimal and loadMinimalSize API
Some pages which contain video element are not loaded.
When I remove the loadMinimal loadMinimalSize it worked
Any Idea why?

@vedran
Copy link

vedran commented Jan 16, 2018

@alonle I believe I have a solution for the loadMinimal issue: #358 (comment)

@Versus2017
Copy link

@tudorilisoi I tried your PR with my own project, and I found it will crash in below code:
Here should be fixed:

if (Platform.OS !== 'ios') {
      this.scrollView && this.scrollView[animated ? 'setPage' : 'setPageWithoutAnimation'](diff)
    } else {
      this.scrollView && this.scrollView.scrollTo({ x, y, animated })
}

with

 if (Platform.OS !== 'ios' && this.props.horizontal === true) {
      this.scrollView && this.scrollView[animated ? 'setPage' : 'setPageWithoutAnimation'](diff)
    } else {
      this.scrollView && this.scrollView.scrollTo({ x, y, animated })
}

because it is the same with ios when you use VertViewPager, they all have scrollTo method.

@CodeLuca
Copy link

CodeLuca commented Mar 10, 2018

Any progress on this?

@smitthakkar1
Copy link

It's working now.

Use this.

https://www.npmjs.com/package/@nart/react-native-swiper

Don't forget to add prop horizontal= {false} to Swiper

@EladZucker
Copy link

@smitthakkar1 using https://www.npmjs.com/package/@nart/react-native-swiper I found myself having the problem
image

@smitthakkar1
Copy link

@EladZucker it seems the problem with styles of scrollview. try to disable the styles and try again

@EladZucker
Copy link

EladZucker commented Apr 10, 2018

@smitthakkar1 got it ! its working now ! thanks ! when do you think it will get pushed to this repo ?

@smitthakkar1
Copy link

@EladZucker can you please paste your code here ?

@zagoa
Copy link

zagoa commented Apr 16, 2018

I try to do something similar to this on android. If I copy the code all is ok but if I want the inverse (scroll verticaly and after to scroll horizontaly) that cause a problem. My code look like this :

<Swiper horizontal={false} loop={false} ref={(swiper) => {this._swiperVertical = swiper;}}>
     <View><Text>home</Text></View>
    <Swiper loop={false}  ref={(swiper) => {this._swiperHorizontal = swiper;}}>
        <View><Text>one</Text></View>
	<View><Text>two</Text></View>
   </Swiper>
</Swiper>

The problem is when I scroll verticaly there is no content in my swiper horizontal, and I don't understand why.

Using https://www.npmjs.com/package/@nart/react-native-swiper.

@Andrfas
Copy link

Andrfas commented May 3, 2018

Installed https://www.npmjs.com/package/@nart/react-native-swiper, set horizontal={false} - still doesn't work (just scrolls horizontally).
ReactNative v0.49.5
React v16

@acollazomayer
Copy link

Im having the same problem with "react": "16.3.1",
"react-native": "0.55.4", on android. Any updates on this

@syntax-e
Copy link

Same issue. The swiper shows up horizontally on Android invariable to the horizontal={false} property.

@VC444
Copy link

VC444 commented Jun 7, 2018

Is there any alternative that works for both android and ios?

@muhammadsr
Copy link

For those who are still struggling, please check the Files Changed up in the pull request. You can also go to https://www.npmjs.com/package/react-native-vertical-view-pager

@kevupton
Copy link

@smitthakkar1 I tried your package, however the index property doesnt seem to do anything when horizontal={false}

@DavidCai75905171
Copy link

+1

1 similar comment
@mrwillowyang
Copy link

+1

@BobFarias
Copy link

@smitthakkar1 I tried your package, however the index property doesnt seem to do anything when horizontal={false}

+1

@ArrayZoneYour ArrayZoneYour added this to the Backlog milestone Jun 24, 2019
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

Successfully merging this pull request may close these issues.

Inner nested swiper doesn't show any content