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

maintainVisibleContentPosition does not work correctly with multiple instances of a Flatlist #16

Closed
jbernhardt23 opened this issue Apr 22, 2021 · 10 comments · Fixed by #31

Comments

@jbernhardt23
Copy link

jbernhardt23 commented Apr 22, 2021

I'm currently facing an issue where the <Flatlist/> component is failing to maintain its position when being used as a common component between multiple other components. My use case goes the following way. I have a common component <Feed/> that holds common logic and renders this package <Flatlist/>. This <Feed/> component is being used from 2 other components, so there are <Home/> and <Verticals/>, both have an instance of <Feed/> in their render functions. The <Home/> is always mounted, meaning that it is only being unmounted when the app is killed, but the <Verticals/> one is unmounted when the user navigates back from that specific screen and mounted when tapping that screen. The Flatlist works as expected and maintains its position only at the <Home/> screen. When I navigate to the <Verticals/> and update the content for that list, the view is not being maintained.

I noticed that if I do not render the <Feed/> component inside the <Home/> screen and only in the <Verticals/> screen, the list behaves as expected and the view is maintained. I tried to do some debugging but I couldn't find anything useful for me to find a solution (so far). I did inspect flRef.current.getScrollableNode() for the <Verticals/> list and I saw that the values were different for those scenarios. Meaning that when I first loaded the Home list screen and then navigate to the Verticals screen that value was different if I only render the Vertical list screen (when is actually working).

Edit:
One more thing, If I go to the main list in the Home screen (currently working), go to the other instance of the list in the Vertical screen (not working) and then go back to the Home screen, now the list in the Home screen does not mantains its position.

This is the configuration for the list in the common <Feed/>

    <FlatList
        ref={ref}
        style={extraStyles}
        accessibilityLabel="Content Feed"
        ListHeaderComponent={header}
        ListEmptyComponent={LoadingStuff}
        data={feedItems}
        renderItem={renderItem}
        onTouchStart={onTouchStart}
        onViewableItemsChanged={onViewableItemsChanged}
        viewabilityConfig={viewabilityConfig}
        keyboardShouldPersistTaps="handled"
        keyExtractor={keyExtractor}
        onEndReached={onInfiniteScroll}
        onEndReachedThreshold={0}
        showsVerticalScrollIndicator={false}
        onScroll={onScroll}
        scrolled={scrolled}
        maintainVisibleContentPosition={{
          minIndexForVisible: 0,
        }}
        refreshControl={
          <RefreshControl refreshing={isRefreshing} onRefresh={onRefresh} />
        }
      />
@jbernhardt23
Copy link
Author

After debugging the native module I think I found something around the native ui listeners. Since I always have a Flatlist mounted/in the background, those native ui listeners are always alive even when I mount the new screen with the other instance of the Flatlist, thats because disableMvcp is never called in the first Flatlist in the Home screen. I believe this is causing the ui values getting mixed from the first list and second list and the native module is not able to mantain the correct position for each one. I will keep debugging/

@jbernhardt23 jbernhardt23 changed the title Flatlist not maintaining position maintainVisibleContentPosition does not work correctly with multiple instances of a Flatlist Apr 27, 2021
@jbernhardt23
Copy link
Author

After more days of debugging, I believe I have found the root cause and a possible solution (still testing). So the problem with the current native implementation is that it will only work if you only ever have one instance of a Flatlist mounted in a component. When the Flatlist component is mounted, it attaches native UI listeners to that react-native node in order to maintain its position if the view changes, however, if you for example have Flatlist A mounted and it never gets unmounted but then open a new screen that loads Flatlist B, the native UI listeners will now be referencing the node for Flatlist B. If you go back to Flatlist A, the listeners will still be pointing to Flatlist B and vice-versa. The reason why this is happening is that in the native module the variables that are holding the values for a given Flatlist are being stored in static fields, meaning that they will always be overridden by the next time enableMaintainVisibleContentPosition. In order to make this library support multiple instance of Flatlist, it should keep a reference of all the Flatlist that are being enabled and update their current values accordingly. I'm working on this proposal and will open a PR for thoughts.

@vishalnarkhede
Copy link
Collaborator

Hey @jbernhardt23 I have got around to looking into this issue yet. But yeah, if you have some PR for it, that would be great :)

Thanks!!

@jbernhardt23
Copy link
Author

Hey @jbernhardt23 I have got around to looking into this issue yet. But yeah, if you have some PR for it, that would be great :)

Thanks!!

I have forked the project and applied those changes there. That work is still being reviewed and tested on my team, once it gets approved I will open a PR.

@bradnorman5490
Copy link

Hey @jbernhardt23, any update on PR for this ?

@jbernhardt23
Copy link
Author

hey @bradnorman5490 ! yes, we have finished doing extensive testing around this on our side so I will be opening a PR shortly (tomorrow or starting next week)

@bradnorman5490
Copy link

Fantastic news! Looking forward to it

@vishalnarkhede
Copy link
Collaborator

vishalnarkhede commented Jun 5, 2021

@jbernhardt23 thats nice to hear. Love it how open source contributions are helping the community :)

@jbernhardt23
Copy link
Author

@bradnorman5490 PR was submitted #21

@vishalnarkhede
Copy link
Collaborator

THanks @bradnorman5490 I will take a look tomorrow :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants