Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,8 @@ internal fun PlatformInsets.toWindowInsets(): WindowInsets = object : WindowInse
override fun getTop(density: Density): Int = top
override fun getRight(density: Density, layoutDirection: LayoutDirection): Int = right
override fun getBottom(density: Density): Int = bottom

override fun toString(): String {
return "PlatformInsets.toWindowInsets(getLeft=$left, getTop=$top, getRight=$right, getBottom=$bottom)"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.node.ModifierNodeElement
import androidx.compose.ui.node.ObserverModifierNode
import androidx.compose.ui.node.observeReads
import androidx.compose.ui.node.traverseAncestors
import androidx.compose.ui.platform.InspectorInfo
import androidx.compose.ui.platform.PlatformWindowInsets
import androidx.compose.ui.platform.PlatformWindowInsetsProviderNode
Expand Down Expand Up @@ -155,9 +156,10 @@ private val mandatorySystemGesturesPaddingLambda: PlatformWindowInsets.() -> Win
@Stable
private fun Modifier.windowInsetsPadding(
inspectorInfo: InspectorInfo.() -> Unit,
insetsCalculation: PlatformWindowInsets.() -> WindowInsets
): Modifier =
this then PlatformWindowInsetsPaddingModifierElement(inspectorInfo, insetsCalculation)
insetsCalculation: PlatformWindowInsets.() -> WindowInsets,
): Modifier = this then
PlatformInsetsPaddingModifierElement(inspectorInfo) then
PlatformWindowInsetsPaddingModifierElement(inspectorInfo, insetsCalculation)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding both of them works, but kind of suspicious and creates more elements to allocate/iterate/etc 🤔

Just to be informed: what's about these alternatives?

  1. Delegate PlatformWindowInsetsProviderNode from InsetsPaddingModifierNode (not visa versa).
  2. Using DelegatableNode.visitAncestors(includeDelegates = true)

(1) looks more promising for me. WDYT?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem in general is that both the PlatformWindowInsetsProviderNode and InsetsPaddingModifierNode are traversable (they have different TraverseKey) and need to traverse their respective descendants to update them with ancestor properties.

As for

  1. I have thought about delegating PlatformWindowInsetsProviderNode from InsetsPaddingModifierNode but the problem is that the common InsetsPaddingModifierNode is not a DelegatingNode so we cannot delegate from it to another node. And I am not sure we want to make it a DelegatingNode in common just to use this functionality on the platform level. Also I think that the problem with the delegated node not being discovered would still remain. Or do you see some other option that I don't?
  2. DelegatableNode.visitAncestors(includeDelegates = true) is internal to commonMain so is the idea to use it in InsetsConsumingModifierNode instead of traverseAncestors ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is internal to commonMain

It's internal to the module, not source-set


