PedroBern / react-native-collapsible-tab-view

A cross-platform Collapsible Tab View component for React Native
MIT License
831 stars 161 forks source link

Updating state with onViewableItemsChanged makes scrolling extremely laggy #160

Open bekatd opened 3 years ago

bekatd commented 3 years ago

I am trying to toggle tab flatlist item visibility prop on/off based on they are visible or not in current viewport via onViewableItemsChanged but seems like this lib does heavy computations and when updating state of flatlist (which rerenders all items) scrolling experience is extremely laggy

Current behavior

Scrolling is buggy, items are jumping on every rerender up and down

Expected behaviour

Scrolling must remain smooth

Code sample

import React, { useState, useRef } from 'react';
import { Tabs } from 'react-native-collapsible-tab-view';

const TabOne = ({ data, ...rest }) => {
    const [visibleIdx, setVisibleIdx] = useState(null);

    const onViewableItemsChanged = useRef(({ changed }) => {
        setVisibleIdx(changed.length ? changed[0].item.id : null);
    });

    const renderItem = (item) => (
        <ContentItem isVisible={item.id === visibleIdx} />
    );

    return (
        <Tabs.FlatList
            data={data}
            viewabilityConfig={{ viewAreaCoveragePercentThreshold: 75 }}
            onViewableItemsChanged={onViewableItemsChanged.current}
            renderItem={({ item, index }) => renderItem(item, index)}
            {...rest}
        />
    );
};

export default TabOne;

What have you tried

I tried to use standard FlatList instead of Tabs.FlatList and scrolling issue gone, but the whole functionality of this lib is lost.

samih7 commented 3 years ago

We are experiencing the exact same issue, which indeed causes a janky experience.

Let me know if I can assist to get this progressing.

PedroBern commented 3 years ago

@bekatd try using a side-effect-free onViewableItemsChanged, instead of controlling the visibility of your items with state, use an animated value mapped to the opacity. It will prevent unnecessary re-renders and probably make a delightful experience.

bekatd commented 3 years ago

@PedroBern Thanks for the quick response, but it is not clear for me, can you elaborate on this? :)

bekatd commented 3 years ago

How can I get this value use an animated value mapped to the opacity and what is opacity, I don't understand how it deals with my issue?

bekatd commented 3 years ago

I think you misunderstood my goal. I am not trying to animate flatlist scrolling, but want to make video start/stop based on their positions, but I can't use ref for this, there is some complex case which prevents me doing so. So I am interested to just pass some prop to renderItem and decide later when to start/stop video. But scrolling is extremely buggy due to rerenders

PedroBern commented 3 years ago

Forget about it. Probably would be a big workaround. Why do you need this? Maybe what you need is to listen to the scrollY position... it's an open issue #156

bekatd commented 3 years ago

No, but thanks. I already found workaround using refs instead of states and scrolling is now smooth again

samih7 commented 3 years ago

@bekatd Can you share your workaround?

I can reproduce the issue with a simple setup like so:

import * as React from 'react';
import { Text, View } from 'react-native';
import { Tabs } from 'react-native-collapsible-tab-view';

const items = [ ]; // Add 40-50 items with an "id" field here

const ListItem = ({ item, index, isInView }) => {
  return (
    <View
      style={{
        height: 100 + index * 10,
        alignItems: 'center',
        justifyContent: 'center',
        borderBottomColor: 'red',
        borderBottomWidth: 1,
      }}
    >
      <Text>{item.id}</Text>
      <Text>In view: {isInView ? 'YES' : 'NO'}</Text>
    </View>
  );
};

const ListComponent = () => {
  const [itemInViewId, setItemInViewId] = React.useState(null);

  const renderItem = ({ item, index }) => (
    <ListItem item={item} index={index} isInView={itemInViewId === item.id} />
  );

  const onViewableItemsChanged = React.useCallback(
    (info: { viewableItems }): void => {
      const { viewableItems } = info;

      const [viewableItem] = viewableItems;

      if (viewableItem) {
        const { key } = viewableItem;

        setItemInViewId(key);
      } else {
        setItemInViewId(null);
      }
    },
    []
  );

  return (
    <Tabs.FlatList
      keyExtractor={(item) => item.id}
      onViewableItemsChanged={onViewableItemsChanged}
      data={items}
      renderItem={renderItem}
    />
  );
};

const Example = () => {
  return (
    <Tabs.Container>
      <Tabs.Tab name="first">
        <ListComponent />
      </Tabs.Tab>
    </Tabs.Container>
  );
};

