Make one pixel beyond the scrollbar thumb react correctly to clicks #505
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
m-sasha
changed the title
MatkovIvan
approved these changes
Apr 28, 2023
...se/foundation/foundation/src/desktopTest/kotlin/androidx/compose/foundation/ScrollbarTest.kt
Outdated
Show resolved
Hide resolved
...se/foundation/foundation/src/desktopTest/kotlin/androidx/compose/foundation/ScrollbarTest.kt
Outdated
Show resolved
Hide resolved
...se/foundation/foundation/src/desktopTest/kotlin/androidx/compose/foundation/ScrollbarTest.kt
Outdated
Show resolved
Hide resolved
...se/foundation/foundation/src/desktopTest/kotlin/androidx/compose/foundation/ScrollbarTest.kt
Outdated
Show resolved
Hide resolved
...se/foundation/foundation/src/desktopTest/kotlin/androidx/compose/foundation/ScrollbarTest.kt
Outdated
Show resolved
Hide resolved
|
@MatkovIvan What do you mean by test app? The change is only about one row of pixels, so it's hard to notice in a real app. It's noticeable in unit tests, though. |
m-sasha
force-pushed
the
m-sasha/scrollbar-thumb-last-pixel
branch
from
a4a050f
to
4bdd0a1
Compare
eymar
pushed a commit
that referenced
this pull request
May 25, 2023
…aging-compose In Multiplatform Paging, the classes `DirectDispatcher` and `TestDispatcher` in `testutils-ktx` had to be multiplatformized to allow dependent tests in `paging-compose`, `paging-common`, and `paging-runtime` to be moved to the respective `commonTest` folders. I'm attempting to replace all usages of `DirectDispatcher` and `TestDispatcher` that Multiplatform Paging depends on with those found in `kotlinx.coroutines.test`, so hopefully the `testutils-ktx` dependency no longer has to be multiplatformized in my fork. I'm starting this effort by doing it with `paging-compose`. Test: ./gradlew test connectedCheck Bug: 270612487 This is an imported pull request from androidx#505. Resolves #505 Github-Pr-Head-Sha: 002b706 GitOrigin-RevId: 2b32273 Change-Id: Id018cf4bf64dac3154eff2ea98e6145aff4290fc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The detection of scrollbar track clicks used the scrollbar thumb's "raw" (floating-point) bounds. However, this misses the first pixel after the thumb, because, for example, the value
10is in the floating point range0..10.0, but the actual pixel bounds of the thumb for that range would not include the 10th pixel (the size of the thumb would be10px, so its pixel range would be0px..9px).Proposed Changes
Size the scrollbar in integer pixels and use that size when detecting clicks on the scrollbar track (outsize the thumb).
Testing
Test: Added a new unit test to test the last pixel (and one beyond) of the thumb, and fixed other tests which used to pass because the one-pixel-beyond the thumb was neither clickable nor draggable.