GetStream / flat-list-mvcp

"maintainVisibleContentPosition" prop support for Android react-native
https://dev.to/vishalnarkhede/maintainvisiblecontentposition-prop-for-android-react-native-3olf
MIT License
133 stars 33 forks source link

`autoscrollToTopThreshold` issue #3

Open sergeykimaia opened 3 years ago

sergeykimaia commented 3 years ago

heres a code example: pressing sub number will add an item to the list and it'll move the entire list, not maintaining the content position

import React, { FunctionComponent, useState } from 'react'
import { View, TouchableOpacity, Text } from 'react-native'
import { FlatList } from '@stream-io/flat-list-mvcp';

const app = () => {
  // stores
  const [numbers, setNumbers] = useState([1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 12, 13, 14, 15, 16, 17, 18, 19])
  const addMaxNumber = () => {
    setNumbers((prev) => [...prev, prev[prev.length - 1] + 1])
  }
  const addMinNumber = () => {
    setNumbers((prev) => [prev[0] - 1, ...prev])
  }
  return (
    <View style={{ flex: 1 }}>
      <TouchableOpacity onPress={addMaxNumber} style={{ padding: 8, backgroundColor: 'green' }}>
        <Text>add number</Text>
      </TouchableOpacity>
      <TouchableOpacity onPress={addMinNumber} style={{ padding: 8, backgroundColor: 'red' }}>
        <Text>sub number</Text>
      </TouchableOpacity>
      <FlatList
        data={numbers}
        initialNumToRender={10}
        maintainVisibleContentPosition={maintainVisibleContentPosition}
        contentContainerStyle={{ flexGrow: 1, justifyContent: 'flex-end' }}
        keyExtractor={(item) => item.toString()}
        renderItem={({ item }) => <View style={{ flex: 1, padding: 32, justifyContent: 'center', alignItems: 'center', borderWidth: 8 }}><Text>{item}</Text></View>}
      />
    </View>
  )
}
const maintainVisibleContentPosition = {
  autoscrollToTopThreshold: 600,
  minIndexForVisible: 0,
}
export default (app)
vishalnarkhede commented 3 years ago

@sergeykimaia you may want to lower the value of autoscrollToTopThreshold. Something like 10 or so. Basically right now, when you add new items to list, if scroll position is below 600, it will autoscroll to end of the list.

Let me know if that fixes your issue.

sergeykimaia commented 3 years ago

@sergeykimaia you may want to lower the value of autoscrollToTopThreshold. Something like 10 or so. Basically right now, when you add new items to list, if scroll position is below 600, it will autoscroll to end of the list.

Let me know if that fixes your issue.

it doesn't scroll me to the top at all

vishalnarkhede commented 3 years ago

I will setup an example Maybe that will be easier :0

sergeykimaia commented 3 years ago

I will setup an example Maybe that will be easier :0

did you try running my code? does it work fine for you? I wrote it just to show the bug being reproduced

vishalnarkhede commented 3 years ago

@sergeykimaia I haven't got chance to run it yet. I will try it tomorrow :)

sergeykimaia commented 3 years ago

@vishalnarkhede got a chance to test it yet?

vishalnarkhede commented 3 years ago

Hey @sergeykimaia I am trying it now. So I can see two buttons in example that you mentioned, its the upwards scroll ("Sub number") which doesn't work for you, right?

sergeykimaia commented 3 years ago

Hey @sergeykimaia I am trying it now. So I can see two buttons in example that you mentioned, its the upwards scroll ("Sub number") which doesn't work for you, right?

@vishalnarkhede yes

vishalnarkhede commented 3 years ago

@sergeykimaia Please use some negative value on autoscrollToTopThreshold. That works in your given example :)

  maintainVisibleContentPosition={{
    autoscrollToTopThreshold: -10,
    minIndexForVisible: 0,
  }}
sergeykimaia commented 3 years ago

@sergeykimaia Please use some negative value on autoscrollToTopThreshold. That works in your given example :)

  maintainVisibleContentPosition={{
    autoscrollToTopThreshold: -10,
    minIndexForVisible: 0,
  }}

@vishalnarkhede it works! I assume then that we cant use autoscrollToTopThreshold for its intended purpose then?

vishalnarkhede commented 3 years ago

No!! It works as expected.

The optional autoscrollToTopThreshold can be used to make the content automatically scroll to the top after making the adjustment if the user was within the threshold of the top before the adjustment was made. https://reactnative.dev/docs/scrollview#maintainvisiblecontentposition

Which means if you set autoscrollToTopThreshold as 600, then your list will automatically scroll to first element, if current scroll position between 0-600. Once you scroll beyond 600, then only scroll position will be maintained.

sergeykimaia commented 3 years ago

No!! It works as expected.

