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

Fix shaky scrolling of LazyColumn when the items are of varying size #362

Merged
merged 1 commit into from Jan 4, 2023

Conversation

m-sasha
Copy link

@m-sasha m-sasha commented Jan 3, 2023

Proposed Changes

This mostly re-implements commit

793810ba245 25/01/2022, 23:12 Desktop Scrollbar: fix position of a thumb while dragging

which broke smooth content scrolling in LazyColumn when not all items are of the same size.
The problem with that commit was that on each mouse-drag event it did (conceptually)

dragPosition += dragDelta
contentPosition = dragPosition/scrollScale

but when some items are larger than others scrollScale can change significantly between the mouse-drag events, so you could have contentPosition decrease even when dragDelta was always positive.
What we want to do instead is to always add to contentPosition:

contentPosition += dragDelta/scrollScale

The problem that commit was attempting to solve (JetBrains/compose-multiplatform#643) is solved in this commit in a different way:

val sliderDelta = 
    (dragPosition + dragDelta).coerceIn(0f, maxScrollPosition) - 
        dragPosition.coerceIn(0f, maxScrollPosition)
contentPosition += sliderDelta/scrollScale
dragPosition += dragDelta

Note that this trades-off smooth thumb scrolling for smooth content scrolling, as you can't currently have both (because LazyScrollbarAdapter.scrollOffset is discontinuous)

Testing

Test: Manually on the reproducer submitted in the bug report, and also via unit tests.

Note that the existing unit test was faulty in the sense that it passed even in the presence of the behavior the commit was meant to fix. It was faulty in two ways: the mouse events it was generating were missing the scrollbar thumb, and it was scrolling in the wrong direction for the test.

Issues Fixed

Fixes: JetBrains/compose-multiplatform#2337

@m-sasha m-sasha force-pushed the bugfix/compose-jb-2338-shaky-lazycolumn-scrollbar branch from 3632ecd to d8fcb2f Compare January 3, 2023 20:08
@m-sasha
Copy link
Author

m-sasha commented Jan 4, 2023

Behavior before:

before.mp4

Behavior after:

after.mp4

@m-sasha m-sasha force-pushed the bugfix/compose-jb-2338-shaky-lazycolumn-scrollbar branch from d8fcb2f to eefae8b Compare January 4, 2023 10:16
@m-sasha m-sasha merged commit de3c710 into jb-main Jan 4, 2023
@m-sasha m-sasha deleted the bugfix/compose-jb-2338-shaky-lazycolumn-scrollbar branch January 4, 2023 19:14
eymar pushed a commit that referenced this pull request Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants