[Bug]: Scrollbar is hardly stable

[Jaehwa-Noh]

Is there an existing issue for this?

• I have searched the existing issues

Is there a StackOverflow question about this issue?

• I have searched StackOverflow

What happened?

Scrollbar is hardly stable.
When I drag down and drag up, you can see the scrollbar position jump other place.
Screen_recording_20240703_165416.webm

2024-07-03.5.00.07.mov

Relevant logcat output

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[SimonMarquis]

Interesting, is it only present on large screen devices, or also on regular phone width?

[Jaehwa-Noh]

@SimonMarquis
I'd not tested it on regular and tablet yet.
What I used a device in the video was Resizable API34 emulator with Foldable size.

[anhtuannd]

I think it happens due to there is one spacer at the end of stagger list.

nowinandroid/feature/foryou/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt

Line 200 in 50b13ec

item(span = StaggeredGridItemSpan.FullLine, contentType = "bottomSpacing") {

Since ScrollBar (thumb position and thumb size) is calculated based on first visible items and percentage of visible items, not by real position, and all other items are long layout, it causes miscalculating when you scroll to end of the list.

nowinandroid/core/designsystem/src/main/kotlin/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/ScrollbarExt.kt

Line 204 in 50b13ec

val itemsVisible = visibleItemsInfo.floatSumOf { itemInfo ->

[rodrigodiasferreira]

Hi @anhtuannd , @Jaehwa-Noh , @SimonMarquis ,
I've done some investigation about this issue, the reason for this behavior is indeed the itemsVisible and the Spacer as commented by @anhtuannd .
When it is close to the end, and in one of the lanes there is no additional news resource, then the StaggeredGridLayoutInfo.visibleItemsInfo list the Spacer, which is way down, which makes the calculation logic of the items visible broke, since the Spacer is out of the viewport. And it happens also for some news resource that are before the viewport and the StaggeredGridLayoutInfo.visibleItemsInfo also list them.
In order to make it smoother this behavior, I added some safeguard to do not consider the size of such items that are outside the viewport. That fixed the issue, you guys may review and test, when you got some time.
If there is any improvement that you see, please let me know. Here it is the PR that fixes this issue: #1553

Since, I think it is weird the StaggeredGridLayoutInfo.visibleItemsInfo list the items that are out of the viewport, as the name says, it should list the visible items (if it is out of the viewport, it is not visible), so I opened an issue to compose team to take a look into this, and if they agree it is an issue, so they can be aware and fix, when they have manpower for that: https://issuetracker.google.com/issues/353143657

There is still one improvement to be done (but I think it is another issue, may I open a new one, or you guys prefer to be handled on this one?).
I will investigate: it is the scenario when some visibleItemsInfo is not visible anymore, the function: interpolateFirstItemIndex is not having a continuous index continuation, so one may see still some small jumps.
I will check, if I can came up with some equation that may keep it smooth on those scenarios, and improve even more the scroll bar for multi lane scenario.

@anhtuannd , about the improvement that I've done on the PR: #1550, actually this fix, that I've done, it makes sense to use the safeguard of not using the items out of the viewport for the types: LazyListState, LazyGridState, LazyStaggeredGridState, as you can see on the PR, it was fixed on the common function: itemVisibilityPercentage. So, the improvement is still valid. I will add this comment on that PR too, to answer to you. Thanks for raising this concern.

[rodrigodiasferreira]

Hi @anhtuannd , @Jaehwa-Noh , @SimonMarquis ,
I was able to fix the other part of the issue related to the interpolated index.
This change, I wasn't able to make it independent PR, as it uses the genericScrollbarState function that I created before. I had to make the fix on top of the refactoring:

• PR: Improving the reusability and maintainability in ScrollbarExt.kt file and Fix the scroll bar thumb jumping a little bit due to lack of smothness on interpolatedIndex #1550
• commit: 2fd6bb4

@anhtuannd , this one also, did not require to separate the function: genericScrollbarState, so I think the refactoring is still a good improvement to reduce logic and code repetition.

On my tests, it looks pretty smooth now.
If you find any problems, please let me know.

Thanks.

[rodrigodiasferreira]

Hi @anhtuannd , @Jaehwa-Noh , @SimonMarquis,
Just giving an update about the issue that I opened to compose team: https://issuetracker.google.com/issues/353143657, they just fixed. As soon as it is available on some version, will update the PR in order to update the library, and remove my fix related to the visibleItems, probably it won't be necessary anymore.

[akp660]

ill try to fix this scroll issue.