The optional autoscrollToTopThreshold can be used to make the content automatically scroll to the top after making the adjustment if the user was within the threshold of the top before the adjustment was made. https://reactnative.dev/docs/scrollview#maintainvisiblecontentposition

Which means if you set autoscrollToTopThreshold as 600, then your list will automatically scroll to first element, if current scroll position between 0-600. Once you scroll beyond 600, then only scroll position will be maintained.

but it doesnt scroll to the first element, it scrolls exactly the size of the added item

for example, setting autoscrollToTopThreshold: 10000, then slightly scroll down, then press the sub number button, you'll see it doesnt scroll you to the top/first item. Or you can even scroll a few items down then press sub item

sergeykimaia commented 3 years ago

also in my example if i set autoscrollToTopThreshold:600 then scroll to bottom, and keep pressing "sub number" it sometimes scrolls up by 1 item.

vishalnarkhede commented 3 years ago

@sergeykimaia you are right!! I can see it now. I will probably setup an example app next week and try to resolve this issue :)

We are using it in our inhouse project, where its working fine. But could be some other config that we have in our project and not in this lib.

sergeykimaia commented 3 years ago

another bug: the following code scrolls 50% of the time when pressing the red 'sub number' on fresh render (when first entering the component, theres like 50% chance that sub button will always scroll)

import React, { FunctionComponent, useState } from 'react'
import { View, TouchableOpacity, Text } from 'react-native'
import { FlatList } from '@stream-io/flat-list-mvcp';

const app = () => {
  // stores
  const [numbers, setNumbers] = useState([1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 12, 13, 14, 15, 16, 17, 18, 19])
  const addMaxNumber = () => {
    setNumbers((prev) => [...prev, prev[prev.length - 1] + 1])
  }
  const addMinNumber = () => {
    setNumbers((prev) => [prev[0] - 7, prev[0] - 6, prev[0] - 5, prev[0] - 4, prev[0] - 3, prev[0] - 2, prev[0] - 1, ...prev])
  }
  return (
    <View style={{ flex: 1 }}>
      <TouchableOpacity onPress={addMaxNumber} style={{ padding: 8, backgroundColor: 'green' }}>
        <Text>add number</Text>
      </TouchableOpacity>
      <TouchableOpacity onPress={addMinNumber} style={{ padding: 8, backgroundColor: 'red' }}>
        <Text>sub number</Text>
      </TouchableOpacity>
      <FlatList
        data={numbers}
        initialNumToRender={10}

        maintainVisibleContentPosition={maintainVisibleContentPosition}
        contentContainerStyle={{ flexGrow: 1, justifyContent: 'flex-end' }}
        keyExtractor={(item) => item.toString()}
        renderItem={({ item }) => <View style={{ flex: 1, padding: 32, justifyContent: 'center', alignItems: 'center', borderWidth: 8 }}><Text>{item}</Text></View>}
      />
    </View>
  )
}
const maintainVisibleContentPosition = {
  autoscrollToTopThreshold: -100,
  minIndexForVisible: 0,
}
export default (app)

easiest way to reproduce is just to reload then press "sub number" EDIT: on emulator it happens to me 50%~ of the time, but on an actual phone it happens around 10%~ of the time

vishalnarkhede commented 3 years ago

@sergeykimaia I found the issue. Will publish a fix soon :)

vishalnarkhede commented 3 years ago

@sergeykimaia Could you try using @stream-io/flat-list-mvcp@0.0.3-dev.0?

sergeykimaia commented 3 years ago

it seems autoscrollToTopThreshold got fixed in the latest version, definitely feels better than the previous version.

another bug: the following code scrolls 50% of the time when pressing the red 'sub number' on fresh render (when first entering the component, theres like 50% chance that sub button will always scroll)

import React, { FunctionComponent, useState } from 'react'
import { View, TouchableOpacity, Text } from 'react-native'
import { FlatList } from '@stream-io/flat-list-mvcp';

const app = () => {
  // stores
  const [numbers, setNumbers] = useState([1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 12, 13, 14, 15, 16, 17, 18, 19])
  const addMaxNumber = () => {
    setNumbers((prev) => [...prev, prev[prev.length - 1] + 1])
  }
  const addMinNumber = () => {
    setNumbers((prev) => [prev[0] - 7, prev[0] - 6, prev[0] - 5, prev[0] - 4, prev[0] - 3, prev[0] - 2, prev[0] - 1, ...prev])
  }
  return (
    <View style={{ flex: 1 }}>
      <TouchableOpacity onPress={addMaxNumber} style={{ padding: 8, backgroundColor: 'green' }}>
        <Text>add number</Text>
      </TouchableOpacity>
      <TouchableOpacity onPress={addMinNumber} style={{ padding: 8, backgroundColor: 'red' }}>
        <Text>sub number</Text>
      </TouchableOpacity>
      <FlatList
        data={numbers}
        initialNumToRender={10}

        maintainVisibleContentPosition={maintainVisibleContentPosition}
        contentContainerStyle={{ flexGrow: 1, justifyContent: 'flex-end' }}
        keyExtractor={(item) => item.toString()}
        renderItem={({ item }) => <View style={{ flex: 1, padding: 32, justifyContent: 'center', alignItems: 'center', borderWidth: 8 }}><Text>{item}</Text></View>}
      />
    </View>
  )
}
const maintainVisibleContentPosition = {
  autoscrollToTopThreshold: -100,
  minIndexForVisible: 0,
}
export default (app)

