Return PopResult from OverlayHost.showFullScreenOverlay()

[aschulz90]

showFullScreenOverlay() now returns the PopResult that was popped by the shown Screen.

I had to change Overlay's generic parameter from Any to Any? to account for the nullable PopResult. I couldn't think of any obvious reason for why overlays shouldn't be able to also return null values. This shouldn't be a breaking change, as the generic parameter is now less restrictive!?

[ZacSweers]

What's the difference between a full screen overlay and just pushing a new screen onto the back stack? I'm also not a fan of breaking the return type contract of overlays just to support this case

[aschulz90]

My usecase is, that I have dialogs that are backed by Screen's, to seaparate their input validation and stuff to presenters. And also get a result after their completion.

Because they are dialogs, I can't just push them onto the backstack, because then the underlying screen wouldn't be visible anymore.

Simplified Example:

data class Group(
    val id: String,
    val name: String,
)

@CommonParcelize
data object CreateGroupDialogScreen: Screen {
    sealed interface Result: PopResult {
        data class Created(
            val group: Group,
        ): Result
        data object Canceled: Result
    }

    data class State(
        val error: String?,
        val eventSink: (Event) -> Unit = {},
    ) : CircuitUiState

    sealed interface Event : CircuitUiEvent {
        data class Submit(
            val name: String,
        ): Event
        data object Cancel: Event
    }
}

@Composable
fun CreateGroupDialogScreenPresenter(
    navigator: Navigator,
): CreateGroupDialogScreen.State {
    var error: String? by remember { mutableStateOf(null) }

    return CreateGroupDialogScreen.State(
        error = error,
    ) {
        when(it) {
            CreateGroupDialogScreen.Event.Cancel -> navigator.pop(CreateGroupDialogScreen.Result.Canceled)
            is CreateGroupDialogScreen.Event.Submit -> {
                if(it.name.isBlank()) {
                    error = "Required"
                    return@State
                }

                navigator.pop(
                    CreateGroupDialogScreen.Result.Created(
                        group = Group(
                            id = "123456",
                            name = it.name,
                        )
                    )
                )
            }
        }
    }
}

@Composable
fun CreateGroupDialog(
    modifier: Modifier,
    state: CreateGroupDialogScreen.State,
) {
    Dialog(
        onDismissRequest = {state.eventSink(CreateGroupDialogScreen.Event.Cancel)},
    ) {
        var name: TextFieldValue by rememberRetained { mutableStateOf(TextFieldValue()) }
        Column(
            modifier = modifier,
        ) {
            TextField(
                value = name,
                onValueChange = {name = it}
            )
            
            state.error?.let { 
                Text(it)
            }
            
            Button(
                onClick = {
                    state.eventSink(CreateGroupDialogScreen.Event.Submit(name = name.text))
                }
            ) {
                Text("Submit")
            }
        }
    }
}

In the parent screen I can then do:

// on button click
scope.launch {
  val result = overlayHost.showFullScreenOverlay(CreateGroupDialogScreen)
  state.eventSink(OtherScreen.Event.CreateGroupResult(result))
}

This of course could also be implemented like this right now with some more code:

var showDialog by rememberRetained { mutableStateOf(false) }

if(showDialog) {
  CircuitContent(
        screen = CreateGroupDialogScreen,
        onNavEvent = {
            when (it) {
                is NavEvent.Pop ->{
                    state.eventSink(OtherScreen.Event.CreateGroupResult(it.result))
                    showDialog = false
                }
                else -> error("not supported")
            }
        },
    )
}

// on button click
{ showDialog  = true }

---

As for the nullable Overlay return type, I have another usecase. A "Pick-One" dialog that also allows selecting None. The None-choice could simply be mapped to null, instead of having to handle it with wrappers.

Currently not possible:

OverlayEffect {
    val choice = it.show(
        BasicAlertDialogOverlay<List<String>, String?>(
            model = listOf("A", "B", "C"),
            onDismissRequest = { null },
        ) { model, navigator ->
            // navigator.finish("A")
        },
    )
}

Workaround with wrapper:

sealed interface DialogResult {
    data class Value(val value: String) : DialogResult
    data object None : DialogResult
}

OverlayEffect {
    val choice = it.show(
        BasicAlertDialogOverlay<List<String>, DialogResult>(
            model = listOf("A", "B", "C"),
            onDismissRequest = { DialogResult.None },
        ) { model, navigator ->
            // navigator.finish(DialogResult.Value("A"))
        },
    )
}

[ZacSweers]

It seems like you're just using the Screen input convenience of a full screen overlay to show a complex dialog (which isn't actually full screen), when you should instead just create a dialog overlay that accepts a screen input?

[aschulz90]

It's convinient, yes. They are technically fullscreen, because they are modal (fullscreen scrim + dialog). I could also duplicate the showFullScrrenOverlay for my app and implement the small adjustment, except using the wrapper workaround for non-nullable overlay results.
So if the nullable overlay results are not desired, then I will close this PR.

[stagg]

Not sure want a nullable return from an Overlay as we do want to emphasize the need to return a result.

That said the FullScreenOverlay does have a bit of an oddity in that it's dropping any returned PopResult. So maybe we update the FullScreenOverlay to have a result returned that wraps the optional PopResult?

[ZacSweers]

I'd be ok with an optional wrapper 👍

[aschulz90]

I have updated it to return the PopResult via a wrapper without nullable Overlay results.

[stagg]

Thanks for the changes!
Not sure whats up with CI 👀