Status Update
No update yet.
Comments
so...@google.com
jo...@google.com
Thanks for filing Alex. Not handling it at all would be a burden for developers to handle IMO, as it will be hard to guess what sides are handled by top/bottom app bars and what should be handled by the content manually. It is probably better than claiming all the padding automatically, I agree.
Would this make sense for scaffold to handle and consume the insets on the sides where we have top and bottom app bar, so that the rest is handled by the application? Still not ideal, but at least it is easier to wire up when nesting is involved.
gl...@gmail.com
I think even in that case, the Scaffold isn't directly handling the insets itself: the underlying component of the top bar or bottom bar is.
I think having a WindowInsets parameter like the app bars would be enough. Then the contract could be something along the lines of "By default, the Scaffold will inset the content to avoid the safeDrawing, either because there is an app bar taking up at least that much space directly or because the Scaffold will inset it."
And that could be an opt-out point as well: Passing an empty window insets would keep the old behavior, and let the insets pass down insets down the hierarchy unaltered.
jo...@google.com
the underlying component of the top bar or bottom bar is.
Right, but in cases when top or bottom app bars handle this insets, we need to make sure that those insets are claimed successfully so that they are not reapplied again? It becomes a burden of the scaffold to make sure this is done properly, otherwise content code will have to check and adjust based on the top/bottom app bar presence, which is not ideal.
And that could be an opt-out point as well: Passing an empty window insets would keep the old behavior
Same here, I would like to avoid the cases where you would need to check the top/bottom app bar presence in the content lambda. Not sure it is completely possible though, i will look into it more.
to...@gmail.com
I would also like to throw in the following:
With the changes, the entire screen is now reduced in size when the keyboard appears.
This was not the case before the change and I would like to deactivate it again. Is it possible to add this as an option?
jo...@google.com
I would also appreciate the ability to turn off Scaffold insets handling. We are handling insets on each screen, and this update made our app look odd (double insets all over the place). Fortunately, it's quite easy to fix it. But there is one thing that is not easily fixable.
We are using Scaffold for the base structure on each screen, but most of our screens with LazyColumn are supposed to draw behind the navigation bar. But this is currently not easily possible with the Scaffold. I know, I can remove the bottom inset from the bottom padding, but this is not a great solution.
Also, Scaffold's snackbarHost slot (or SnackbarHost) doesn't handle insets by default.
04...@mail.ru
otherwise content code will have to check and adjust based on the top/bottom app bar presence, which is not ideal.
The usage I'm thinking of shouldn't have the content care if the top/bottom app bar is there.
Let's say the Scaffold didn't interact with insets at all.
Then, the inner padding passed to the content will pass down the space information taken up by the app bars. That may or may not include the insets.
But what the content can do is do call consumedWindowInsets(innerPadding) unconditionally. If there are app bars, then the height of those will be consumed so that they are not reapplied later. If there aren't app bars, then the innerPadding will be empty, and the insets will be left available for an inner component to take into account.
I think that pass-through behavior should be available as an option for Scaffold, to also handle the cases for the reasons highlighted above by others.
04...@mail.ru
re
With the changes, the entire screen is now reduced in size when the keyboard appears.
I'm not sure I follow what's happening in your case. This change handles no ime insets and ime insets should work as before. Please file a separate bug with the repro sample if you see something strange.
04...@mail.ru
Currently Scaffold is including safeDrawing, which includes IME insets as well.
So innerPadding will contain the ime insets (assuming there is no bottom bar).
If there is a bottom bar, it will depend on whether the bottom bar is handling the ime insets or not.
jo...@google.com
re Alex
But what the content can do is do call consumedWindowInsets(innerPadding) unconditionally.
calling consumedWindowInsets(innerPadding) unconditionally would consume more than intended as it will include the top and bottom app bar heights, causing the amount of unconsumed insets to be smaller then the real insets are. This will cause ime and other insets to break potentially and show incorrect padding.
This is one of the reasons we might never promote the PaddingValues overload of consumeWindowInsets, it is just too dangerous.
I'm looking into what would be a sensible shape of the API
04...@mail.ru
Currently Scaffold is including safeDrawing, which includes IME insets as well.
Ooooh, that's true. I don't think we want to handle ime at all here in the scaffold or in other places to be honest. Is there a reason to handle it there? Probably for snackbar only..
jo...@google.com
04...@mail.ru
I don't think we want to handle ime at all here in the scaffold or in other places to be honest. Is there a reason to handle it there?
It's horrible when ime is handled there that the bottom bar moves up. It looks really weird.
ap...@google.com
It is a bug, we should only consume status and navigation bars, but not ime. It can be an opt-in behavior if desired.
gl...@gmail.com
snackbar and fab problem will be adressed here:
04...@mail.ru
Branch: androidx-main
commit 82cc43345ae181e2b1d708627e42fd0313738efa
Author: Matvei Malkov <malkov@google.com>
Date: Mon Aug 29 18:15:05 2022
Adjust material inset logic to account for ime and to allow for scaffold opt-out.
This change makes sure scaffold is handling the insets well. Allows for opt-outs and doesn't interfere with the IME paddings.
While we are at it, this change adjusts other components to never account for IME padding as well, making it an opt-in behavior.
Fixes: 243713323
Test: added new for new API + adjusted existing
Relnote: Default components insets introduced in m3 components in beta01 version are no longer account for IME insets.
Relnote: material3 Scaffold component now has a contentWindowInsets parameter, allowing to specify the amount of insets to handle for the content slot.
Change-Id: Icf11a4169c801d2670d88066984328205f48bb4f
M compose/material3/material3/src/androidAndroidTest/kotlin/androidx/compose/material3/ScaffoldTest.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/NavigationBar.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/SystemBarsDefaultInsets.kt
M compose/material3/material3/api/public_plus_experimental_current.txt
A compose/material3/material3/api/restricted_current.ignore
M compose/material3/material3/src/desktopMain/kotlin/androidx/compose/material/SystemBarsDefaultInsets.desktop.kt
M compose/material3/material3/api/restricted_current.txt
A compose/material3/material3/api/current.ignore
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/AppBar.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/NavigationDrawer.kt
M compose/material3/material3/api/current.txt
M compose/material3/material3/api/public_plus_experimental_1.0.0-beta02.txt
M compose/material3/material3/src/androidAndroidTest/kotlin/androidx/compose/material3/MaterialComponentsInsetSupportTest.kt
M compose/material3/material3/api/1.0.0-beta02.txt
M compose/material3/material3/src/androidMain/kotlin/androidx/compose/material3/SystemBarsDefaultInsets.android.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Scaffold.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/NavigationRail.kt
M compose/material3/material3/api/restricted_1.0.0-beta02.txt
M compose/integration-tests/demos/src/main/java/androidx/compose/integration/demos/DemoApp.kt
M compose/material3/material3/samples/src/main/java/androidx/compose/material3/samples/ScaffoldSamples.kt
M compose/material3/material3/integration-tests/material3-catalog/src/main/java/androidx/compose/material3/catalog/library/ui/component/Component.kt
Description
Jetpack Compose version:
Jetpack Compose component(s) used:
Android Studio Build: Build #AI-223.8836.35.2231.10671973, built on August 17, 2023
Kotlin version: 1.9.0
Code snippet to reproduce this issue:
Stack trace (if applicable):