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

Propagate LocalLayoutDirection into PopupLayout #562

Merged
merged 2 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,14 @@ import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.input.pointer.PointerEventType
import androidx.compose.ui.layout.Layout
import androidx.compose.ui.platform.LocalDensity
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.assertThat
import org.junit.Ignore
import org.junit.Rule
import org.junit.Test

Expand All @@ -74,6 +75,27 @@ class DesktopPopupTest {
assertThat(actualLocalValue).isEqualTo(3)
}

@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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,8 +368,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,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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()

Expand All @@ -107,7 +114,7 @@ internal class SkiaBasedOwner(
) {
registerOnEndApplyChangesListener(it)
}.apply {
layoutDirection = platform.layoutDirection
layoutDirection = this@SkiaBasedOwner.layoutDirection
}

override val inputModeManager: InputModeManager
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) }

Expand All @@ -72,6 +68,7 @@ internal fun PopupLayout(
platform = scene.platform,
pointerPositionUpdater = scene.pointerPositionUpdater,
initDensity = density,
initLayoutDirection = layoutDirection,
isFocusable = focusable,
onDismissRequest = onDismissRequest,
onPreviewKeyEvent = onPreviewKeyEvent,
Expand All @@ -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(
Expand All @@ -107,12 +104,15 @@ internal fun PopupLayout(
}
owner to composition
}
owner.density = density
DisposableEffect(Unit) {
onDispose {
scene.detach(owner)
composition.dispose()
owner.dispose()
}
}
LaunchedEffect(density, layoutDirection) {
igordmn marked this conversation as resolved.
Show resolved Hide resolved
owner.density = density
owner.layoutDirection = layoutDirection
}
}