Im in a ridiculous situation with nested Scaffolds...

# compose

[Zoltan Demant]

Im in a ridiculous situation with nested Scaffolds, someone please help? ⚠️

The gist of it is that Im using one at the root of a feature just to properly inset a Toolbar, then delegating content to screen A/B.

Screens A & B both have their own Scaffold setup so that placement of Fab, NavigationBar and stuff like that is correctly inset.

Removing the nesting improves performance by a lot. Do I just handle insets manually to eliminate the Scaffold? I.e. at the root do this instead of letting Scaffold (basically) do it?

Column {
    Toolbar()

    Box(Modifier.consumeWindowInsets(WindowInsets.statusBars)) {
        Content()
    }
}

[Zoltan Demant]

Another instance where Im effectively doing the same thing:

Column {
    Content(Modifier.consumeWindowInsets(WindowInsets.navigationBars)
    

    NavBar()
}

Instead of having nested Scaffolds.

[Zoltan Demant]

Do I just do this everywhere, is that the right way?

[Stylianos Gakis]

Are you using scaffold just for top app bar/bottom nav bar + insets? If yes just stop using scaffold since you'll be avoiding sub composition completely.
If you're also using it for fab placement or snackbar placement you will probably just have to add more logic around placing those two

[Stylianos Gakis]

Here's our scaffold https://github.com/HedvigInsurance/android/blob/821cf9607883f5a2f29290914c9922689b[…]otlin/com/hedvig/android/design/system/hedvig/HedvigScaffold.kt since we need it just for top app bar. but you can take it and adjust it to your needs I think. The most important thing to get right is the insets and perhaps manually adding a consumeWindowInsets modifier where needed.

[Zoltan Demant]

I was just looking at that when I saw your comment 😁 Your project has grown immensely since I first saw it!
I think this is what I'm going to do too. If the slots are nullable, I think I could even do all the proper consuming of insets directly in the scaffold.
I need most of the slots: that's probably just a column with toolbar, box with content + fab (and padding passed in), footer. Even supporting snackbar, seems rather simple this way. Any other gotchas you can think of?

[Stylianos Gakis]

Might wanna star this too https://issuetracker.google.com/issues/295960070 so that perhaps they just remove subcomposition from there so you don't even have to do all this yourself.
I think I saw some experiments around this, but I really can't find it now. But the insets are probably gonna be the biggest gotcha.

[Zoltan Demant]

Done! Yeah if I recall correctly there was some boolean deciding whether to use subcomposition, but maybe that was in M2?
Ive been trying to figure out why the performance was so bad on Screen B, but just removing one layer of scaffold nesting solved like 90% of it. Is (nested) subcomposition that bad? Itll be interesting to see how it looks when both layers are removed, wroking on it now until the baby wakes up 😃

[Stylianos Gakis]

Yeah I think subcomposition is a bigger hit than you'd imagine, especially two stacked in each other. I haven't measured it myself because we simply got rid of it immediately so we never had to deal with it tbh 😅

[Zoltan Demant]

I can remember the very early days of compose when there were basically no charting libraries, I ended up creating one using SubcomposeLayout, where each of the bars used SubcomposeLayout too, so like 100 different subcompositions (performance was ok tbh, but still feels insane with todays knowledge).

[Zoltan Demant]

Poor mans scaffold:

Column(modifier) {
    toolbar?.invoke()

    Box(Modifier.weight(1f)) {
        content(None)

        if (fab != null) {
            Box(Modifier.align(BottomEnd).padding(KeylineSmall)) {
                fab()
            }
        }
    }

    footer?.invoke()
}

[Alex Vanyo]

If you’re in a good position to benchmark a difference, https://android-review.googlesource.com/c/platform/frameworks/support/+/2690433 is the experiment of removing subcomposition from the Scaffold in material3 while keeping the same API shape - the only difference in behavior with the non-subcomposition version is that the PaddingValues passed to the content won’t be correct if you calculate the values of them directly in composition (but you probably should avoid doing that anyway)

I think it should be mostly copy-paste-able outside of androidx?

[Zoltan Demant]

Absolutely amazing, thank you Alex!

I dont have any benchmarks setup unfortunately, but I take it you mean using androidx.benchmark:benchmark? Id be curious to know the numbers, because just eyeballing I can already tell a few differences:
• Even with just one scaffold, animations between screens are a bit smoother; it certainly feels like the complexity of a screen was multiplied like O(N) before, and is just O(1) now. I dont think anyone would have complained about the earlier case, but Im sure people will feel the difference, which is amazing.
• With nested scaffolds, the difference is ridiculous. Jumping through ALL hoops I knew of compose earlier got it to like 95% smooth (contrary to like 30% before); now its 100% without me even thinking about recompositions, stability, and a trillion of the other things.
Any advice around the consumption of windowInsets for nested scaffolds? Now that it "doesnt matter" that I nest scaffolds, Id love to continue doing it (I love that it positions everything correctly, minus the inset issues ofc). I certainly dont want to overcomplicate things more than they already are, so heres the most minimal approach I could devise: Make composables like top/bottom nullable and depending on their existence, consume top/bottom, then figure out an elegant way to skip content padding in the outer scaffold.

[Zoltan Demant]

Did not have much chance to work on this today, but just tried this and surprisingly enough: it seems to work! 😃
The returned insets is what I pass into ScaffoldLayout, and each consumeInsets boolean is just a check like topBar != null (they are not nullable in the androidx code).
Maybe Ill run into some edge case, but with this I can have one scaffold with just a toolbar+content, a nested one with content+fab, and sometimes switch to another with content+bottomBar, and all of the insets are correct!

@Composable
private fun rememberConsumedInsets(
    insets: WindowInsets,
    consumeTopInsets: Boolean,
    consumeBottomInsets: Boolean,
    consumeHorizontalInsets: Boolean,
    density: Density = LocalDensity.current,
    ld: LayoutDirection = LocalLayoutDirection.current,
) = remember(insets, consumeTopInsets, consumeBottomInsets, consumeHorizontalInsets, density, ld) {
    WindowInsets(
        left = if (consumeHorizontalInsets) insets.getLeft(density, ld) else 0,
        right = if (consumeHorizontalInsets) insets.getRight(density, ld) else 0,
        top = if (consumeTopInsets) insets.getTop(density) else 0,
        bottom = if (consumeBottomInsets) insets.getBottom(density) else 0,
    )
}

[Alex Vanyo]

Yeah exact numbers would be great in release mode, to make the "feeling" more objective

How are you using rememberConsumedInsets? Modifier.consumeWindowInsets(innerPadding) , where the innerPadding is the PaddingValues that come from the Scaffold should do what you need to manage consumption correctly (and that should still work even with the subcomposition-free Scaffold)

[Zoltan Demant]

Agreed, I don't have very much time, but I will try to get some benchmark going when I'm creating screenshot tests. Which is soon! Will you and the team have use of the numbers? For the nested situation, I think what you wrote and a clever way to know if the scaffold is used as a wrapper will cover all edge cases! For example, by default I think the behavior should be aligned with the original code, but with wrapping the root scaffold should only consume insets that are relevant to it (e.g. if topBar is used). Will try this on Monday.

[Zoltan Demant]

I thought about this a lot this past weekend. The simplest solution was to just add wrapper: Boolean to my Scaffold, which leads to it consuming insets like !wrapper || topBar != null. So root scaffolds consume all insets, and if there is nesting then each scaffold consumes what it uses only (and inner-most scaffold consumes everything thats left).

If anyone knows a way I could automate that (remove wrapper: Boolean) then Id love to know. I dont think its the most intuitive way to do it, because the different screens are often not direct children of each other, so specifying that the root scaffold is a wrapper is knowing too much about what the other screens are doing. So basically what I want is a compositionLocal that flows from inside -> out.

[Stylianos Gakis]

Using the modifiers that add inset padding already use a modifierlocal internally to keep track of which insets are already consumed by a parent or not. So optimally in any of your callers, you should always just use the right modifier, and it will automatically add or not add the space depending on if the insets are consumed already or not.
That's why you see the usage of the .consumeWindowInsets there, since it propagates that information to the children, if you happen to consume the insets somewhere which is not the parent of your layouts, hence the modifier local does not automatically propagate there

[Zoltan Demant]

I think I get what you mean @Stylianos Gakis; DerivedHeightModifier.unconsumedInsets is whats used in Modifier.windowInsetsTopHeight etc. So if parent scaffold consumes top, children wont see the top, right? But in my case, I want the reverse.. kind of.. or well, wrapper scaffold should only consume insets if it has content for it; otherwise the children will consume it (regardless of them having content there or not).

[Zoltan Demant]

Otherwise the wrapper scaffold will eat up all the insets, leading to minor issues like LazyColumn not scrolling underneath the system navigation, or major issues like in-app NavigationBar not being extended to under the system nav

[Stylianos Gakis]

Yes, so you can add the .consumeWindowInsets modifier to your content to consume the top insets only if the top app bar is present for example. And the same for bottom bar or whatever else you do

[Alex Vanyo]

It sounds like you want to pass WindowInsets(0, 0, 0, 0) to contentWindowInsets for each of your `Scaffold`s? To avoid any of the contentPadding accumulating any values from windowInsets directly, unless there is a bar.

[Zoltan Demant]

@Stylianos Gakis Yes! That's exactly what I'm doing 👌🏼

@Alex Vanyo Have you tried doing that? For me it wasn't working, maybe I made a mistake along the way but just saw top insets being applied at the root scaffold, then once again by my topBar even though I'd consume the insets at the root.