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

Flashlist caches scrolling carousels from previous cells #891

Closed
2 tasks done
ClemCornet opened this issue Aug 3, 2023 · 12 comments
Closed
2 tasks done

Flashlist caches scrolling carousels from previous cells #891

ClemCornet opened this issue Aug 3, 2023 · 12 comments
Labels
bug Something isn't working

Comments

@ClemCornet
Copy link

Current behavior

I have a list with cells containing photo carousels. Each time a new batch of items is loaded, the cell keep in cache the scroll of the previous item.

Expected behavior

Is there a way to prevent this cell memoization behavior?

To Reproduce

Create a flashlist with a simple vertical carousel photo inside rows

Platform:

  • iOS
  • Android

Environment

"@shopify/flash-list": "1.4.0"
"react-native": "0.71.8"
"expo": "~48.0.18"
@ClemCornet ClemCornet added the bug Something isn't working label Aug 3, 2023
@ahmetares
Copy link

ahmetares commented Aug 5, 2023

Using react-native-fast-image solved this problem on images for me

@ClemCornet
Copy link
Author

@ahmetares Thank you for your reply. Even using react-native-fast-image (expo-image for me), I still have the same problem, it keep the scroll of previous cards >

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-08-08.at.10.21.45.mp4

I don't know if it's possible to bypass the library's behavior 🤔

@misha-belokon
Copy link

@ClemCornet have you found any solution?

@ClemCornet
Copy link
Author

@misha-belokon No, I'm sorry. I'm back to a flatlist, impossible to change this behavior with a slider in the cells.
I hope a contributor will see this!

@yaminyassin
Copy link

yaminyassin commented Sep 11, 2023

can you add a expo snack with reproducible code. My guess is that this might be a key problem.

@tanlucvo
Copy link

tanlucvo commented Oct 3, 2023

Yupp!! Assume is when Item1 out of screen and Item10 move into screnn, Flashlist will recycle Item1 by data of Item10, and keep current state of Item1, then cause issue above. Solution is reset carousel index when data change example

useEffect(() => {
  // Reset carousel position (view was recycled for another item),method move to index inside carousel. 
  carouselRef.current.moveToIndex(0);
}, [item.id]);

@galibov
Copy link

galibov commented Oct 6, 2023

I encountered a similar issue and resolved it with the following approach:

const flashListRef = useRef(null);

useEffect(() => {
  flashListRef.current?.scrollToOffset({ offset: 0, animated: false });

//items as example you can use id prop or something better
}, [items]);

// ... other code ...

return (
    <FlashList
      ref={flashListRef}
      // ... other props ...
    />
);

@neaRRRRR
Copy link

Giving key for Carousel component solved the issue for me

@ClemCornet
Copy link
Author

@neaRRRRR Thanks a lot for your solution, it appears to be working without any side effects!

@jspizziri
Copy link

jspizziri commented Feb 9, 2024

@ClemCornet I actually think this should be reopened as it seems like something that would ideally be handled in FlashList. AFAIK, using a key isn't ideal as that is against the recommendations of flashlist as it causes another render and reduces performance.

Here's the homebrewed solution that I came up with:

// OuterList is a vertically scrolling list which. Each item will be a horizontally scrolling nested list
const OuterList = () => {
  // ...
  const scrollMap = useRef<Record<string, number | undefined>>({});
  
  return (
    <FlashList
      data={items}
      extraData={scrollMap}
      renderItem={(props) => (
        <InnerList
          item={props.item}
          scrollMap={props.extraData.current}
        />
      )}
    />
  )
}

// The nested, horizontally scrolling list. Recieves the `scrollMap`, updates it, and scrolls appropriately
const InnerList = ({
  item,
  scrollMap,
}: { 
  item: any;
  scrollMap: Record<string, number | undefined>;
}) => {
  // ...
  const flatListRef = useRef<FlashList<CollectionItem> | null>();
  const lastItem = useRef(item);

  // FlashList recycles components but it doesn't properly update their scroll
  // position. So we reset the scroll position here to the last known position.
  // This approach avoids an additional render from `useEffect`.
  if (lastItem.current.id !== item.id) {
    lastItem.current = item.id;
    const position = scrollMap[item.id] || 0;

    console.log(">>>>>>> scrolling to offset", item.id, position);
    flatListRef.current?.scrollToOffset({ offset: position, animated: false });
  }

  return (
    <FlashList
      ref={r => flatListRef.current = r}
      horizontal
      data={item.someData}
      extraData={scrollMap.current}
      renderItem={() => {
        // Item to render
      }}
      // NOTE: we need to track the scroll position
      onScroll={({ nativeEvent: { contentOffset: { x }}}) => {
        scrollMap[item.id] = x;
      }}
    />
  )
}

Anyways, this seems to be working for now. It'd be great if FlashList could take care of this internally somehow.

EDIT: this approach could also be used for a nested carousel, you'd just replace the scrollMap with a map of your carousel positions.

@alyosha
Copy link

alyosha commented Mar 12, 2024

+1 to reopen 🙏

@ClemCornet ClemCornet reopened this Mar 14, 2024
@naqvitalha
Copy link
Collaborator

While we can automatically scroll to offset 0 internally, it may not be the best idea. In a lot of cases people generate new data array across renders and there's no reliable way to predict that the list has been recycled inside a parent list or due to some manipulation by the dev. My suggestion is to just scroll to 0 when the data array changes and make sure data is memoized correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

10 participants