private class PlatformWindowInsetsPaddingModifierElement(
private val inspectorInfo: InspectorInfo.() -> Unit,
Expand All @@ -178,9 +180,7 @@ private class PlatformWindowInsetsPaddingModifierNode(
private var insetsGetter: PlatformWindowInsets.() -> WindowInsets,
): PlatformWindowInsetsProviderNode(), ObserverModifierNode {

private val insetsPaddingNode = delegate(
InsetsPaddingModifierNode(WindowInsets())
)
private var insetsPaddingNode: PlatformInsetsPaddingModifierNode? = null

fun update(insetsGetter: (PlatformWindowInsets) -> WindowInsets) {
if (this.insetsGetter !== insetsGetter) {
Expand All @@ -194,9 +194,26 @@ private class PlatformWindowInsetsPaddingModifierNode(
override fun onAttach() {
super.onAttach()

traverseAncestors(InsetsConsumingModifierNodeKey) { node ->
if (node is PlatformInsetsPaddingModifierNode) {
insetsPaddingNode = node
false
} else true
}

onObservedReadsChanged()
}

override fun onReset() {
insetsPaddingNode = null
super.onReset()
}

override fun onDetach() {
insetsPaddingNode = null
super.onDetach()
}

override fun windowInsetsInvalidated() {
super.windowInsetsInvalidated()

Expand All @@ -205,7 +222,26 @@ private class PlatformWindowInsetsPaddingModifierNode(

override fun onObservedReadsChanged() {
observeReads {
insetsPaddingNode.update(windowInsets.insetsGetter())
insetsPaddingNode?.update(windowInsets.insetsGetter())
}
}
}
}

private class PlatformInsetsPaddingModifierElement(
private val inspectorInfo: InspectorInfo.() -> Unit,
): ModifierNodeElement<PlatformInsetsPaddingModifierNode>() {
override fun create(): PlatformInsetsPaddingModifierNode = PlatformInsetsPaddingModifierNode()
override fun update(node: PlatformInsetsPaddingModifierNode) {}
override fun hashCode(): Int = 0
override fun equals(other: Any?): Boolean {
if (this === other) return true
if (other !is PlatformInsetsPaddingModifierElement) return false
return inspectorInfo === other.inspectorInfo
}
override fun InspectorInfo.inspectableProperties() = inspectorInfo()
}

private class PlatformInsetsPaddingModifierNode: InsetsPaddingModifierNode(WindowInsets())

// TODO: https://youtrack.jetbrains.com/issue/CMP-9483 remove with a conflict after AOSP merge
internal const val InsetsConsumingModifierNodeKey = "androidx.compose.foundation.layout.ConsumedInsetsProvider"
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,29 @@

package androidx.compose.ui.layout

import androidx.compose.foundation.background
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.WindowInsets
import androidx.compose.foundation.layout.consumeWindowInsets
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.imePadding
import androidx.compose.foundation.layout.systemBars
import androidx.compose.foundation.layout.systemBarsPadding
import androidx.compose.foundation.layout.windowInsetsPadding
import androidx.compose.material.TextField
import androidx.compose.runtime.Composable
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.mutableStateOf
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.geometry.Rect
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.platform.PlatformInsets
import androidx.compose.ui.platform.PlatformWindowInsets
import androidx.compose.ui.test.ExperimentalTestApi
import androidx.compose.ui.test.InternalTestApi
import androidx.compose.ui.test.runInternalSkikoComposeUiTest
import androidx.compose.ui.unit.dp
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNotEquals
Expand Down Expand Up @@ -177,6 +185,154 @@ class WindowInsetsTest {
assertEquals(1, compositionCount)
}
}

@Test
fun testConsumeWindowInsetsWithWindowInsetsPadding() = runInternalSkikoComposeUiTest {
var outerBoxRect = Rect.Zero
var innerBoxRect = Rect.Zero

setContent {
Box(Modifier
.fillMaxSize()
.consumeWindowInsets(WindowInsets(top = 100.dp))
.background(Color.Red)
.onGloballyPositioned { outerBoxRect = it.boundsInRoot() }
) {
Box(Modifier
.fillMaxSize()
.windowInsetsPadding(WindowInsets(top = 101.dp))
.background(Color.Blue)
.onGloballyPositioned { innerBoxRect = it.boundsInRoot() }
)
}
}

assertEquals(Rect(outerBoxRect.left, 1f, outerBoxRect.right, outerBoxRect.bottom), innerBoxRect)
}

@Test
fun testConsumeSystemWindowInsetsWithSystemPadding() = runInternalSkikoComposeUiTest(
windowInsets = TestWindowInsets(systemBarsInsets = mutableStateOf(PlatformInsets(top = 100)))
) {
var outerBoxRect = Rect.Zero
var innerBoxRect = Rect.Zero

setContent {
Box(Modifier
.fillMaxSize()
.consumeWindowInsets(WindowInsets.systemBars)
.background(Color.Red)
.onGloballyPositioned { outerBoxRect = it.boundsInRoot() }
) {
Box(Modifier
.fillMaxSize()
.systemBarsPadding()
.background(Color.Blue)
.onGloballyPositioned { innerBoxRect = it.boundsInRoot() }
)
}
}

assertEquals(outerBoxRect, innerBoxRect)
}

@Test
fun testConsumeWindowInsetsWithSystemPadding() = runInternalSkikoComposeUiTest(
windowInsets = TestWindowInsets(systemBarsInsets = mutableStateOf(PlatformInsets(top = 101)))
) {
var outerBoxRect = Rect.Zero
var innerBoxRect = Rect.Zero

setContent {
Box(Modifier
.fillMaxSize()
.consumeWindowInsets(WindowInsets(top = 100.dp))
.background(Color.Red)
.onGloballyPositioned { outerBoxRect = it.boundsInRoot() }
) {
Box(Modifier
.fillMaxSize()
.systemBarsPadding()
.background(Color.Blue)
.onGloballyPositioned { innerBoxRect = it.boundsInRoot() }
)
}
}

assertEquals(Rect(outerBoxRect.left, 1f, outerBoxRect.right, outerBoxRect.bottom), innerBoxRect)
}

@Test
fun testConsumeWindowInsetsPaddingOnSystemInsetsChange() {
val systemBarsInsets = mutableStateOf(PlatformInsets.Zero)

runInternalSkikoComposeUiTest(
windowInsets = TestWindowInsets(systemBarsInsets = systemBarsInsets)
) {
var outerBoxRect = Rect.Zero
var innerBoxRect = Rect.Zero

setContent {
Box(Modifier
.fillMaxSize()
.consumeWindowInsets(WindowInsets(top = 100.dp))
.background(Color.Red)
.onGloballyPositioned { outerBoxRect = it.boundsInRoot() }
) {
Box(Modifier
.fillMaxSize()
.systemBarsPadding()
.background(Color.Blue)
.onGloballyPositioned { innerBoxRect = it.boundsInRoot() }
)
}
}

assertEquals(outerBoxRect, innerBoxRect)

systemBarsInsets.value = PlatformInsets(top = 101)
waitForIdle()

assertEquals(Rect(outerBoxRect.left, 1f, outerBoxRect.right, outerBoxRect.bottom), innerBoxRect)
}
}

@Test
fun testConsumeSystemWindowInsetsPaddingOnSystemInsetsChange() {
val systemBarsInsets = mutableStateOf(PlatformInsets.Zero)

runInternalSkikoComposeUiTest(
windowInsets = TestWindowInsets(systemBarsInsets = systemBarsInsets)
) {
var outerBoxRect = Rect.Zero
var innerBoxRect = Rect.Zero
val maxBottomInset = 100

setContent {
Box(Modifier
.fillMaxSize()
.consumeWindowInsets(WindowInsets.systemBars)
.background(Color.Red)
.onGloballyPositioned { outerBoxRect = it.boundsInRoot() }
) {
Box(Modifier
.fillMaxSize()
.systemBarsPadding()
.background(Color.Blue)
.onGloballyPositioned { innerBoxRect = it.boundsInRoot() }
)
}
}

assertEquals(outerBoxRect, innerBoxRect)

for (i in 1..maxBottomInset) {
systemBarsInsets.value = PlatformInsets(bottom = i)
waitForIdle()
assertEquals(outerBoxRect, innerBoxRect)
}
}
}
}

@Composable
Expand All @@ -197,12 +353,20 @@ private fun TestContent(
}

private fun TestWindowInsets(
imeInsets: MutableState<PlatformInsets>
imeInsets: MutableState<PlatformInsets> = mutableStateOf(PlatformInsets.Zero),
systemBarsInsets: MutableState<PlatformInsets> = mutableStateOf(PlatformInsets.Zero)
): PlatformWindowInsets = object : PlatformWindowInsets {
override val ime: PlatformInsets get() = PlatformInsets(
getBottom = { imeInsets.value.bottom },
getTop = { imeInsets.value.top },
getLeft = { imeInsets.value.left },
getRight = { imeInsets.value.right }
)

override val systemBars: PlatformInsets get() = PlatformInsets(
getBottom = { systemBarsInsets.value.bottom },
getTop = { systemBarsInsets.value.top },
getLeft = { systemBarsInsets.value.left },
getRight = { systemBarsInsets.value.right }
)
}