Allow read-only access to consumed insets [237019262]

Fixed

Bug

Status Update

No update yet.

Description

sa...@gmail.com

created issue #1

Jun 24, 2022 04:00PM

Jetpack Compose version: 1.2.0-rc02

Jetpack Compose component(s) used: Foundation

Android Studio Build: #AI-221.4165.146.2211.8713749

Kotlin version: 1.6.21

When accessing the window insets via asPaddingValues() it does not account for the already consumed insets further in the hierarchy. E.g.

   Box(
        modifier = Modifier
            .consumedWindowInsets(WindowInsets.navigationBars)
    ) {
        // will have non-zero insets
        val insets = WindowInsets.navigationBars.asPaddingValues()
    }

The PaddingValues could be added to a LazyColumn or similar layout that need extra padding, but only when a parent has not already consumed them. As far as I can tell it is not possible to access the amount of consumed insets, which makes it difficult to achieve this kind of layout.

My feature request is either having asPaddingValues already account for the consumed insets (just like Modifier.navigationBarPadding()) or at least provide read-only access to the amount of consumed insets.

Comments

ma...@google.com <ma...@google.com> Jun 27, 2022 10:38AM

Reassigned to mo...@google.com.

mo...@google.com <mo...@google.com> #2Jun 27, 2022 05:12PM

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

sa...@gmail.com <sa...@gmail.com> #3Jun 27, 2022 05:42PM

A problem I had was passing the insets to a LazyColumn. The LazyColumn was sometimes under the insets and so needed the insets given as its contentPadding parameter. In tablet mode the LazyColumn wasn't under the navigation bar and a parent consumed the insets, but there was no easy way to pass that info to the LazyColumn. The only alternative that I can see is to have the parent that consumed the insets to calculate what the child insets should be manually and then to pass the insets down the tree to where the LazyColumn is. This seems like reinventing the ModifierLocal that handles the consumed insets when it would be easier if that info could be accessed directly.

mo...@google.com <mo...@google.com> #4Jun 27, 2022 07:45PM

Thank you. It doesn't look like this will be a trivial solution as yet. However, one of our team members is just now adding a feature that will make this possible. Because the feature is going to be experimental API, we likely won't be able to use it in the foundation library for a few months.

va...@google.com <va...@google.com> #5Aug 17, 2022 03:57PM

One potential workaround is applying a windowInsetsPadding or windowInsets*Height to an item inside the LazyColumn, as opposed to using contentPadding (since that allows using the ModifierLocal normally. But that may not always be an option (or that has other non-desirable effects).

mo...@google.com <mo...@google.com> #6Aug 29, 2022 11:25PM

Any solution that involves composition means that layout will be delayed by 1 frame.

For the above problem, I think we need something like this:

val consumedNavigationBars by remember { MutableWindowInsets() }
Box(
        modifier = Modifier
            .consumedWindowInsets(WindowInsets.navigationBars)
            .withConsumedWindowInsets {
                consumedNavigationBars.insets = WindowInsets.navigationBars.exclude(it)
            }
    ) {
        // will have non-zero insets
        val insets = consumedNavigationBars.asPaddingValues()
    }

class MutableWindowInsets : WindowInsets {
    var insets by mutableStateOf(WindowInsets(0, 0, 0, 0))
    fun getLeft(density: Density, layoutDirection: LayoutDirection): Int = insets.getLeft(density, layoutDirection)
    fun getTop(density: Density): Int = insets.getTop(density)
    fun getRight(density: Density, layoutDirection: LayoutDirection): Int = insets.getRight(density, layoutDirection)
    fun getBottom(density: Density): Int = insets.getBottom(density)
}

ap...@google.com <ap...@google.com> #7Sep 1, 2022 06:07PM

Marked as fixed.

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

commit f6c646af9c5d6eed33998affaccdba72dcc9b825
Author: George Mount <mount@google.com>
Date: Mon Aug 29 15:55:07 2022

Allow access to consumed WindowInsets

Fixes: 237019262
Fixes: 243119659

Relnote: "Added Modifier.withConsumedWindowInsets() to
to get consumed WindowInsets for use outside windowInsetsPadding.
Added MutableWindowInsets to allow easily changing WindowInsets
without recomposition."

Test: new tests
Change-Id: I7fd28c29953ca10b4182d07996f16b66263548b1

M compose/foundation/foundation-layout/src/test/kotlin/androidx/compose/foundation/layout/WindowInsetsTest.kt
M compose/foundation/foundation-layout/samples/src/main/java/androidx/compose/foundation/layout/samples/WindowInsetsPaddingSample.kt
M compose/foundation/foundation-layout/api/public_plus_experimental_1.3.0-beta02.txt
M compose/foundation/foundation-layout/src/androidAndroidTest/kotlin/androidx/compose/foundation/layout/WindowInsetsPaddingTest.kt
M compose/foundation/foundation-layout/api/public_plus_experimental_current.txt
M compose/foundation/foundation-layout/src/commonMain/kotlin/androidx/compose/foundation/layout/WindowInsets.kt
M compose/foundation/foundation-layout/src/commonMain/kotlin/androidx/compose/foundation/layout/WindowInsetsPadding.kt

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

na...@google.com <na...@google.com> #8Sep 21, 2022 05:54PM

This bug was linked in a change in the following release(s):

androidx.compose.foundation:foundation-layout:1.3.0-beta03