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 the direction of scrolling when pressing on the scrollbar track with reverseLayout=true #1221

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,29 @@ class ScrollbarTest {
rule.onNodeWithTag("box0").assertTopPositionInRootIsEqualTo(-300.dp)
}

@Theory
fun `press on track outside slider with reverseLayout`(
scrollbarProvider: ScrollbarProvider
) {
rule.setContent(scrollbarProvider) {
TestBox(
size = 100.dp,
childSize = 50.dp,
childCount = 3,
scrollbarWidth = 10.dp,
reverseLayoutAndScrolling = true
)
}

rule.onNodeWithTag("box2").assertTopPositionInRootIsEqualTo(50.dp)

rule.onNodeWithTag("scrollbar").performMouseInput {
click(Offset(5f, 1f))
}

rule.onNodeWithTag("box1").assertTopPositionInRootIsEqualTo(50.dp)
}

@Theory
fun `dynamically change content then drag slider to the end`(
scrollbarProvider: ScrollbarProvider
Expand Down Expand Up @@ -1235,11 +1258,15 @@ class ScrollbarTest {
childSize: Dp,
childCount: Int,
scrollbarWidth: Dp,
scrollbarHeight: Dp = size
scrollbarHeight: Dp = size,
reverseLayoutAndScrolling: Boolean = false
) = withTestEnvironment {
Box(Modifier.size(size)) {
Column(
Modifier.fillMaxSize().testTag("column").verticalScroll(scrollState)
Modifier
.fillMaxSize()
.testTag("column")
.verticalScroll(scrollState, reverseScrolling = reverseLayoutAndScrolling)
) {
repeat(childCount) {
Box(Modifier.size(childSize).testTag("box$it"))
Expand All @@ -1248,6 +1275,7 @@ class ScrollbarTest {

ScrollbarProviderLocal.current.VerticalScrollbar(
scrollState = scrollState,
reverseLayout = reverseLayoutAndScrolling,
modifier = Modifier
.width(scrollbarWidth)
.height(scrollbarHeight)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ internal fun <T> OldOrNewScrollbar(
},
modifier
.hoverable(interactionSource = interactionSource)
.scrollOnPressTrack(isVertical, sliderAdapter),
.scrollOnPressTrack(isVertical, reverseLayout, sliderAdapter),
measurePolicy
)
}
Expand Down Expand Up @@ -785,12 +785,13 @@ interface ScrollbarAdapter {

}

private fun thumbPixelRange(sliderAdapter: SliderAdapter): IntRange {
val start = sliderAdapter.position.roundToInt()
val endExclusive = start + sliderAdapter.thumbSize.roundToInt()
private val SliderAdapter.thumbPixelRange: IntRange
get() {
val start = position.roundToInt()
val endExclusive = start + thumbSize.roundToInt()

return start until endExclusive
}
return (start until endExclusive)
}

private val IntRange.size get() = last + 1 - first

Expand All @@ -800,8 +801,7 @@ private fun verticalMeasurePolicy(
scrollThickness: Int
) = MeasurePolicy { measurables, constraints ->
setContainerSize(constraints.maxHeight)
val pixelRange = thumbPixelRange(sliderAdapter)

val pixelRange = sliderAdapter.thumbPixelRange
val placeable = measurables.first().measure(
Constraints.fixed(
constraints.constrainWidth(scrollThickness),
Expand All @@ -819,8 +819,7 @@ private fun horizontalMeasurePolicy(
scrollThickness: Int
) = MeasurePolicy { measurables, constraints ->
setContainerSize(constraints.maxWidth)
val pixelRange = thumbPixelRange(sliderAdapter)

val pixelRange = sliderAdapter.thumbPixelRange
val placeable = measurables.first().measure(
Constraints.fixed(
pixelRange.size,
Expand Down Expand Up @@ -865,11 +864,12 @@ private fun Modifier.scrollbarDrag(

private fun Modifier.scrollOnPressTrack(
isVertical: Boolean,
reverseLayout: Boolean,
sliderAdapter: SliderAdapter,
) = composed {
val coroutineScope = rememberCoroutineScope()
val scroller = remember(sliderAdapter, coroutineScope) {
TrackPressScroller(coroutineScope, sliderAdapter)
val scroller = remember(sliderAdapter, coroutineScope, reverseLayout) {
TrackPressScroller(coroutineScope, sliderAdapter, reverseLayout)
}
Modifier.pointerInput(scroller) {
detectScrollViaTrackGestures(
Expand All @@ -884,7 +884,8 @@ private fun Modifier.scrollOnPressTrack(
*/
private class TrackPressScroller(
private val coroutineScope: CoroutineScope,
private val sliderAdapter: SliderAdapter
private val sliderAdapter: SliderAdapter,
private val reverseLayout: Boolean,
) {

/**
Expand All @@ -906,10 +907,10 @@ private class TrackPressScroller(
* Calculates the direction of scrolling towards the given offset (in pixels).
*/
private fun directionOfScrollTowards(offset: Float): Int {
val pixelRange = thumbPixelRange(sliderAdapter)
val pixelRange = sliderAdapter.thumbPixelRange
return when {
offset < pixelRange.first -> -1
offset > pixelRange.last -> 1
offset < pixelRange.first -> if (reverseLayout) 1 else -1
offset > pixelRange.last -> if (reverseLayout) -1 else 1
Comment on lines +912 to +913
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This the crux of the fix.

else -> 0
}
}
Expand Down
Loading