export default Example;
bekatd commented 3 years ago

Sure give me some time

bekatd commented 3 years ago
import React, { useRef } from 'react';
import { Tabs } from 'react-native-collapsible-tab-view';

const TabOne = ({ data, ...rest }) => {
    const refs = useRef({});

    const onViewableItemsChanged = useRef(({ changed }) => {
        changed.forEach((item) => {
            refs.current[item.item.id].setVisible(item.isViewable);
        });
    });

    return (
            <Tabs.FlatList
                data={data}
                viewabilityConfig={{ viewAreaCoveragePercentThreshold: 75 }}
                onViewableItemsChanged={onViewableItemsChanged.current}
                renderItem={({ item, index }) => {
                    return (
                        <View
                            ref={(ref) => {
                                refs.current[item.id] = ref;
                            }}
                        />
                    );
                }}
                {...rest}
            />
        );
};

export default TabOne;

hope it helps

To explain what happens here, we just store references to every flatlist item and onViewableItemsChanged just calls some state update function of changed items based on they are viewable or not. With that we only update 1 flatlist item at a time during scroll and thus scroll remains smooth

bekatd commented 3 years ago

@samih7 Did you manage to solve the problem?

samih7 commented 3 years ago

@bekatd Thanks for your answer, but not really. I gave it a try, my use case is exactly the same as yours (auto-playing videos when in view) but using a ref doesn't completely solve the issue – I still get some jumps when scrolling, whereas it's smoother with a normal FlatList. By the way, did you use the useImperativeHandle hook to add the setVisible method on your ref? I think the benefit of this comes down to the parent (where Tabs.FlatList is rendered) not rerending anymore when an item gets into view since we don't set a state in there anymore, right? When it comes to "updating one FlatList item at a time", my understanding is that only the items that get into/out of view will rerender and not the whole list if you wrap your list items with memo

Here is a basic example of how I implemented your approach, please tell me if I'm missing something:

const ListItem = React.forwardRef(({ item, index }, ref) => {
  const [isInView, setIsInView] = React.useState(false);

  React.useImperativeHandle(ref, () => ({
    setVisible: (inView) => {
      setIsInView(inView);
    },
  }));

  return (
    <View
      style={{
        height: 100 + index * 10,
        alignItems: 'center',
        justifyContent: 'center',
        borderBottomColor: 'red',
        borderBottomWidth: 1,
      }}
    >
      <Text>{item.id}</Text>
      <Text>In view: {isInView ? 'YES' : 'NO'}</Text>
    </View>
  );
});

const List = () => {
  const refs = React.useRef({});

  const onViewableItemsChanged = React.useRef(({ changed }) => {
    changed.forEach((item) => {
      refs.current[item.item.id].setVisible(item.isViewable);
    });
  });

  return (
    <Tabs.FlatList
      keyExtractor={(item) => item.id}
      onViewableItemsChanged={onViewableItemsChanged.current}
      viewabilityConfig={{ viewAreaCoveragePercentThreshold: 75 }}
      data={items}
      renderItem={({ item, index }) => (
        <ListItem
          item={item}
          index={index}
          ref={(ref) => {
            refs.current[item.id] = ref;
          }}
        />
      )}
    />
  );
};
newmanz12 commented 3 years ago

@samih7 Did you manage to solve the problem?

Bekatd can you respond to @samih7 using his solution. I am also experiencing some issues still. Can you show us how you implemented the setVisible function for your items

bekatd commented 3 years ago

Hey, sorry for late response. This is how I implemented refs and it works really smooth.

Please copy this code and modify it

    import React, { useRef } from 'react';
    import { Tabs } from 'react-native-collapsible-tab-view';

    const TabOne = ({ data, ...rest }) => {
        const refs = useRef({});

        const onViewableItemsChanged = useRef(({ changed }) => {
            changed.forEach((item) => {
                refs.current[item.item.id]?.setVisible(item.isViewable);
            });
        });

        return (
            <Tabs.FlatList
                data={data}
                keyExtractor={(_, index) => index.toString()}
                renderItem={({ item }) => (
                    <ListItem ref={(ref) => (refs.current[item.id] = ref)} />
                )}
                viewabilityConfig={{ viewAreaCoveragePercentThreshold: 75 }}
                onViewableItemsChanged={onViewableItemsChanged.current}
                {...rest}
            />
        );
    };

    export default TabOne;

PS: please make sure you do not force state updates somewhere else, which could trigger flatlist rerender.

