Shopify / flash-list

A better list for React Native
https://shopify.github.io/flash-list/
MIT License
5.51k stars 283 forks source link

`initialScrollIndex` and `estimatedFirstItemOffset` don't work together #671

Open david-arteaga opened 1 year ago

david-arteaga commented 1 year ago

Current behavior

When providing both initialScrollIndex and estimatedFirstItemOffset, the content is not scrolled the right amount. The estimatedFirstItemOffset value is ignored.

This is the recording from this snack: https://snack.expo.dev/@david.arteaga/faulty-initialscrollindex-with-estimatedfirstitemoffset The initialScrollIndex is set to 3, so the Item 3 item should be at the top of the list, but it's not.

https://user-images.githubusercontent.com/7199015/200994419-7488c158-b8d5-4f7f-a784-c8d1999f282c.MP4

If the header is removed then initialScrollIndex behaves as expected.

Expected behavior

The estimatedFirstItemOffset should be considered when calculating the initial offset when initialScrollIndex is provided.

To Reproduce

Run this snack on iOS: https://snack.expo.dev/@david.arteaga/faulty-initialscrollindex-with-estimatedfirstitemoffset

Platform:

I have not tested on Android.

Environment

1.1.0

david-arteaga commented 1 year ago

Also wanted to mention that if initialScrollIndex is set to 0, then the list does render with the correct offset, respecting the header height. That's the only index that works though.

nandorojo commented 1 year ago

I have a similar issue. It actually does get close to the right index, but has a slight flicker for a second.

siddharth-kt commented 1 year ago

any solution guys ?

david-arteaga commented 1 year ago

As a temporary workaround I’ve added an effect that scrolls to the initial index after a delay in mount. I’m using with with a 200ms delay. The scroll view animates the last part of the scroll (around however much the header height is). You could also set animate to false but I think it looks better to make it look like the content actually scrolled to the desired element (at least the last part) than to make the content jump up.

function useScrollToInitialIndexOnce({
  initialScrollIndex,
  shouldScroll,
  flashListRef,
  afterMs,
}: {
  /**
   * Will only scroll once if this value is defined
   */
  initialScrollIndex: number | undefined;
  shouldScroll: boolean;
  flashListRef: React.RefObject<FlashList<any>>;
  afterMs: number;
}) {
  const hasScrolled = useRef(false);
  useEffect(() => {
    if (isDefined(initialScrollIndex) && shouldScroll && !hasScrolled.current) {
      const index = initialScrollIndex;
      hasScrolled.current = true;
      setTimeout(() => {
        flashListRef.current?.scrollToIndex({
          animated: true,
          index,
        });
      }, afterMs);
    }
  }, [shouldScroll, initialScrollIndex, afterMs, flashListRef]);
}

I set the shouldScroll param to whether the data in the list has already loaded, to make sure the scroll call runs after the list has rendered its content and has had a chance to measure its layout.

And I added the ref check because in our app the other params could change during the time that hook is mounted and that shouldn’t cause any additional scrolls.

siddharth-kt commented 1 year ago

@david-arteaga thanks, it seems to work fine. 🙂

fortmarek commented 1 year ago

This seems to be happening on iOS only but looks like a real issue. We'll try looking into this.

dr1verrr commented 1 year ago

This seems to be happening on iOS only but looks like a real issue. We'll try looking into this.

It is also android issue

Zenb0t commented 1 year ago

Same issue here. I have some custom insets and offsets, this seems to affect the position of the card when scrolled to as well.

blyszcz commented 10 months ago

@fortmarek I was wondering if you've had the opportunity to look into this matter. Your assistance would be greatly appreciated. Thanks!

pgk1216 commented 9 months ago

Has anyone figured out a solution to this issue?

hkhawar21 commented 6 months ago

As a temporary workaround I’ve added an effect that scrolls to the initial index after a delay in mount. I’m using with with a 200ms delay. The scroll view animates the last part of the scroll (around however much the header height is). You could also set animate to false but I think it looks better to make it look like the content actually scrolled to the desired element (at least the last part) than to make the content jump up.

function useScrollToInitialIndexOnce({
  initialScrollIndex,
  shouldScroll,
  flashListRef,
  afterMs,
}: {
  /**
   * Will only scroll once if this value is defined
   */
  initialScrollIndex: number | undefined;
  shouldScroll: boolean;
  flashListRef: React.RefObject<FlashList<any>>;
  afterMs: number;
}) {
  const hasScrolled = useRef(false);
  useEffect(() => {
    if (isDefined(initialScrollIndex) && shouldScroll && !hasScrolled.current) {
      const index = initialScrollIndex;
      hasScrolled.current = true;
      setTimeout(() => {
        flashListRef.current?.scrollToIndex({
          animated: true,
          index,
        });
      }, afterMs);
    }
  }, [shouldScroll, initialScrollIndex, afterMs, flashListRef]);
}

I set the shouldScroll param to whether the data in the list has already loaded, to make sure the scroll call runs after the list has rendered its content and has had a chance to measure its layout.

And I added the ref check because in our app the other params could change during the time that hook is mounted and that shouldn’t cause any additional scrolls.

I do not think it will be performant for a large list because the initial scroll can be too much. In my use case, I have paging enabled as well

david-arteaga commented 6 months ago

I’ve used it with a list with about 5k images in rows of 3 (so about 1.6k rows) and it’s worked fine for that. At least I haven’t seen any visual jankyness. Haven’t profiled it though. I did provide heights for all elements (overrideItemLayout).

But yeah, it’s a workaround, not a fix 🥲