[go: nahoru, domu]

Add test and temporary fix for padding placement issue in Rtl

There is an issue where the padding will not correctly position the wrapped layout when it is placed in between layout direction modifiers. This Cl adds a test for this use case. Also it temporary falls back to a previous logic where we manually handle the rtl placement in padding modifier by checking the layout direction and calling placeAbsolute with calculated position.

Bug: 153317665
Test: new test and old tests passed
Change-Id: Ib1ef45d7e67721d6c1941700bbecb9f8ecf4604c
diff --git a/ui/ui-layout/src/androidTest/java/androidx/ui/layout/test/LayoutPaddingTest.kt b/ui/ui-layout/src/androidTest/java/androidx/ui/layout/test/LayoutPaddingTest.kt
index 03c26b8..dfd1be4 100644
--- a/ui/ui-layout/src/androidTest/java/androidx/ui/layout/test/LayoutPaddingTest.kt
+++ b/ui/ui-layout/src/androidTest/java/androidx/ui/layout/test/LayoutPaddingTest.kt
@@ -22,12 +22,18 @@
 import androidx.ui.core.Layout
 import androidx.ui.core.LayoutCoordinates
 import androidx.ui.core.Modifier
+import androidx.ui.core.Ref
+import androidx.ui.core.onChildPositioned
 import androidx.ui.core.onPositioned
+import androidx.ui.core.positionInRoot
+import androidx.ui.layout.Column
 import androidx.ui.layout.DpConstraints
 import androidx.ui.layout.LayoutPadding
 import androidx.ui.layout.Row
 import androidx.ui.layout.Stack
 import androidx.ui.layout.aspectRatio
+import androidx.ui.layout.fillMaxSize
+import androidx.ui.layout.ltr
 import androidx.ui.layout.padding
 import androidx.ui.layout.preferredSize
 import androidx.ui.layout.preferredWidth
@@ -41,6 +47,7 @@
 import androidx.ui.unit.min
 import androidx.ui.unit.px
 import androidx.ui.unit.toPx
+import androidx.ui.unit.toPxSize
 import org.junit.Assert
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
@@ -281,6 +288,48 @@
         assertEquals(IntPxSize(size, size), childSize[2])
     }
 
+    @Test
+    fun testPaddingRtl_whenBetweenLayoutDirectionModifiers() = with(density) {
+        val padding = 50.ipx
+        val size = 300.ipx
+        val paddingDp = padding.toDp()
+        val latch = CountDownLatch(1)
+        val resultPosition = Ref<PxPosition>()
+        val resultSize = Ref<IntPxSize>()
+
+        show {
+            Column(Modifier.rtl) {
+                Stack(Modifier
+                    .preferredSize(size.toDp())
+                    .ltr
+                    .padding(start = paddingDp)
+                    .rtl
+                    .onChildPositioned {
+                        resultPosition.value = it.positionInRoot
+                        resultSize.value = it.size
+                        latch.countDown()
+                    }
+                ) {
+                    Stack(Modifier.fillMaxSize()) {}
+                }
+            }
+        }
+
+        assertTrue(latch.await(1, TimeUnit.SECONDS))
+        val root = findOwnerView()
+        waitForDraw(root)
+        val rootWidth = root.width.ipx
+
+        assertEquals(
+            IntPxSize(size - padding, size).toPxSize(),
+            resultSize.value?.toPxSize()
+        )
+        assertEquals(
+            PxPosition(rootWidth - size + padding, 0.ipx),
+            resultPosition.value
+        )
+    }
+
     private fun testPaddingIsAppliedImplementation(
         padding: Dp,
         paddingContainer: @Composable() (@Composable() () -> Unit) -> Unit
diff --git a/ui/ui-layout/src/main/java/androidx/ui/layout/LayoutPadding.kt b/ui/ui-layout/src/main/java/androidx/ui/layout/LayoutPadding.kt
index 68af16e..bcdd42b 100644
--- a/ui/ui-layout/src/main/java/androidx/ui/layout/LayoutPadding.kt
+++ b/ui/ui-layout/src/main/java/androidx/ui/layout/LayoutPadding.kt
@@ -159,8 +159,12 @@
         val height = (placeable.height + vertical)
             .coerceIn(constraints.minHeight, constraints.maxHeight)
         return layout(width, height) {
-            // Place will automatically mirror based on the incoming layout direction.
-            placeable.place(start.toIntPx(), top.toIntPx())
+            // TODO (b/153317665) use place() instead when bug is fixed
+            if (layoutDirection == LayoutDirection.Ltr) {
+                placeable.placeAbsolute(start.toIntPx(), top.toIntPx())
+            } else {
+                placeable.placeAbsolute(width - placeable.width - start.toIntPx(), top.toIntPx())
+            }
         }
     }
 }