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

[Bug]: Scrollbar is hardly stable #1526

Open
3 tasks done
Jaehwa-Noh opened this issue Jul 3, 2024 · 8 comments
Open
3 tasks done

[Bug]: Scrollbar is hardly stable #1526

Jaehwa-Noh opened this issue Jul 3, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@Jaehwa-Noh
Copy link
Contributor

Jaehwa-Noh commented Jul 3, 2024

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
@Jaehwa-Noh Jaehwa-Noh added the bug Something isn't working label Jul 3, 2024
@SimonMarquis
Copy link
Contributor

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

@Jaehwa-Noh
Copy link
Contributor Author

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

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

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.

@rodrigodiasferreira
Copy link

rodrigodiasferreira commented Jul 15, 2024

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

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:

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

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

akp660 commented Oct 10, 2024

ill try to fix this scroll issue.

@mtotschnig
Copy link

mtotschnig commented Dec 21, 2024

The scrollbar also jumps on a simple LazyList when a long tap is happening within the thumb. The reason seems that the position of the gesture is translated into the new top position of the thumb. Attached is a video of a preview composable using the scrollbar with draggable thumb. In second 3, I am long tapping toward the bottom edge of the thumb.
1526.webm

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

6 participants