Im looking for some feedback/thoughts; I have a co...

# compose

[Zoltan Demant]

Im looking for some feedback/thoughts; I have a composable that renders a loading indicator followed by an image (using coil3). Id like to make the image clickable sometimes, but Im not sure what the best approach is. More details in 🧵

[Zoltan Demant]

• One alternative Ive looked into is simply passing Modifier.clickable to my composable, but compose best practices tells me that this is likely a bad idea (I should apply the modifier to the root composable). I do like that its flexible though.
• My earlier approach was to simply declare onClick: (() -> Unit)? for my composable, this makes the functionality clear, but I also end adding more and more to it over time.

[Zoltan Demant]

Of course, if I add the modifier to the root of my composable, the clickable ripple will reach outside of the image, which is not what I want.

[Zoltan Demant]

My composable is a bit long, so I put together this shorter example ...

@Composable
fun Awesomeness(
    title: String?,
    modifier: Modifier = Modifier,
) {
    AnimatedContent(title) { current ->
        if (current != null) {
            Circle(
                modifier = modifier, // nested usage
                color = Color.Blue,
                size = 40.dp,
                content = {
                    Text(current)
                },
            )
        } else {
            Indicator(
                modifier = modifier, // nested usage
                size = Small,
            )
        }
    }
}

[Ian Lake]

Sounds like a good place to use a slot based API: https://developer.android.com/jetpack/compose/layouts/basics#slot-based-layouts

[Zoltan Demant]

Thanks for the feedback Ian, I havent really considered using slots for this since Im doing quite a lot in the content block already:

when (state) {
        is Success -> {
            Image(
                modifier = Modifier
                    .clip(shape)
                    .conditional { onClick?.let { clickable(onClick = onClick) } },
                painter = state.painter,
                contentDescription = contentDescription,
            )
        }

        is Error -> {
            Icon(
                icon = Theme.icons.networkError,
                emphasis = Moderate,
            )
        }

        is Loading -> {
            Indicator(Medium)
        }

        is Empty -> Unit
    }

[Sean Proctor]

I don't think passing a modifier like that is a good idea. You're creating the modifier in a different context than you're using it, which has led me to a number of bugs.

[Sean Proctor]

If you use the composable in a column and pass in a modifier with a weight, you're going to get a really unexpected result.

[Zoltan Demant]

I completely agree with you Sean. I havent found a better way than just declaring onClick and onLongClick as additional function params. Inline with what Ian mentioned earlier, maybe using a slot in this manner is a good idea:

fun interface ImageScope {
    
    @Composable
    fun Image(modifier: Modifier)
}
@Composable
fun Image(
    imageContent: @Composable (ImageScope) -> Unit
){}