Fix the direction of scrolling when pressing on the scrollbar track w…

…ith `reverseLayout=true` (#1221)
JetBrains · Mar 25, 2024 · 66c4e0d · 66c4e0d
1 parent c574c9b
commit 66c4e0d
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 18 deletions.
diff --git a/...foundation/foundation/src/desktopTest/kotlin/androidx/compose/foundation/ScrollbarTest.kt b/...foundation/foundation/src/desktopTest/kotlin/androidx/compose/foundation/ScrollbarTest.kt
@@ -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
@@ -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"))
@@ -1248,6 +1275,7 @@ class ScrollbarTest {
 
             ScrollbarProviderLocal.current.VerticalScrollbar(
                 scrollState = scrollState,
+                reverseLayout = reverseLayoutAndScrolling,
                 modifier = Modifier
                     .width(scrollbarWidth)
                     .height(scrollbarHeight)

diff --git a/...foundation/foundation/src/skikoMain/kotlin/androidx/compose/foundation/Scrollbar.skiko.kt b/...foundation/foundation/src/skikoMain/kotlin/androidx/compose/foundation/Scrollbar.skiko.kt
@@ -421,7 +421,7 @@ internal fun <T> OldOrNewScrollbar(
         },
         modifier
             .hoverable(interactionSource = interactionSource)
-            .scrollOnPressTrack(isVertical, sliderAdapter),
+            .scrollOnPressTrack(isVertical, reverseLayout, sliderAdapter),
         measurePolicy
     )
 }
@@ -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
 
@@ -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),
@@ -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,
@@ -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(
@@ -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,
 ) {
 
     /**
@@ -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
             else -> 0
         }
     }