Propagate LocalLayoutDirection into PopupLayout (#562)

# Conflicts: # compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/DesktopPopupTest.kt
JetBrains · Jun 7, 2023 · 9c486c1 · 9c486c1
1 parent 2ad6ed5
commit 9c486c1
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 17 deletions.
diff --git a/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/DesktopPopupTest.kt b/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/DesktopPopupTest.kt
@@ -43,14 +43,15 @@ import androidx.compose.ui.input.pointer.PointerEventType
 import androidx.compose.ui.layout.Layout
 import androidx.compose.ui.platform.LocalDensity
 import androidx.compose.ui.test.ExperimentalTestApi
+import androidx.compose.ui.platform.LocalLayoutDirection
 import androidx.compose.ui.test.junit4.createComposeRule
 import androidx.compose.ui.unit.Density
 import androidx.compose.ui.unit.IntRect
+import androidx.compose.ui.unit.LayoutDirection
 import androidx.compose.ui.unit.dp
 import androidx.compose.ui.use
 import com.google.common.truth.Truth
 import com.google.common.truth.Truth.assertThat
-import org.junit.Ignore
 import org.junit.Rule
 import org.junit.Test
 
@@ -78,6 +79,28 @@ class DesktopPopupTest {
         Truth.assertThat(actualLocalValue).isEqualTo(3)
     }
 
+    // https://github.com/JetBrains/compose-multiplatform/issues/3142
+    @Test
+    fun `pass LayoutDirection to popup`() {
+        lateinit var localLayoutDirection: LayoutDirection
+
+        var layoutDirection by mutableStateOf(LayoutDirection.Rtl)
+        rule.setContent {
+            CompositionLocalProvider(LocalLayoutDirection provides layoutDirection) {
+                Popup {
+                    localLayoutDirection = LocalLayoutDirection.current
+                }
+            }
+        }
+
+        assertThat(localLayoutDirection).isEqualTo(LayoutDirection.Rtl)
+
+        // Test that changing the local propagates it into the popup
+        layoutDirection = LayoutDirection.Ltr
+        rule.waitForIdle()
+        assertThat(localLayoutDirection).isEqualTo(LayoutDirection.Ltr)
+    }
+
     @Test
     fun `onDispose inside popup`() {
         var isPopupShowing by mutableStateOf(true)

diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/ComposeScene.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/ComposeScene.skiko.kt
@@ -369,8 +369,8 @@ class ComposeScene internal constructor(
             platform,
             platform.focusManager,
             pointerPositionUpdater,
-            density,
-            IntSize(constraints.maxWidth, constraints.maxHeight).toIntRect(),
+            initDensity = density,
+            bounds = IntSize(constraints.maxWidth, constraints.maxHeight).toIntRect(),
             onPreviewKeyEvent = onPreviewKeyEvent,
             onKeyEvent = onKeyEvent,
         )

diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/SkiaBasedOwner.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/SkiaBasedOwner.skiko.kt
@@ -33,11 +33,9 @@ import androidx.compose.ui.focus.FocusDirection.Companion.Previous
 import androidx.compose.ui.geometry.Offset
 import androidx.compose.ui.graphics.Canvas
 import androidx.compose.ui.graphics.asComposeCanvas
-import androidx.compose.ui.input.InputMode
 import androidx.compose.ui.input.InputMode.Companion.Keyboard
 import androidx.compose.ui.input.InputMode.Companion.Touch
 import androidx.compose.ui.input.InputModeManager
-import androidx.compose.ui.input.InputModeManagerImpl
 import androidx.compose.ui.input.key.*
 import androidx.compose.ui.input.key.Key.Companion.Back
 import androidx.compose.ui.input.key.Key.Companion.DirectionCenter
@@ -72,6 +70,7 @@ internal class SkiaBasedOwner(
     parentFocusManager: FocusManager = EmptyFocusManager,
     private val pointerPositionUpdater: PointerPositionUpdater,
     initDensity: Density = Density(1f, 1f),
+    initLayoutDirection: LayoutDirection = platform.layoutDirection,
     bounds: IntRect = IntRect.Zero,
     val isFocusable: Boolean = true,
     val onDismissRequest: (() -> Unit)? = null,
@@ -89,7 +88,15 @@ internal class SkiaBasedOwner(
 
     override var density by mutableStateOf(initDensity)
 
-    override val layoutDirection: LayoutDirection = platform.layoutDirection
+    private var _layoutDirection by mutableStateOf(initLayoutDirection)
+
+    override var layoutDirection: LayoutDirection
+        get() = _layoutDirection
+        set(value) {
+            _layoutDirection = value
+            focusOwner.layoutDirection = value
+            root.layoutDirection = value
+        }
 
     override val sharedDrawScope = LayoutNodeDrawScope()
 
@@ -107,7 +114,7 @@ internal class SkiaBasedOwner(
     ) {
         registerOnEndApplyChangesListener(it)
     }.apply {
-        layoutDirection = platform.layoutDirection
+        layoutDirection = this@SkiaBasedOwner.layoutDirection
     }
 
     override val inputModeManager: InputModeManager
@@ -129,7 +136,7 @@ internal class SkiaBasedOwner(
     var constraints: Constraints = Constraints()
 
     override val root = LayoutNode().also {
-        it.layoutDirection = platform.layoutDirection
+        it.layoutDirection = layoutDirection
         it.measurePolicy = RootMeasurePolicy
         it.modifier = semanticsModifier
             .then(focusOwner.modifier)

diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/window/PopupLayout.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/window/PopupLayout.skiko.kt
@@ -16,20 +16,15 @@
 
 package androidx.compose.ui.window
 
-import androidx.compose.runtime.Composable
-import androidx.compose.runtime.DisposableEffect
-import androidx.compose.runtime.getValue
-import androidx.compose.runtime.mutableStateOf
-import androidx.compose.runtime.remember
-import androidx.compose.runtime.rememberCompositionContext
-import androidx.compose.runtime.setValue
+import androidx.compose.runtime.*
 import androidx.compose.ui.LocalComposeScene
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.geometry.Offset
 import androidx.compose.ui.input.key.KeyEvent
 import androidx.compose.ui.layout.Layout
 import androidx.compose.ui.layout.onGloballyPositioned
 import androidx.compose.ui.platform.LocalDensity
+import androidx.compose.ui.platform.LocalLayoutDirection
 import androidx.compose.ui.platform.SkiaBasedOwner
 import androidx.compose.ui.platform.setContent
 import androidx.compose.ui.unit.IntRect
@@ -47,6 +42,7 @@ internal fun PopupLayout(
 ) {
     val scene = LocalComposeScene.current
     val density = LocalDensity.current
+    val layoutDirection = LocalLayoutDirection.current
 
     var parentBounds by remember { mutableStateOf(IntRect.Zero) }
 
@@ -72,6 +68,7 @@ internal fun PopupLayout(
             platform = scene.platform,
             pointerPositionUpdater = scene.pointerPositionUpdater,
             initDensity = density,
+            initLayoutDirection = layoutDirection,
             isFocusable = focusable,
             onDismissRequest = onDismissRequest,
             onPreviewKeyEvent = onPreviewKeyEvent,
@@ -92,7 +89,7 @@ internal fun PopupLayout(
                             val position = popupPositionProvider.calculatePosition(
                                 anchorBounds = parentBounds,
                                 windowSize = IntSize(width, height),
-                                layoutDirection = layoutDirection,
+                                layoutDirection = this@Layout.layoutDirection,
                                 popupContentSize = IntSize(placeable.width, placeable.height)
                             )
                             owner.bounds = IntRect(
@@ -107,12 +104,15 @@ internal fun PopupLayout(
         }
         owner to composition
     }
-    owner.density = density
     DisposableEffect(Unit) {
         onDispose {
             scene.detach(owner)
             composition.dispose()
             owner.dispose()
         }
     }
+    SideEffect {
+        owner.density = density
+        owner.layoutDirection = layoutDirection
+    }
 }