easiest way to reproduce is just to reload then press "sub number" EDIT: on emulator it happens to me 50%~ of the time, but on an actual phone it happens around 10%~ of the time

still sometimes happens, but noticed it only happens when the device is very slow (such as in an emulator) (i quickly press sub number as the component loads)

EDIT: and interestingly, when the bug occures it scrolls 50% to the top instead of to the top EDIT: uploading a video of how i reproduce the bug in a different comment

sergeykimaia commented 3 years ago

https://user-images.githubusercontent.com/58387319/104116727-ecfe9480-5323-11eb-997a-17d7a653e9d4.mov

vishalnarkhede commented 3 years ago

Thanks for testing @sergeykimaia. Published v0.0.3 since it definitely works better than 0.0.2

Will do some more investigation soon about the slow emulator thing that you mentioned and get back :)

sergeykimaia commented 3 years ago

autoscrollToTopThreshold 0 doesnt work on scrollview: (-1 and lower works tho)

import React, { FunctionComponent, useState } from 'react'
import { View, TouchableOpacity, Text } from 'react-native'
import { FlatList, ScrollView } from '@stream-io/flat-list-mvcp';

const app = () => {
  // stores
  const [numbers, setNumbers] = useState([1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 12, 13, 14, 15, 16, 17, 18, 19])
  const addMaxNumber = () => {
    setNumbers((prev) => [...prev, prev[prev.length - 1] + 1])
  }
  const addMinNumber = () => {
    setNumbers((prev) => [prev[0] - 7, prev[0] - 6, prev[0] - 5, prev[0] - 4, prev[0] - 3, prev[0] - 2, prev[0] - 1, ...prev])
  }
  return (
    <View style={{ flex: 1 }}>
      <TouchableOpacity onPress={addMaxNumber} style={{ padding: 8, backgroundColor: 'green' }}>
        <Text>add number</Text>
      </TouchableOpacity>
      <TouchableOpacity onPress={addMinNumber} style={{ padding: 8, backgroundColor: 'red' }}>
        <Text>sub number</Text>
      </TouchableOpacity>
      <ScrollView
        data={numbers}
        initialNumToRender={10}

        maintainVisibleContentPosition={maintainVisibleContentPosition}
        contentContainerStyle={{ flexGrow: 1, justifyContent: 'flex-end' }}
        keyExtractor={(item) => item.toString()}
        renderItem={({ item }) => <View style={{ flex: 1, padding: 32, justifyContent: 'center', alignItems: 'center', borderWidth: 8 }}><Text>{item}</Text></View>}
      >
        {numbers.map((item) => <View key={item.toString()} style={{ flex: 1, padding: 32, justifyContent: 'center', alignItems: 'center', borderWidth: 8 }}><Text>{item}</Text></View>)}
      </ScrollView>
    </View>
  )
}
const maintainVisibleContentPosition = {
  autoscrollToTopThreshold: 0,
  minIndexForVisible: 0,
}
export default (app)
sergeykimaia commented 3 years ago

autoscrollToTopThreshold 0 doesnt work on scrollview: (-1 and lower works tho)

import React, { FunctionComponent, useState } from 'react'
import { View, TouchableOpacity, Text } from 'react-native'
import { FlatList, ScrollView } from '@stream-io/flat-list-mvcp';

