Skip to content

Commit 283d269

Browse files
svastvensvastven
authored
Fix consuming window insets in system window insets padding (#2665)
This PR fixes an issue where window insets (like `statusBarsPadding()`) did not correctly respect or propagate consumed window insets. The core problem was that the `InsetsPaddingModifierNode` logic was "hidden" from the layout traversal system because it was implemented via delegation. The `Modifier.windowInsetsPadding` now applies two separate modifiers which create nodes of two types: - `PlatformInsetsPaddingModifierNode` is an `InsetsPaddingModifierNode` that handles actual layout padding and correct resolve of window insets consumed by ancestors. - `PlatformWindowInsetsPaddingModifierNode` is responsible for retrieving platform-specific `WindowInsets` values. On attachment, it traverses up the modifier chain to find the ancestor `PlatformInsetsPaddingModifierNode` and establishes a link. It provides the insets back up to this ancestor node. Fixes [CMP-9390](https://youtrack.jetbrains.com/issue/CMP-9390) iOS seems to ignore consumeWindowInsets ## Testing Adds skiko tests to `WindowInsetsTest` This should be tested by QA ## Release Notes ### Fixes - Multiple Platforms - Fix window insets consumption in system window insets padding modifiers.
1 parent a4732ff commit 283d269

File tree

3 files changed

+213
-9
lines changed

3 files changed

+213
-9
lines changed

‎compose/foundation/foundation-layout/src/skikoMain/kotlin/androidx/compose/foundation/layout/WindowInsets.skiko.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,8 @@ internal fun PlatformInsets.toWindowInsets(): WindowInsets = object : WindowInse
9494
override fun getTop(density: Density): Int = top
9595
override fun getRight(density: Density, layoutDirection: LayoutDirection): Int = right
9696
override fun getBottom(density: Density): Int = bottom
97+
98+
override fun toString(): String {
99+
return "PlatformInsets.toWindowInsets(getLeft=$left, getTop=$top, getRight=$right, getBottom=$bottom)"
100+
}
97101
}

‎compose/foundation/foundation-layout/src/skikoMain/kotlin/androidx/compose/foundation/layout/WindowInsetsPadding.skiko.kt‎

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import androidx.compose.ui.Modifier
2424
import androidx.compose.ui.node.ModifierNodeElement
2525
import androidx.compose.ui.node.ObserverModifierNode
2626
import androidx.compose.ui.node.observeReads
27+
import androidx.compose.ui.node.traverseAncestors
2728
import androidx.compose.ui.platform.InspectorInfo
2829
import androidx.compose.ui.platform.PlatformWindowInsets
2930
import androidx.compose.ui.platform.PlatformWindowInsetsProviderNode
@@ -155,9 +156,10 @@ private val mandatorySystemGesturesPaddingLambda: PlatformWindowInsets.() -> Win
155156
@Stable
156157
private fun Modifier.windowInsetsPadding(
157158
inspectorInfo: InspectorInfo.() -> Unit,
158-
insetsCalculation: PlatformWindowInsets.() -> WindowInsets
159-
): Modifier =
160-
this then PlatformWindowInsetsPaddingModifierElement(inspectorInfo, insetsCalculation)
159+
insetsCalculation: PlatformWindowInsets.() -> WindowInsets,
160+
): Modifier = this then
161+
PlatformInsetsPaddingModifierElement(inspectorInfo) then
162+
PlatformWindowInsetsPaddingModifierElement(inspectorInfo, insetsCalculation)
161163

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

181-
private val insetsPaddingNode = delegate(
182-
InsetsPaddingModifierNode(WindowInsets())
183-
)
183+
private var insetsPaddingNode: PlatformInsetsPaddingModifierNode? = null
184184

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

197+
traverseAncestors(InsetsConsumingModifierNodeKey) { node ->
198+
if (node is PlatformInsetsPaddingModifierNode) {
199+
insetsPaddingNode = node
200+
false
201+
} else true
202+
}
203+
197204
onObservedReadsChanged()
198205
}
199206

