https://kotlinlang.org
Join Slack
Menu
Home
Archive
:thread: Is there any reason why the invoke operator in a sealed interface with inheritance doesn’t ...
# compose
a
🧵 Is there any reason why the invoke operator in a sealed interface with inheritance doesn’t work?
Code Example:
Copy code
sealed interface Button {

    sealed interface Filled : Button {
        object Primary : Filled
        object Secondary : Filled
//        object Success : Filled
//        object Warn : Filled
    }

    sealed interface Outlined : Button {
        object Primary : Outlined
        object Secondary : Outlined
//        object Success : Filled
//        object Warn : Filled
    }

    sealed interface Text : Button {
        object Primary : Text
        object Secondary : Text
//        object Success : Filled
//        object Warn : Filled
    }

    @Composable
    operator fun invoke(
        onClick: () -> Unit,
        modifier: Modifier = Modifier,
        enabled: Boolean = true,
        interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
        content: @Composable RowScope.() -> Unit,
    ) {
        val indication = rememberRipple(true, color = LocalContentColor.current)

        CompositionLocalProvider(
            LocalContentColor provides contentColor,
            LocalTextStyle provides Typography.mediumStrong,
            LocalIconSize provides 20.dp,
        ) {
            Row(
                modifier = Modifier
                    .clickable(
                        interactionSource = interactionSource,
                        indication = indication,
                        enabled = enabled,
                        role = Role.Button,
                        onClick = onClick,
                    )
                    .background(backgroundColor)
                    .height(40.dp)
                    .padding(16.dp, 0.dp)
                    .then(modifier),
                verticalAlignment = Alignment.CenterVertically,
                horizontalArrangement = Arrangement.Center,
            ) {
                content()
            }
        }
    }

    private val contentColor
        @Composable
        @ReadOnlyComposable
        get() = when (this) {
            is Filled -> Color.White

            is Outlined.Primary, is Text.Primary -> Theme.colors.primary
            is Outlined.Secondary, is Text.Secondary -> LocalContentColor.current
        }

