[Bug]: Material3 beta01 Scaffold behaves differently regarding status bar paddings

StylianosGakis commented 2 years ago

Is there an existing issue for this?

[X] I have searched the existing issues

Is there a StackOverflow question about this issue?

[X] I have searched StackOverflow

What happened?

This bug hasn't really happened yet, but it will when the Material3 dependency is bumped to at least beta01. I have encountered this myself when I was trying to achieve a similar result to how NiA sets up the Scaffold with the NavRail and the BottomBar. The new Scaffold implicitly adds the necessary padding to the paddingValues provided to its lambda to pad the status bar as well. This means that NiA will then look like this instead:

compile SDK 32 and m3 alpha13	compile SDK 33 and m3 beta01

I suppose this is so that the Scaffold sort of does everything for you that you'd need for a very cookie-cutter screen, which this one isn't necessarily one like that. I see a couple of approaches to fixing this. 1) Take this opportunity and move away from a Scaffold which uses subcomposition anyway and is not necessary in this use case since we're not using snackbars and stuff like that. Inspired from this discussion in slack as well: Instead of

Scaffold(
  BottomBar()
) {
  Row {
    NavRail()
    Content()
  }
}

Do

Column {
  Row {
    NavRail()
    Content()
  }
  BottomBar
}

2) Take the paddings which come from the scaffold and are passed here and manually ignore the top part of it? Kinda hacky maybe, I am not sure what the general advice will be with this new behavior of Scaffold.

Or what do you think?

Relevant logcat output

No response

Code of Conduct

[X] I agree to follow this project's Code of Conduct

StylianosGakis commented 2 years ago

Looking at the screenshot again, I see that the inner Scaffold inside the ForYou screen here also makes it so that at the bottom of that screen there's an extra padding which cuts off the button a bit as you can see. Kind of a wild change of how Scaffolds work. To me this really makes me feel like the lib wants you to use the Scaffold in the very specific case where it's the top level and you got this cookie cutter screen, or you should go with Columns/Rows instead. Now there's a lot of automatic adding of insets inside the Scaffold code which seems to not have been there before. How would you solve that inner scaffold padding problem as well as the original one I was referring to? If one still wants to use Scaffold for some reason, I suspect they'd need to be smart about which insets you have to inform the Scaffold that you've handled already with consumedWindowInsets? I've tried doing something like that too but the library itself was adding this padding using val insets = WindowInsets.safeDrawingForVisualComponents.asPaddingValues(this@SubcomposeLayout) which doesn't respect if they've been consumed before, so that doesn't seem to be solution either.

I must be missing something with all these changes I feel like. Some approach that shouldn't bring all these issues 🤔

alexvanyo commented 2 years ago

See https://github.com/android/nowinandroid/pull/242 for the in-progress work addressing this for beta01, and that sparked some internal changes as well linked in that PR.

The next version will make this new behavior of Scaffold customizable.

StylianosGakis commented 2 years ago

Yes, I'm so glad this has been addressed already. Thanks for bringing this discussion into my attention. It seems like all of my concerns were addressed there already.

Also really interesting that the lib apis are quickly changed after dogfooding them a bit in this repo. Yet another benefit of NiA existing 💪

StylianosGakis commented 2 years ago

https://developer.android.com/jetpack/androidx/releases/compose-material3#1.0.0-beta02 🥳

android / nowinandroid