[Material3] Scaffold contentWindowInsets uses already consumed WindowInsets [264601542]

Fixed

Bug

Status Update

No update yet.

Description

an...@steadfastinnovation.com

created issue #1

Jan 7, 2023 01:07AM

Jetpack Compose version: 1.1.0-alpha03 Jetpack Compose component used: material3 Android Studio Build: Android Studio Dolphin | 2021.3.1 Patch 1 Kotlin version: 1.7.10

The Scaffold contentWindowInsets does not take into account whether or not the provided WindowInsets are consumed or not. All the other components (app bar, drawer, etc.) use the windowInsetsPadding modifier which only applies the unconsumed WindowInsets to the respective component.

In Scaffold, the raw values of the contentWindowInsets are used, regardless of whether any part of the insets is already consumed by a parent of Scaffold, causing the insets to be applied twice. This can be worked around by applying the appropriate insets outside of Scaffold and setting contentWindowInsets = WindowInsets(0, 0, 0, 0), but it would be helpful if instead Scaffold only used the unconsumed parts of contentWindowInsets.

I'm not entirely sure if this can be solved the same way, but I noticed that the unreleased SearchBar component appears to be doing something similar.

Comments

se...@gmail.com <se...@gmail.com> #2Apr 13, 2023 12:41AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

se...@google.com <se...@google.com> Jun 1, 2023 08:22PM

Reassigned to se...@google.com.

se...@google.com <se...@google.com> Jun 6, 2023 08:29PM

Accepted by se...@google.com.

ap...@google.com <ap...@google.com> #3Jun 14, 2023 02:18PM

Project: platform/frameworks/support
Branch: androidx-main

commit b9a0af92feb056cf42411d0e2e9618b0c6dd5f37
Author: José Figueroa Santos <serniebanders@google.com>
Date: Thu Jun 08 10:14:00 2023

[m3][Scaffold] Respect consumed window insets before applying to content.

Test: Unit tests without top and bottom bar testing for windowInsets
Test: Updated testing inset logic to measure within threshold.
RelNote: "Scaffold's contentWindowInsets parameter now respects consumed window insets. Note that the behavior of content padding based on topBar and bottomBar remains unchanged when these parameters are provided."
Bug: 264601542
Change-Id: I08b739c22d2f2ac541801ae92e466ae41f265ff4

M compose/material3/material3/src/androidAndroidTest/kotlin/androidx/compose/material3/ScaffoldTest.kt
M compose/material3/material3/src/androidMain/kotlin/androidx/compose/material3/SearchBar.kt
A compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/MutableWindowInsets.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Scaffold.kt

https://android-review.googlesource.com/2617313

se...@google.com <se...@google.com> #4Jun 14, 2023 02:19PM

Marked as fixed.

An update to this behavior can be expected to land alongside 1.2.0-alpha04!

pr...@google.com <pr...@google.com> #5Jun 21, 2023 08:58PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.compose.material3:material3:1.2.0-alpha03
androidx.compose.material3:material3-android:1.2.0-alpha03

Issue 264601542