    private val backgroundColor
        @Composable
        @ReadOnlyComposable
        get() = when (this) {
            is Filled.Primary -> Theme.colors.primary
            is Filled.Secondary -> Theme.colors.primary.copy(ContentAlpha.secondary)

            else -> Color.Transparent
        }
}
(this can be stripped down to bare minimum too, same result)
Here is the stack trace, my guess is the compiler isn’t transforming the call site to a composable
Untitled.txt
e
not related to 
sealed
a
Yeah, wasn’t sure if that was a cause or not, but it’s my usecase 🙂
But it’s just the default impl throwing the issue, wow
I can make an extension function with generics… but then you need to import the invoke function… very dirty solution and I hate it. 🥴
k
Why not just create a top Composable function that receives your "style" sealed interface as a parameter and decide each style color/configuration just as you did with the 
contentColor
and 
backgroundColor
a
Reducing the amount of parameters that get passed in
That's the way it's currently done
Plus all of this is gonna be handed off to Jr devs to work on eventually, this will be cleaner to read and document
k
I have to disagree with that, I don't think there's anything wrong with the approach of passing the style to a 
Button
Composable, you've already reducing the amount of parameters needed by defining the style
a
I'm not saying it's wrong to do it the other way, but this approach encapsulates everything
It's just a different way of approaching the same problem
k
Yeah, I'm not saying that either, but it's not a very idiomatic way to do it with Compose
Moreover if you're going to hand it over to junior devs, that'll probably confuse them as I haven't seen any other place that follows this approach, the official Compose library follows the approach I mentioned you in my comment above, just plain arguments
a
I'm not discussing the implementation of this here, either. In the end I'll have to go back to my old way of doing it regardless, just more of a shortcoming of the compose compiler. There's nothing really that differentiates this way from the other way other than the composable function being attached to a sealed interface default impl, it's still a function either way
But bear in mind, in this corporate design system that I'm porting over we have, not every certain combos that just shouldn't exist. This impl lets me be able to define everything in a finite way as opposed to having a combination for everything as well 🙂
Having examples of how kotlin can be used idiomatically and extended upon can really empower someone to do their best work, or look into how it works
So while there are plenty of examples of just passing in params, etc, there are some other ways of doing things that can be learned and gatekeeping that knowledge away isn't in anyone's best interests on my team
k
Yeah, we did something similar to what you're mentioning with the design system to keep the combinations restricted to what the design system allows, in our codebase we have a 
ButtonStyle
that contains the allowed styles, each style returns the properties used in the top composable function called 
Button
for example:
Copy code
@Composable
fun Button(
    modifier: Modifier = Modifier,
    style: ButtonStyle,
    // ...
) {
   val isPressed = // ...
   val foregroundColor = style.foregroundColor(isPressed)
   
   // ....
}
Here for example each style overrides the 
foregroundColor
composable function that returns a different 
State<Color>
depending on whether the button is pressed or not, similar to what you did with the accessors, so I'm not saying your approach is wrong at all, what doesn't convince is the override of the 
invoke
operator, that's all
a
Yes, that's how it's currently done, just not satisfied with it
k
To add on why the override/overload of the 
invoke
operator doesn't convince me is that it's a bit obscure to use as part of a defined interface and its objects, while using a top function is clearer in this context
e
all the standard composables are top-level functions. this design forces them be pure (unless you have mutable top-level properties, which is a serious code smell). while you're not intending for it here, making member functions composable makes it all too easy to accidentally add state (from properties or functions of 
this
) that isn't explicitly passed in.
so while this is clearly a bug in the compose compiler, you're also doing something quite non-standard
a
I've used it with enums before without any hangups
I also made sure to make the member properties purely read-only
e
and it'll work if you use a 
sealed class
instead of a 
sealed interface
, but it's still not standard practices
a
@Composable
with 
@ReadOnlyComposable
Ah, that's worth a try
e
yes, you are currently purely read-only. but there's no safeguards for another developer coming in and adding another subtype.
a
Very true, but it's also a teachable moment
e
which is completely avoidable if you stick to the same patterns as what comes out of the box with Compose. your project your rules, but I don't see how this makes it good for jr devs at all
k
For anyone else looking, reference to how it's done on the 
Switch
composable with 
SwitchColors
: https://cs.android.com/androidx/platform/tools/dokka-devsite-plugin/+/master:testData/compose/source/androidx/compose/material/Switch.kt?q=Switch
a
Yes, that is also the goal with this moving it to a separate class 🙂
Yeah, still moot regarding sealed classes too. I’ll go back to having individual composables referencing a private impl as it was before
e
I want to note that the Compose library code is written with specific ABI concerns in mind that you don't have to worry about unless you are also publishing a library that is expected to evolve through binary-compatible releases
also the current implementation of m3 Switch is at https://github.com/androidx/androidx/blob/androidx-main/compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/Switch.kt - not sure what the file Kevin linked to is used for
a
It was pulled from the wrong folder in codesearch, but yes, the proper one should mirror that
e
mostly, but the real m3 implementation has some different choices, such as 
@Immutable class SwitchColors internal constructor
instead of 
@Stable interface SwitchColors; @Immutable private class DefaultSwitchColors(...) : SwitchColors
. what you linked looks closer to the m2 implementation (which is still elsewhere in that repo). maybe there were other factors at play but the way m3 works looks clearly better to me for this usage
a
CS is up to date with AOSP upstream, it should have the most up to date impl, gh merely syncs to it: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/[…]l3/src/commonMain/kotlin/androidx/compose/material3/Switch.kt
The previous CS link was the dokka devsite one is iffy to use afaik
e
a
CS is useful for search, which is why I use it, I hate using gitiles haha 😆
But yeah, that’s always most up to date, alongside gerrit
15 Views
Open in Slack
PreviousNext
Powered by

kotlinlang

A modern programming language that makes developers happier.

Powered by