207+
override fun onReset() {
208+
insetsPaddingNode = null
209+
super.onReset()
210+
}
211+
212+
override fun onDetach() {
213+
insetsPaddingNode = null
214+
super.onDetach()
215+
}
216+
200217
override fun windowInsetsInvalidated() {
201218
super.windowInsetsInvalidated()
202219

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

206223
override fun onObservedReadsChanged() {
207224
observeReads {
208-
insetsPaddingNode.update(windowInsets.insetsGetter())
225+
insetsPaddingNode?.update(windowInsets.insetsGetter())
209226
}
210227
}
211-
}
228+
}
229+
230+
private class PlatformInsetsPaddingModifierElement(
231+
private val inspectorInfo: InspectorInfo.() -> Unit,
232+
): ModifierNodeElement<PlatformInsetsPaddingModifierNode>() {
233+
override fun create(): PlatformInsetsPaddingModifierNode = PlatformInsetsPaddingModifierNode()
234+
override fun update(node: PlatformInsetsPaddingModifierNode) {}
235+
override fun hashCode(): Int = 0
236+
override fun equals(other: Any?): Boolean {
237+
if (this === other) return true
238+
if (other !is PlatformInsetsPaddingModifierElement) return false
239+
return inspectorInfo === other.inspectorInfo
240+
}
241+
override fun InspectorInfo.inspectableProperties() = inspectorInfo()
242+
}
243+
244+
private class PlatformInsetsPaddingModifierNode: InsetsPaddingModifierNode(WindowInsets())
245+
246+
// TODO: https://youtrack.jetbrains.com/issue/CMP-9483 remove with a conflict after AOSP merge
247+
internal const val InsetsConsumingModifierNodeKey = "androidx.compose.foundation.layout.ConsumedInsetsProvider"

‎compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/layout/WindowInsetsTest.kt‎

Lines changed: 165 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,29 @@
1616

1717
package androidx.compose.ui.layout
1818

