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

initialScrollIndex and estimatedFirstItemOffset don't work together #671

Open
1 of 2 tasks
david-arteaga opened this issue Nov 10, 2022 · 13 comments
Open
1 of 2 tasks
Labels
bug Something isn't working P0

Comments

@david-arteaga
Copy link

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.

RPReplay_Final1668051117.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:

  • iOS
  • Android

I have not tested on Android.

Environment

1.1.0

@david-arteaga david-arteaga added the bug Something isn't working label Nov 10, 2022
@david-arteaga
Copy link
Author

david-arteaga commented Nov 10, 2022

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
Copy link

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

@siddharth-kt
Copy link

any solution guys ?

@david-arteaga
Copy link
Author

david-arteaga commented Jan 6, 2023

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
Copy link

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

@fortmarek fortmarek added the P0 label Apr 11, 2023
@fortmarek
Copy link
Contributor

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

@dr1verrr
Copy link

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
Copy link

Zenb0t commented Jul 17, 2023

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
Copy link

blyszcz commented Dec 6, 2023

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

@pgk1216
Copy link

pgk1216 commented Jan 15, 2024

Has anyone figured out a solution to this issue?

@hkhawar21
Copy link

hkhawar21 commented Apr 8, 2024

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
Copy link
Author

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 🥲

@affectful
Copy link

I'm using scrollToOffset instead and calculating the offset I need manually

const useFlashListScrollOffsetWorkaround = (
  flashListRef: React.RefObject<FlashList<unknown>>,
  scrollOffset: number,
) => {
  useEffect(() => {
    // timeout 200ms because scrolling immediately before list calculations
    // causes problems with scrolling to elements at the end of a list
    const timeout = setTimeout(() => {
      flashListRef.current?.scrollToOffset({
        offset: scrollOffset,
        animated: true,
      })
    }, 200)
    return () => clearTimeout(timeout)
  }, [])
}

However, the 200ms was too small on web. I needed to bump it up to 500ms on an MBP M1X, so it'll be even flakier on smaller devices. Anyone know a timing signal I can use to determine readiness?

@naqvitalha this has been open for two years and still quite broken with a deep rooted issue in RecyclerListView. Any plans to fix it soon?

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

No branches or pull requests

10 participants