Fix long text field overscroll effect not clipped correctly (#859)

## Proposed Changes Make CupertinoOverscrollEffect add `clip` Modifier before `offset` if requested by a using widget. Some widgets (like LazyList) do it internally, so adding it unconditionally will be redundant. ## Testing Test: reported case should clip correctly now. ## Before https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/2158106e-f782-4824-b46c-c576859c6d0e ## After https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/da8ebf8a-4106-4d19-9bac-49963eae3977 <img width="413" alt="Screenshot 2023-10-04 at 11 48 36" src="https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/eb0523b3-c412-4bf7-bbf5-6b5a94cc45ac"> ## API change `CupertinoOverscrollEffect` (uikit source set) now has an additional argument `val applyClip: Boolean` in the constructor. `CupertionOverscrollEffect` is marked as `@ExperimentalFoundationApi` now. --------- Co-authored-by: dima.avdeev <[email protected]>
JetBrains · Oct 4, 2023 · 5b48791 · 5b48791
1 parent 7bc20a9
commit 5b48791
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 8 deletions.
diff --git a/...oundation/foundation/src/uikitMain/kotlin/androidx/compose/foundation/Overscroll.uikit.kt b/...oundation/foundation/src/uikitMain/kotlin/androidx/compose/foundation/Overscroll.uikit.kt
@@ -26,12 +26,17 @@ import androidx.compose.ui.platform.LocalLayoutDirection
 @OptIn(ExperimentalFoundationApi::class)
 @Composable
 internal actual fun rememberOverscrollEffect(): OverscrollEffect =
+    rememberOverscrollEffect(applyClip = false)
+
+@OptIn(ExperimentalFoundationApi::class)
+@Composable
+internal fun rememberOverscrollEffect(applyClip: Boolean): OverscrollEffect =
     if (UiKitScrollConfig.isRubberBandingOverscrollEnabled) {
         val density = LocalDensity.current.density
         val layoutDirection = LocalLayoutDirection.current
 
         remember(density, layoutDirection) {
-            CupertinoOverscrollEffect(density, layoutDirection)
+            CupertinoOverscrollEffect(density, layoutDirection, applyClip)
         }
     } else {
         NoOpOverscrollEffect

diff --git a/...n/src/uikitMain/kotlin/androidx/compose/foundation/cupertino/CupertinoOverscrollEffect.kt b/...n/src/uikitMain/kotlin/androidx/compose/foundation/cupertino/CupertinoOverscrollEffect.kt
@@ -25,6 +25,7 @@ import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.draw.clipToBounds
 import androidx.compose.ui.geometry.Offset
 import androidx.compose.ui.geometry.Size
 import androidx.compose.ui.input.nestedscroll.NestedScrollSource
@@ -60,14 +61,20 @@ private data class CupertinoOverscrollAvailableDelta(
     val newOverscrollValue: Float
 )
 
-@OptIn(ExperimentalFoundationApi::class)
+/**
+ * CupertinoOverscrollEffect
+ *
+ * @param density to be taken into consideration during computations;
+ * Cupertino formulas use DPs, and scroll machinery uses raw values.
+ *
+ * @param applyClip Some consumers of overscroll effect apply clip by themselves and some don't,
+ * thus this flag is needed to update our modifier chain and make the clipping correct in every case while avoiding redundancy
+ */
+@ExperimentalFoundationApi
 class CupertinoOverscrollEffect(
-    /*
-     * Density to be taken into consideration during computations; Cupertino formulas use
-     * DPs, and scroll machinery uses raw values.
-     */
     private val density: Float,
-    layoutDirection: LayoutDirection
+    layoutDirection: LayoutDirection,
+    val applyClip: Boolean
 ) : OverscrollEffect {
     /*
      * Direction of scrolling for this overscroll effect, derived from arguments during
@@ -117,10 +124,18 @@ class CupertinoOverscrollEffect(
         .onPlaced {
             scrollSize = it.size.toSize()
         }
+        .clipIfNeeded()
         .offset {
             visibleOverscrollOffset
         }
 
+    private fun Modifier.clipIfNeeded(): Modifier =
+        if (applyClip) {
+            clipToBounds() then this
+        } else {
+            this
+        }
+
     private fun NestedScrollSource.toCupertinoScrollSource(): CupertinoScrollSource? =
         when (this) {
             NestedScrollSource.Drag -> CupertinoScrollSource.DRAG

diff --git a/...foundation/src/uikitMain/kotlin/androidx/compose/foundation/text/TextFieldScroll.uikit.kt b/...foundation/src/uikitMain/kotlin/androidx/compose/foundation/text/TextFieldScroll.uikit.kt
@@ -41,7 +41,7 @@ import kotlin.math.roundToInt
 @ExperimentalFoundationApi
 @Composable
 internal actual fun rememberTextFieldOverscrollEffect(): OverscrollEffect? =
-    rememberOverscrollEffect()
+    rememberOverscrollEffect(applyClip = true)
 
 internal actual fun Modifier.textFieldScroll(
     scrollerPosition: TextFieldScrollerPosition,