flat-list-mvcp icon indicating copy to clipboard operation
flat-list-mvcp copied to clipboard

`autoscrollToTopThreshold` issue

Open sergeykimaia opened this issue 5 years ago • 29 comments

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)

sergeykimaia avatar Dec 29 '20 13:12 sergeykimaia

@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.

vishalnarkhede avatar Dec 29 '20 16:12 vishalnarkhede

@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

sergeykimaia avatar Dec 29 '20 19:12 sergeykimaia

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

vishalnarkhede avatar Dec 29 '20 21:12 vishalnarkhede

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

sergeykimaia avatar Dec 29 '20 22:12 sergeykimaia

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

vishalnarkhede avatar Dec 29 '20 22:12 vishalnarkhede

@vishalnarkhede got a chance to test it yet?

sergeykimaia avatar Dec 30 '20 19:12 sergeykimaia

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 avatar Dec 30 '20 20:12 vishalnarkhede

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

sergeykimaia avatar Dec 30 '20 21:12 sergeykimaia

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

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

vishalnarkhede avatar Dec 30 '20 22:12 vishalnarkhede

@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?

sergeykimaia avatar Dec 30 '20 22:12 sergeykimaia

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.

vishalnarkhede avatar Dec 30 '20 23:12 vishalnarkhede

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 avatar Dec 30 '20 23:12 sergeykimaia

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.

sergeykimaia avatar Dec 30 '20 23:12 sergeykimaia

@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.

vishalnarkhede avatar Dec 31 '20 15:12 vishalnarkhede

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

sergeykimaia avatar Jan 03 '21 09:01 sergeykimaia

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

vishalnarkhede avatar Jan 08 '21 00:01 vishalnarkhede

@sergeykimaia Could you try using @stream-io/[email protected]?

vishalnarkhede avatar Jan 09 '21 22:01 vishalnarkhede

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 avatar Jan 10 '21 06:01 sergeykimaia

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

sergeykimaia avatar Jan 10 '21 07:01 sergeykimaia

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 :)

vishalnarkhede avatar Jan 10 '21 19:01 vishalnarkhede

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 avatar Jan 14 '21 07:01 sergeykimaia

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

sergeykimaia avatar Jan 17 '21 15:01 sergeykimaia

@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?

vishalnarkhede avatar Jan 22 '21 07:01 vishalnarkhede

@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 avatar Jan 23 '21 22:01 sergeykimaia

any progress on this?

sergeykimaia avatar Apr 07 '21 08:04 sergeykimaia

@sergeykimaia could you try v0.10.0 please?

vishalnarkhede avatar Apr 07 '21 08:04 vishalnarkhede

@sergeykimaia could you try v0.10.0 please?

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

sergeykimaia avatar Apr 21 '21 07:04 sergeykimaia

same issue

NavidHosseini avatar Apr 26 '21 11:04 NavidHosseini

same issue occured

pawanstackinfinite avatar Nov 27 '23 11:11 pawanstackinfinite