bekatd commented 3 years ago

@newmanz12, about @samih7's code. I doubt it would work (unfortunately not able to test now), since he did not pass correct ref prop value to the ListItem component. If you decide to use useImperativeHandle ensure that component gets ref value, not function which sets refs variable for parent object.

newmanz12 commented 3 years ago

@newmanz12, about @samih7's code. I doubt it would work (unfortunately not able to test now), since he did not pass correct ref prop value to the ListItem component. If you decide to use useImperativeHandle ensure that component gets ref value, not function which sets refs variable for parent object.

Got it. I'm having trouble understanding what you did in ListItem to get your code to run in terms of defining setVisible. Can you post yours?

Included my code below. Currently not working. refs.current[item.item.id] is coming back as undefined. Console logged both refs.current and item.item.id which are coming back with values. However, not able to combine them together

import React, {useState, useEffect, useRef, useMemo, useCallback} from "react";
import {View, FlatList, StatusBar, Dimensions, Text} from 'react-native'
import Post from '../../components/Post'
import SearchResultsMap from '../../screens/SearchResultsMap'
import Global from '../../components/Global'
import {useNavigation} from '@react-navigation/native';

const HomeDisplay = (props) => {

const { posts, users } = props;

const snapToInterval = useMemo(() => Dimensions.get('window').height + StatusBar.currentHeight-48, []);

const refs = useRef();

const onViewableItemsChanged = useRef(({ changed }) => {
        changed.forEach((item) => {
            refs.current[item.item.id]?.setVisible(item.isViewable);
        });
    });

return(
    <View>
        <FlatList 
            ref={refs}
            data = {posts}
            keyExtractor={(item) => item.id}
            renderItem={({item, index}) => <Post post={item}  ref={(ref) => (refs.current[item.id] = ref)}  user = {users} index ={index} />}
            showsVerticalScrollIndicator ={false}
            snapToInterval ={snapToInterval}
            snapToAlignment ={"start"}
            decelrationRate={"fast"}
            viewabilityConfig={{viewAreaCoveragePercentThreshold: 25,}}
            onViewableItemsChanged={onViewableItemsChanged.current}
        />
    </View>
)
}

export default HomeDisplay;

//Post Page below

import React, {useEffect, useState, useImperativeHandle} from 'react';
import{View, Text, TouchableWithoutFeedback, StatusBar, Image, TouchableOpacity, Pressable} from 'react-native';
import Video from 'react-native-video'

const Post = (props) =>
{

    const [paused, setPaused] = useState(false)
    const [muted, setMuted] = useState(false)

      const [isInView, setIsInView] = useState(false);

useImperativeHandle(props.ref, () => ({
     setVisible: (inView) => {
      setIsInView(inView);
    },
  }));

//Deleted lots of code as unnecessary for the example
    return(

//Deleted lots of code as unnecessary for the example
                <View >
                    <Video 
                    source = {{ uri: videoUri}}
                    style = {styles.video}
                    onError = {(e) => console.log(e)}
                    resizeMode ={'cover'}
                    repeat = {true}
                    paused = {!(isInView) || paused}
                   muted = {!(isInView) || muted}
                    />

        </View>

    )
}

export default Post;
bekatd commented 3 years ago

@newmanz12 please indent your code when pasting here, to ensure markup.

And please post minimal code and using my code example above and let me know if it works

andreialecu commented 3 years ago

Closing this since it seems it was caused by excessive re-renders and not a library issue.

samih7 commented 3 years ago

@andreialecu I think it's a library issue, since it doesn't occur with a normal FlatList

bekatd commented 3 years ago

Definitely library issue. In my opinion internal state updates or references cause some excessive number of adjustments (maybe scrolling position calculations) and when adding even more functionality or state updates, multiple concurrent things happen. Like, vissualy jumping scroll back and forth means that some timing function causes it, which is used internally by lib. Anyway great lib, but this issue needs resolve, since its one of the mostly use case in my opinion

andreialecu commented 3 years ago

Alright, PRs welcome.

The code for Tabs.FlatList is pretty simple: https://github.com/PedroBern/react-native-collapsible-tab-view/blob/main/src/FlatList.tsx feel free to dig into it and see if you can find the reason.

vbylen commented 2 years ago

@andreialecu The lag problem is caused by the snapThreshold prop in the useScrollHandlerY hook.

Something in that logic is causing excessive re-renders when scrolling.

When I leave out the snapThreshold prop in <Tabs.Container/> the lag disappears.