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

Discussion: scrollToIndex precision #24

Open
mbilenko-florio opened this issue Nov 28, 2024 · 10 comments · Fixed by #43
Open

Discussion: scrollToIndex precision #24

mbilenko-florio opened this issue Nov 28, 2024 · 10 comments · Fixed by #43

Comments

@mbilenko-florio
Copy link
Contributor

mbilenko-florio commented Nov 28, 2024

Currently scrollToItem works precisely only when getEstimatedItemSize callback is provided for each list item. This is either not possible or not very convenient for most cases.

On the following picture, let's imagine we need to scroll to "Item 9" in the list. Obtaining y position of item 9 is not straightforward, because to calculate scroll position we need to render and measure "item6", "item 7" and "item 9".

Screenshot 2024-11-28 at 10 10 52

Naive solution here can be to delay scroll, until those missing items are rendered and measured. But it also looks bit complicated to implement. It will also be a problem, if there is 500 items before current and scrollToItem position. Therefore I'm opening this issue to brainstorm possible solution for a scrollTo algorithm.

@mbilenko-florio mbilenko-florio changed the title [DISCUSSION] scrollToIndex precision Discussion: scrollToIndex precision Nov 28, 2024
@MateWW
Copy link

MateWW commented Nov 28, 2024

Thats very interesting thread.

I'm wondering how hard it would be if we could scroll to estimated location(based on estimatedItemSize) and enforce that item to be rendered there.

In that case we would need some kind of bidirectional list which would allow us to scroll back to first item.

The issue I'm seeing which may happen is some wired jumping while we will mount prepending items.
Actually quite curious if that could be possible.

@mbilenko-florio
Copy link
Contributor Author

mbilenko-florio commented Nov 28, 2024

@MateWW very precisely described! Either we need to correct position when user scrolls closer to the screen edges(which will result most likely in the content jump. Or to have some sort of bidirectional scrollview where we can adjust top and bottom edges dynamically.
One of implementations of bidirectional scrollview can be found there: https://github.com/GetStream/react-native-bidirectional-infinite-scroll

@jmeistrich
Copy link
Contributor

This is a tough one. I attempted it in an early prototype but it only worked in certain scenarios, and only on iOS (it depended on contentInset).

The specific problem I was trying to solve was when starting with an initialScrollIndex, to be able to scroll up without any visual jumping. I managed to get it to work on iOS by adding a padding element to the top when size was less than estimated size and offsetting it by contentInset, but that only worked on iOS because Android doesn't support contentInset. And it didn't work at all if size was greater than estimated size.

I also tried adjusting scroll to compensate, but it seemed to sometimes be 1 frame off so it would flash to an incorrect scroll position for 1 frame and then to the correct place.

But then I tried the other list libraries and they didn't solve it either, so I punted it to focus on the basics first.

So I think the best solution to this question would be to scroll to the estimated position, and when actual sizes come in above the current scroll positions, either adjust scroll or a top padding (or something) so that it visually looks correct.

I'll revisit this idea after a v1 release, or if someone wants to attempt a solution to it, I would be happy to advise!

@alizahid
Copy link

alizahid commented Dec 1, 2024

and enforce that item to be rendered there

Love this idea!

A potential solution for the weird jumping could be a virtual window where when scrolling to an index, the item is rendered anywhere possible and the list just scrolls to it. Previous and next items are then rendered before and after to accommodate.

FlashList does a decent enough job but if the index is way out of draw distance, it will just scroll based on estimatedItemSize and calls it a day. FlatList just fails.

@AronBe
Copy link

AronBe commented Dec 2, 2024

Yeah as mentioned, Flashlist has this issue as well, it will just estimate where it should land and usually fails if you have items of different sizes, it does well with equal size items.

Flatlist can actually do it precisely even for different sizes, but you have set initialNumToRender to the index + 1 your are looking for, which is obviously performance hell in long lists. Difficult one to crack, fingers crossed 🤞

@jmeistrich
Copy link
Contributor

I think I've cracked it 🎉. I'm just finalizing some edge cases around it before releasing it.

@mbilenko-florio
Copy link
Contributor Author

@jmeistrich I think this issue is not completely finished, because we solved only part related to the initialScrollIndex, but when using ref.scrollToIndex, it will still land to incorrect position. We still need to add code to choose new anchor element when scrolling to position.

@jmeistrich
Copy link
Contributor

That's when animating the scroll, right? Because sizes change during the scroll so the target position is incorrect? Agreed, this isn't finished until that is solved 👍

@alizahid
Copy link

@jmeistrich Could this be included in v1?

@michbil
Copy link
Contributor

michbil commented Jan 19, 2025

This would work on top of maintainVisibleContentPosition. Groundwork for this task is already done, what’s left is to detect if item to be displayed does not have connection through the measured items to the anchor element, if connection not found, make this item new anchor and use estimated position as its coordinate.

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