I’m going through some design review and fixing up...

# compose

[Chris Fillmore]

I’m going through some design review and fixing up inconsistent design. Honestly my UI code is a bit of a mess in this regard; padding, for example, is applied inconsistently, all over the place, as I’ve been just patching things together. So I’d like to build some reusable components to get it into a more consistent state. Can I ask for feedback on my approach?

[Chris Fillmore]

I have many bottom sheet screens of this form. A column with a header, followed by clickable rows which include an icon, description, and optionally a control, like a switch:

[header with gray background and black text]
[icon][descriptive text][optional control]
[icon][descriptive text][optional control]
...
etc

[Chris Fillmore]

I am considering an approach like the below, where I have a reusable composable that supplies a “scope” which can be used to add item rows. It’s incomplete but hopefully you get the basic idea:

private val bottomSheetPadding = PaddingValues(horizontal = 24.dp, vertical = 16.dp)
private val bottomSheetItemPadding = PaddingValues(horizontal = 24.dp, vertical = 12.dp)

@Composable
fun BottomSheetScreen(
  headerContent: @Composable (PaddingValues) -> Unit,
  mainContent: @Composable BottomSheetContentScope.(PaddingValues)  -> Unit,
) {
  val scrollState = rememberScrollState()
  Column(
    modifier = Modifier.verticalScroll(scrollState),
  ) {
    Surface(
      color = PaleGrey,
      contentColor = Color.Black,
    ) {
      headerContent(bottomSheetPadding)
    }

    Surface(
      color = Color.White,
    ) {
      BottomSheetContentScopeImpl.mainContent(bottomSheetPadding)
    }
  }
}

interface BottomSheetContentScope {
  fun item(content: (PaddingValues) -> Unit)
}

object BottomSheetContentScopeImpl : BottomSheetContentScope {
  override fun item(content: (PaddingValues) -> Unit) {
    content(bottomSheetItemPadding)
  }
}

[Chris Fillmore]

Is this overkill?

I don’t yet have any composables in my app that provide consistent layout, since my app is relatively UI-lite (it’s a camera app basically), so I’m wondering if this is a good approach

[Filip Wiesner]

Why do you feel like you need the BottomSheetContentScope? 🤔 Why not just a normal composable slot? content: @Composable () -> Unit

[Chris Fillmore]

I want to pass the PaddingValues, so that each row can have consistent padding

[Chris Fillmore]

Do you see what I mean?
This is kind of what I’m getting at; I wonder if I’m missing a simpler, obvious solution

[Chris Fillmore]

I suppose I could just create a function like

@Composable
fun BottomSheetRow(content: @Composable (PaddingValues) -> Unit) {
  content(bottomSheetItemPadding)
}

[Filip Wiesner]

But you would still have to pass the PaddingValues value for each item, right? That does seem like overengineering to me 😅

I would maybe do something like this:

@Composable
fun BottomSheetScreen(
  headerContent: @Composable () -> Unit,
  mainContent: @Composable ()  -> Unit,
) {...}

@Composable
fun BottomSheetScreenContentColumn(itemPadding: PaddingValues) {...}

[Filip Wiesner]

The point is to make more composables that do one thing and compose them together. Usually you want to avoid constraining the content of your composable functions to one type of content.

[Filip Wiesner]

Ofcourse 10 people, 10 different tastes but I would think that this is a good strategy to follow.
I really suggest

Thinking in Compose▾

video by Leland Richardson if you haven't seen it already. And even if you did see it 😄 I keep coming back and re-watching certain parts of it myself.

[Chris Fillmore]

It’s been a while but I think I have watched that. I’ll give it another look

[Chris Fillmore]

I agree with all you’ve said… let me take try another approach

[Chris Fillmore]

I’ll circle back

[Filip Wiesner]

And think you can end up with something like this in the end:

ControlBottomSheet(
    headerContent = {
        Header(Modifier.padding(paddingValues))
    }
) {
    ControlColumn(
        verticalArrangement = Arrangement.spacedBy(16.dp)
    ) {
        ControlItem(
            icon = painter(<http://R.drawable.xyz|R.drawable.xyz>), 
            text = stringResource(R.string.text),
        )

        ControlItem(
            icon = painter(R.drawable.abc), 
            text = stringResource(R.string.text2),
        ) {
            Switch(
                enabled = switchEnabledState, 
                onEnabledChange = { onEnablecChange(it) }
            )
        }
    }
}

[Chris Fillmore]

Mmm I see where you’re going. Yes I like that, however, the control rows are clickable so I guess spacedBy won’t work here

[Chris Fillmore]

I’ll just provide default padding to the control row

[Filip Wiesner]

Or if you can represent the controls as list of some data (List<ControlViewItem>), you could just iterate over the data and pass in the same Modifier. That'll result in uniform padding.
Example:

for (control in controls) {
    ControlItem(control, Modifier.padding(paddingValue)
}

[Filip Wiesner]

And because there is no constraint on what you put in your content composable slot parameter, you could use list of ControlViewItem in one place and call specific items in other place.