Is there a particular reason why a bottom sheet de...
# compose-android
h
Is there a particular reason why a bottom sheet destination has a column as a default layout, can't we just have a content to decide for ourselves what we want to put there, maybe I don't want a Column to be my parent layout
s
Which bottom destination are we talking about here? m2, m3, standalone, integration with navigation, there are many options šŸ˜…
h
I thought bottom sheet ā€œdestinationā€ already pointed out that i’m talking about integration with navigation, sorry for confusion but yes, that’s the one https://developer.android.com/reference/kotlin/androidx/compose/material/navigation/package-summary#(androidx.navigation.NavGraphBuilder).bottomSheet(kotlin.String,kotlin.collections.List,kotlin.collections.List,kotlin.Function2)
h
Why are you concerned about this? You can treat the ColumnScope like a box and just place any layout you like as a single item. cc @jossiwolf
ā˜ļø 1
j
material-navigation provides an integration specifically for Material's bottom sheet, so we mirror the API shape. A material bottom sheet always has items arranged in vertical order, so a ColumnScope receiver makes sense here. But as Halil said, there's no need to use Column's capabilities
s
Only... it isn't exactly https://kotlinlang.slack.com/archives/CJLTWPH7S/p1712328544046829?thread_ts=1712320576.946129&cid=CJLTWPH7S In my opinion forcing content to be inside a column/row, like Card does for example, is such a clunky API decision that has propagated in places where it really shouldn't. Especially when it does not expose the fact that it is inside a ColumnScope to content. At least bottomSheet does expose the fact that you are in a
ColumnScope
, but that's still strictly worse than making no assumptions and giving the control to the callers IMO. With it being in a column we simply lose flexibility, and we gain... avoiding to do
Column { //whatever else we were doing anyway }
? So if we wanted it to be a Column, we just do
Column {}
ourselves. If we want to wrap in the available space we'd do wrapContentSize. If we want to... you get the point. Things would really be much simpler!
āž• 1
l
In my opinion forcing content to be inside a column/row, like Card does for example, is such a clunky API decision that has propagated in places where it really shouldn’t.
still strictly worse than making no assumptions and giving the control to the callers IMO.
Maybe I’m misunderstanding the concern here, please correct me if I’m wrong. A composable lambda / function in Compose can emit 0-n layout nodes, so when you have a component that accepts a
content: @Composable () -> Unit
lambda, it needs to handle all permutations of this.
Copy code
Card { 
 
}
Card { 
    Text() 
}
Card {
    Text()
    Text()
}
etc are all valid. So at some level, inside these components, we need to handle these options. Typically the options that make the most sense here are using a
Row
,
Column
, or
Box
- and these match the options that end users will expect / want to have inside the component in most cases. Adding the scope type to the public API doesnt ā€˜limit’ anything here, it’s just a) documenting the internal behavior of the component b) adding some extra possibilities for developers if the built-in layout is the desired one, as you can now directly use APIs on that scope without needing an extra layout. If you want to have a different layout, it’s expected and desired that you do:
Copy code
Card {
    Row { … }
}
This is totally valid and how we intend for these APIs to be consumed.
s
I'd argue
Copy code
Card {
  Text()
  Text()
}
Is a mistake anyway. If you do not know under which scope you are in, this is simply undefined behavior. And everyone should be actively discouraged from doing this. If you do not know that you are inside a BoxScope/ColumnScope/RowScope/WhateverScope you should simply not just emit UI composables without a care of how they will be laid out. It is the entire reason why this lint rule exists, to work around exactly this problem of things behaving differently depending on what your parent happens to do to your content without telling you. And in any case, I would much rather have
Card
just lay those out without any opinions, just in a box. Then you would see the broken UI, figure out that "ah yes, I need to tell things how they should be laid out here actually" and do the right thing for your UI instead. It may be wrapping them in a
Column
again, but if that is the case, so be it. With the current situation, you end up in situations where you do
fillMaxSize()
in one place, it does what you expect. Then you move that composable inside a
Card
and it does something different. Only because the container you are in decided to put your content in a Column without even telling you about it. How is this an acceptable scenario? Who does this benefit? Who expects this to be the case? Have you had user tests with users using the material libraries who were happy for that to be the case? I am trying to understand because I personally have never had a scenario where I liked this default. The only people who would actively know that this happens would be the ones who have read the internals of Card and remembered that "ah yes, this one puts things in a Column without actually giving me the
ColumnScope
receiver to my content lambda". And those people would super easily know that if they want a column, they can just do
Card { Column { content() } }
themselves. Again, all of this is fixed by not putting things in column, and giving the power to the callers. This in my opinion is especially hurting people getting into compose and trying to make sense of how everything works. I can tell you for sure I was super confused for many months/years before I figured out that this is what is happening.
l
Again, all of this is fixed by not putting things in column, and giving the power to the callers
To make sure I understand, what is your proposed API signature and behaviour in this case?
s
The card API signature would stay exactly the same. The implementation would simply not put the content in a column, but simply let it lay out without any wrapper. So any time I see a
content: @Composable () -> Unit
I would like it to always just be without a container which adjusts the layout. Or just a box if anything. I understand that it'd be a breaking behavior now, and you probably can never do this without a full depreciation cycle which might be hard to communicate, but still, that's what I'd optimally like to happen here.
l
When you say 'lay it out without any wrapper', I'm not sure I follow. There has to be a layout here to put modifiers such as background / default sizes etc on. So we have to have a layout.
We used to default to box for this but I think this tested very badly too, as it's almost never the correct behaviour if you emit multiple layouts inside
s
Right yes, I would mean a Box here indeed. And I do understand that it's not what you want if you emit more than one composable inside the card without the container. What I argue is that this is a good thing, and it lets people then decide what they want for their UI instead of getting some default behavior which fits some cases, perhaps.
h
Can't it just be a surface instead of a Box/Column without enforcement of how things are aligned inside it? As Stylianos menitoned above for the propagation of min constraints for the Row/Column
l
Surface is still a box, it’s just a box that propagates minimum constraints differently - but that is still an opinionated layout. I think we basically settled on ā€˜sometimes right is better than always wrong’, especially given that the recommended usages / samples don’t require changing the layout. For example, a Button with an icon looks like:
Copy code
Button(
        onClick = { /* Do something! */ },
        contentPadding = ButtonDefaults.ButtonWithIconContentPadding
    ) {
        Icon(
            Icons.Filled.Favorite,
            contentDescription = "Localized description",
            modifier = Modifier.size(ButtonDefaults.IconSize)
        )
        Spacer(Modifier.size(ButtonDefaults.IconSpacing))
        Text("Like")
    }
Since this is a common case, and in documentation / guidance buttons lay out content horizontally, having a Row makes sense as the default here - rather than forcing developers to add a row for this ā€˜default’ / recommended case
s
I see. You definitely have more input from all the devs using compose, so I will not argue against this any more I think. Me personally, we have through the years migrated away from using m3 due to problems like these, and all the container composables we make in our Design System always avoid making any such assumptions. So if you don't know which scope you're in, you're in a box. This has worked wonders for us, and I can't imagine a scenario where I would ever not build a new DS this way. But I understand that the material DS has to adhere to its own needs, and it also has to account for more people than what I need to account for (By a humongous margin). I will just say I disagree with the default being there. You can take it as input from one random dev and I think that's good enough 😊 By the way, thanks a lot for engaging in this conversation with us here, I do appreciate you sharing all this. I do understand that it's impossible to build things in a way which will make 100% of the users happy.
šŸ’Æ 1