const app = () => {
  // stores
  const [numbers, setNumbers] = useState([1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 12, 13, 14, 15, 16, 17, 18, 19])
  const addMaxNumber = () => {
    setNumbers((prev) => [...prev, prev[prev.length - 1] + 1])
  }
  const addMinNumber = () => {
    setNumbers((prev) => [prev[0] - 7, prev[0] - 6, prev[0] - 5, prev[0] - 4, prev[0] - 3, prev[0] - 2, prev[0] - 1, ...prev])
  }
  return (
    <View style={{ flex: 1 }}>
      <TouchableOpacity onPress={addMaxNumber} style={{ padding: 8, backgroundColor: 'green' }}>
        <Text>add number</Text>
      </TouchableOpacity>
      <TouchableOpacity onPress={addMinNumber} style={{ padding: 8, backgroundColor: 'red' }}>
        <Text>sub number</Text>
      </TouchableOpacity>
      <ScrollView
        data={numbers}
        initialNumToRender={10}

        maintainVisibleContentPosition={maintainVisibleContentPosition}
        contentContainerStyle={{ flexGrow: 1, justifyContent: 'flex-end' }}
        keyExtractor={(item) => item.toString()}
        renderItem={({ item }) => <View style={{ flex: 1, padding: 32, justifyContent: 'center', alignItems: 'center', borderWidth: 8 }}><Text>{item}</Text></View>}
      >
        {numbers.map((item) => <View key={item.toString()} style={{ flex: 1, padding: 32, justifyContent: 'center', alignItems: 'center', borderWidth: 8 }}><Text>{item}</Text></View>)}
      </ScrollView>
    </View>
  )
}
const maintainVisibleContentPosition = {
  autoscrollToTopThreshold: 0,
  minIndexForVisible: 0,
}
export default (app)

nvm, ignore what i wrote, 'sub item' often doesnt work on scrollview for me. even with autoscrollToTopThreshol<0

this morning when i tested it worked consistently though... hmm EDIT3: on my phone it worked fine, guess its probably the slow emulator bug

vishalnarkhede commented 3 years ago

@sergeykimaia I noticed that I missed a race condition between ref setting and enabling mvcp. I have published a new patch to fix it, so can you please upgrade and check again?

sergeykimaia commented 3 years ago

@sergeykimaia I noticed that I missed a race condition between ref setting and enabling mvcp. I have published a new patch to fix it, so can you please upgrade and check again?

will check in 12 hrs, will edit this comment with the results

EDIT: it feels flawless, im pretty sure i had the bug happen once out of 60+ attempts, but I'm not sure anymore since I wasn't able to reproduce it since. nvm restarted emulator reproduced again, infrequently though.

to reproduce, just keep reloading and you'll see it's sometimes scrolling:

import React, { FunctionComponent, useState } from 'react'
import { View, TouchableOpacity, Text } from 'react-native'
import { FlatList, ScrollView } from '@stream-io/flat-list-mvcp';

const app = () => {
  // stores
  const [numbers, setNumbers] = useState([1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 12, 13, 14, 15, 16, 17, 18, 19])
  const addMaxNumber = () => {
    setNumbers((prev) => [...prev, prev[prev.length - 1] + 1])
  }
  const addMinNumber = () => {
    setNumbers((prev) => [prev[0] - 7, prev[0] - 6, prev[0] - 5, prev[0] - 4, prev[0] - 3, prev[0] - 2, prev[0] - 1, ...prev])
  }
  React.useEffect(() => {
    setTimeout(() => {
      // setNumbers((prev) => [...prev, prev[prev.length - 1] + 1])
      setNumbers((prev) => [prev[0] - 7, prev[0] - 6, prev[0] - 5, prev[0] - 4, prev[0] - 3, prev[0] - 2, prev[0] - 1, ...prev])
    }, 10);
  }, [])
  return (
    <View style={{ flex: 1 }}>
      <TouchableOpacity onPress={addMaxNumber} style={{ padding: 8, backgroundColor: 'green' }}>
        <Text>add number</Text>
      </TouchableOpacity>
      <TouchableOpacity onPress={addMinNumber} style={{ padding: 8, backgroundColor: 'red' }}>
        <Text>sub number</Text>
      </TouchableOpacity>
      <ScrollView
        data={numbers}
        initialNumToRender={10}

        maintainVisibleContentPosition={maintainVisibleContentPosition}
        contentContainerStyle={{ flexGrow: 1, justifyContent: 'flex-end' }}
        keyExtractor={(item) => item.toString()}
        renderItem={({ item }) => <View style={{ flex: 1, padding: 32, justifyContent: 'center', alignItems: 'center', borderWidth: 8 }}><Text>{item}</Text></View>}
      >
        {numbers.map((item) => <View key={item.toString()} style={{ flex: 1, padding: 32, justifyContent: 'center', alignItems: 'center', borderWidth: 8 }}><Text>{item}</Text></View>)}
      </ScrollView>
    </View>
  )
}
const maintainVisibleContentPosition = {
  autoscrollToTopThreshold: 0,
  minIndexForVisible: 0,
}
export default (app)

the main difference in this code is that theres a settimeout inside a useeffect that adds items quickly even with 1000 ms delay it still happens

sergeykimaia commented 3 years ago

any progress on this?

vishalnarkhede commented 3 years ago

@sergeykimaia could you try v0.10.0 please?

sergeykimaia commented 3 years ago

@sergeykimaia could you try v0.10.0 please?

still happens, settimeout adds items after 10 ms (tested 100 ms aswell), and sometimes it scrolls

NavidHosseini commented 3 years ago

same issue

pawanstackinfinite commented 9 months ago

same issue occured