19+
import androidx.compose.foundation.background
1920
import androidx.compose.foundation.layout.Box
21+
import androidx.compose.foundation.layout.WindowInsets
22+
import androidx.compose.foundation.layout.consumeWindowInsets
2023
import androidx.compose.foundation.layout.fillMaxSize
2124
import androidx.compose.foundation.layout.imePadding
25+
import androidx.compose.foundation.layout.systemBars
26+
import androidx.compose.foundation.layout.systemBarsPadding
27+
import androidx.compose.foundation.layout.windowInsetsPadding
2228
import androidx.compose.material.TextField
2329
import androidx.compose.runtime.Composable
2430
import androidx.compose.runtime.MutableState
2531
import androidx.compose.runtime.mutableStateOf
2632
import androidx.compose.ui.Alignment
2733
import androidx.compose.ui.Modifier
2834
import androidx.compose.ui.geometry.Rect
35+
import androidx.compose.ui.graphics.Color
2936
import androidx.compose.ui.platform.PlatformInsets
3037
import androidx.compose.ui.platform.PlatformWindowInsets
3138
import androidx.compose.ui.test.ExperimentalTestApi
3239
import androidx.compose.ui.test.InternalTestApi
3340
import androidx.compose.ui.test.runInternalSkikoComposeUiTest
41+
import androidx.compose.ui.unit.dp
3442
import kotlin.test.Test
3543
import kotlin.test.assertEquals
3644
import kotlin.test.assertNotEquals
@@ -177,6 +185,154 @@ class WindowInsetsTest {
177185
assertEquals(1, compositionCount)
178186
}
179187
}
188+
189+
@Test
190+
fun testConsumeWindowInsetsWithWindowInsetsPadding() = runInternalSkikoComposeUiTest {
191+
var outerBoxRect = Rect.Zero
192+
var innerBoxRect = Rect.Zero
193+
194+
setContent {
195+
Box(Modifier
196+
.fillMaxSize()
197+
.consumeWindowInsets(WindowInsets(top = 100.dp))
198+
.background(Color.Red)
199+
.onGloballyPositioned { outerBoxRect = it.boundsInRoot() }
200+
) {
201+
Box(Modifier
202+
.fillMaxSize()
203+
.windowInsetsPadding(WindowInsets(top = 101.dp))
204+
.background(Color.Blue)
205+
.onGloballyPositioned { innerBoxRect = it.boundsInRoot() }
206+
)
207+
}
208+
}
209+
210+
assertEquals(Rect(outerBoxRect.left, 1f, outerBoxRect.right, outerBoxRect.bottom), innerBoxRect)
211+
}
212+
213+
@Test
214+
fun testConsumeSystemWindowInsetsWithSystemPadding() = runInternalSkikoComposeUiTest(
215+
windowInsets = TestWindowInsets(systemBarsInsets = mutableStateOf(PlatformInsets(top = 100)))
216+
) {
217+
var outerBoxRect = Rect.Zero
218+
var innerBoxRect = Rect.Zero
219+
220+
setContent {
221+
Box(Modifier
222+
.fillMaxSize()
223+
.consumeWindowInsets(WindowInsets.systemBars)
224+
.background(Color.Red)
225+
.onGloballyPositioned { outerBoxRect = it.boundsInRoot() }
226+
) {
227+
Box(Modifier
228+
.fillMaxSize()
229+
.systemBarsPadding()
230+
.background(Color.Blue)
231+
.onGloballyPositioned { innerBoxRect = it.boundsInRoot() }
232+
)
233+
}
234+
}
235+
236+
assertEquals(outerBoxRect, innerBoxRect)
237+
}
238+
239+
@Test
240+
fun testConsumeWindowInsetsWithSystemPadding() = runInternalSkikoComposeUiTest(
241+
windowInsets = TestWindowInsets(systemBarsInsets = mutableStateOf(PlatformInsets(top = 101)))
242+
) {
243+
var outerBoxRect = Rect.Zero
244+
var innerBoxRect = Rect.Zero
245+
246+
setContent {
247+
Box(Modifier
248+
.fillMaxSize()
249+
.consumeWindowInsets(WindowInsets(top = 100.dp))
250+
.background(Color.Red)
251+
.onGloballyPositioned { outerBoxRect = it.boundsInRoot() }
252+
) {
253+
Box(Modifier
254+
.fillMaxSize()
255+
.systemBarsPadding()
256+
.background(Color.Blue)
257+
.onGloballyPositioned { innerBoxRect = it.boundsInRoot() }
258+
)
259+
}
260+
}
261+
262+
assertEquals(Rect(outerBoxRect.left, 1f, outerBoxRect.right, outerBoxRect.bottom), innerBoxRect)
263+
}
264+
265+
@Test
266+
fun testConsumeWindowInsetsPaddingOnSystemInsetsChange() {
267+
val systemBarsInsets = mutableStateOf(PlatformInsets.Zero)
268+
269+
runInternalSkikoComposeUiTest(
270+
windowInsets = TestWindowInsets(systemBarsInsets = systemBarsInsets)
271+
) {
272+
var outerBoxRect = Rect.Zero
273+
var innerBoxRect = Rect.Zero
274+
275+
setContent {
276+
Box(Modifier
277+
.fillMaxSize()
278+
.consumeWindowInsets(WindowInsets(top = 100.dp))
279+
.background(Color.Red)
280+
.onGloballyPositioned { outerBoxRect = it.boundsInRoot() }
281+
) {
282+
Box(Modifier
283+
.fillMaxSize()
284+
.systemBarsPadding()
285+
.background(Color.Blue)
286+
.onGloballyPositioned { innerBoxRect = it.boundsInRoot() }
287+
)
288+
}
289+
}
290+
291+
assertEquals(outerBoxRect, innerBoxRect)
292+
293+
systemBarsInsets.value = PlatformInsets(top = 101)
294+
waitForIdle()
295+
296+
assertEquals(Rect(outerBoxRect.left, 1f, outerBoxRect.right, outerBoxRect.bottom), innerBoxRect)
297+
}
298+
}
299+
300+
@Test
301+
fun testConsumeSystemWindowInsetsPaddingOnSystemInsetsChange() {
302+
val systemBarsInsets = mutableStateOf(PlatformInsets.Zero)
303+
304+
runInternalSkikoComposeUiTest(
305+
windowInsets = TestWindowInsets(systemBarsInsets = systemBarsInsets)
306+
) {
307+
var outerBoxRect = Rect.Zero
308+
var innerBoxRect = Rect.Zero
309+
val maxBottomInset = 100
310+
311+
setContent {
312+
Box(Modifier
313+
.fillMaxSize()
314+
.consumeWindowInsets(WindowInsets.systemBars)
315+
.background(Color.Red)
316+
.onGloballyPositioned { outerBoxRect = it.boundsInRoot() }
317+
) {
318+
Box(Modifier
319+
.fillMaxSize()
320+
.systemBarsPadding()
321+
.background(Color.Blue)
322+
.onGloballyPositioned { innerBoxRect = it.boundsInRoot() }
323+
)
324+
}
325+
}
326+
327+
assertEquals(outerBoxRect, innerBoxRect)
328+
329+
for (i in 1..maxBottomInset) {
330+
systemBarsInsets.value = PlatformInsets(bottom = i)
331+
waitForIdle()
332+
assertEquals(outerBoxRect, innerBoxRect)
333+
}
334+
}
335+
}
180336
}
181337

182338
@Composable
@@ -197,12 +353,20 @@ private fun TestContent(
197353
}
198354

199355
private fun TestWindowInsets(
200-
imeInsets: MutableState<PlatformInsets>
356+
imeInsets: MutableState<PlatformInsets> = mutableStateOf(PlatformInsets.Zero),
357+
systemBarsInsets: MutableState<PlatformInsets> = mutableStateOf(PlatformInsets.Zero)
201358
): PlatformWindowInsets = object : PlatformWindowInsets {
202359
override val ime: PlatformInsets get() = PlatformInsets(
203360
getBottom = { imeInsets.value.bottom },
204361
getTop = { imeInsets.value.top },
205362
getLeft = { imeInsets.value.left },
206363
getRight = { imeInsets.value.right }
207364
)
365+
366+
override val systemBars: PlatformInsets get() = PlatformInsets(
367+
getBottom = { systemBarsInsets.value.bottom },
368+
getTop = { systemBarsInsets.value.top },
369+
getLeft = { systemBarsInsets.value.left },
370+
getRight = { systemBarsInsets.value.right }
371+
)
208372
}

0 commit comments

Comments
 (0)