Hristijan
11/06/2024, 8:28 AMStylianos Gakis
11/06/2024, 8:41 AMHristijan
11/06/2024, 8:45 AMHalil Ozercan
11/06/2024, 10:52 AMjossiwolf
11/06/2024, 11:00 AMStylianos Gakis
11/06/2024, 11:34 AMColumnScope
, 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!Louis Pullen-Freilich [G]
11/06/2024, 12:57 PMIn 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.
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:
Card {
Row { ⦠}
}
This is totally valid and how we intend for these APIs to be consumed.Stylianos Gakis
11/06/2024, 1:42 PMCard {
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.Louis Pullen-Freilich [G]
11/06/2024, 2:12 PMAgain, all of this is fixed by not putting things in column, and giving the power to the callersTo make sure I understand, what is your proposed API signature and behaviour in this case?
Stylianos Gakis
11/06/2024, 2:18 PMcontent: @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.Louis Pullen-Freilich [G]
11/06/2024, 2:21 PMLouis Pullen-Freilich [G]
11/06/2024, 2:21 PMStylianos Gakis
11/06/2024, 2:26 PMHristijan
11/06/2024, 3:07 PMLouis Pullen-Freilich [G]
11/06/2024, 5:15 PMButton(
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 caseStylianos Gakis
11/06/2024, 6:47 PM