In `@Composable` APIs, is it preferable to have a ...

# compose

[julioromano]

In @Composable APIs, is it preferable to have a default onClick param as an empty lambda (onClick: () -> Unit = {}) or as a nullable function (onclick: (() -> Unit)? = null )? Are there any pros and cons?

[albertosh]

API-wise, I prefer to work with non nullable objects while possible. It’s even a design pattern, the null object one. Applied to this usecase, our “null” object would be an empty closure so we can execute it freely without any concerns
But beware that that’s API wise, it just makes developer life easier. But it comes with some penalties at runtime as doing a null check will probably be more performant than executing a closure, probably someone else can bring more light on the topic
Of course, we have to put everything in a context. For this scenario, I don’t think that an empty closure exeution will have a significant impact in the overall performance. But it’s something to keep in mind

[Bryan Herbst]

Offering an empty onClick as a default seems strange- it implies that your Composable requires handling click behavior, but offers doing nothing as a default. If I think about this in the context of a Button for example, I expect clicking the button would produce a ripple or other indication when I tap it, but that just wouldn’t do anything. That doesn’t sound like expected behavior.

On the other hand, making the onClick nullable implies that handling clicks is optional. A good example of this type of API design decision in action is Checkbox, which offers a nullable onCheckedChange. Checkbox only adds the selectable() Modifier when this parameter is not null, so it doesn’t handle clicks at all when onCheckedChange is null.

Short version: your choice here implies behavior for the component, and you should consider what correct behavior for your component is, including what’s required vs. what’s not.

[julioromano]

Cool, I like this semantics:
1. onClick: () -> Unit = {} => onClick is required
2. onclick: (() -> Unit)? = null => onClick is optional
But since in case 1 onClick is required, why having a default value at all? Doesn’t the = {} add confusion?
It seems sort of… Yeah this parameter is required but you don’t have to pass it in…

[Albert Chang]

That's why you shouldn't provide a default value when it is required.

[Alexandre Elias [G]]

note that there is an important behavior difference between passing null or {} to a Composable like Checkbox. when onClick is null, we do not provide a semantic onClick action to screenreaders, and they change their behavior to "inert UI element" instead of "clickable UI element".

[Alexandre Elias [G]]

this accessibility concern was actually the primary motivation why we decided to make optional onClicks nullable

[Alexandre Elias [G]]

likewise, in your library components, the recommended pattern is that when your onClick is optional, make sure that your implementation looks like val clickable = if (onClick != null) { Modifier.clickable(...) } else { Modifier } ... `modifier.then(clickable)`(as opposed to the shorter way modifier.clickable(onClick ?: {}, ...) ). That way, not only will you get a slight performance improvement, but more importantly it will send the right signals to accessibility

[julioromano]

I was a bit misled by the new (albeit experimental) API of Surface in beta09: it’s got two different @Composables with either no onClick at all or a non-null one.
Since I needed an optional onClick in my custom Composable (which uses Surface internally) I had to use the version of Surface without onClick and then place a Box inside it and apply the onClick modifier to the Box:

modifier = Modifier
        .fillMaxSize()
        .run {
          if (onClick == null) this
          else clickable(onClick = onClick)
        }

[Alexandre Elias [G]]

I see. I understand you needed to do that in order to get the indicator ripple to clip correctly. In fact, an earlier version of Button did it this way. However, we found a problem with this approach: the strong Compose convention is to put passed-in modifier parameter on the outermost Layout, but given that the Box is inside the Surface, either this convention must be violated, or a testTag that's passed in will appear not clickable

[Alexandre Elias [G]]

here are two alternatives I would suggest: A) stop using Surface itself and instead directly chain the elevation, shadow, clip modifiers, B) conditionally use one Surface overload or the other by storing the "content" lambda in a val instead of following the usual concise last-lambda style

[Alexandre Elias [G]]

there is a subtle behavior difference between A and B. A will pass through input events to other elements covered by the composable when nonclickable, and B will unconditionally block them (Surfaces are opaque to input). in usecases like `Switch`/`Slider` we use solution A inside Compose, whereas in cases like `Pane`/`Drawer`/`Card` I recommend B instead

[julioromano]

This was a very good explanation and helped a lot. Thank you!