One Off the Slack: Avoid UI That Has Side Effects on Other UI

Igor Escodro asked:

Quick question regarding DisposableEffect. I have a ModalBottomSheetLayout with different contents based on user interaction. My idea is to clean all the content and clear the focus when the BottomSheet is hidden.

My current implementation is:

DisposableEffect(modalSheetState.currentValue) {
    onDispose {
        if (modalSheetState.isVisible.not()) {
            focusManager.clearFocus()
            sheetContentState = SheetContentState.Empty
        }
    }
}

Is it a correct implementation of DisposableEffect?

In addition to the code above which stays closer to the ModalBottomSheetLayout, each type of BottomSheet has a code similar to:

val focusRequester = remember { FocusRequester() }
LaunchedEffect(Unit) {
    delay(600)
    focusRequester.requestFocus()
}
TaskInputTextField(
    text = taskInputText,
    onTextChange = { text -> taskInputText = text },
    modifier = Modifier.focusRequester(focusRequester)
)

Google’s Adam Powell was unconvinced:

[The first code snippet is] using recomposition to generate edge events and it’s not clear that it will be accurate/not generate duplicate events in some recompositions

effects (including disposal) always run after the composition has been committed; they can’t influence the current frame. sheetContentState = SheetContentState.Empty looks like the sort of thing where you’re expecting sheetContentState to be consumed somewhere else in the same recomposition where the DisposableEffect changes and thereby “fires”

at best you’ll get a frame of jank where the original source data and sheetContentState are out of sync.

Igor explained a bit more:

Basically I have a ModalBottomSheetLayout that changes the BottomSheet content when the tabs in the BottomAppBar changes. Also, I want to keep the BottomSheet opened when the user rotates the screen.

So I tried to create a mechanism where the Scaffold content communicates with the ModalBottomSheetLayout, updating the SheetContentState to be possible to have multiple BottomSheet.

It works… But every new bevavior I try to add on it is complex task. In this case I want to try to open the keyboard automatically when the BottomSheet is opened.

I added a DisposableEffect to close the keyboard every time the BottomSheet is closed. Now I added a new one to clean all the content, setting to Empty again.

Adam then gave is this article’s title:

as a rule of thumb try to avoid UI that has side effects on other UI

onDispose() is triggered by recomposition, causing some composable to no longer be needed. Ideally, onDispose() should not itself be mutating state that triggers further recomposition elsewhere in your hierarchy.

Or, as Adam put it:

in this case, the contents of the sheet leaving the composition isn’t the significant event, the significant event is either the same event that caused them to leave the composition, or in some cases something like the end of an animation somewhere

sheetContentState in particular looks like it wants to be driven somewhere else and not by a presentation like this going away

Later, Google’s Sean McQuillan added:

Don’t hesitate to hoist the controller-style objects (e.g. FocusRequester, SoftwareKeyboardController) to the thing managing events (the controller). They don’t need to be coupled with compose, and it’s expected to call methods on them in other places (e.g. a ViewModel).

A good way to think of these hoisted controller objects as encapsulated (state+events) you can use to add features to your controller (e.g. ViewModel, stateful composable) without having to wire up primitive state/events yourself (edited)

Read the original thread in the kotlinlang Slack workspace. Not a member? Join that Slack workspace here!