Hello, everybody! :wave: Quick question regarding `DisposableEffect`. I have a `ModalBottomSheetLay...

# compose

[escodro]

Hello, everybody! 👋

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
        }
    }
}

The code is working, but in all samples I see there is a code before onDispose(), like a callback register.

Is it a correct implementation of DisposableEffect?

Thanks a lot for your help! ❤️

[Zach Klippenstein (he/him) [MOD]]

That seems fine to me. The only thing to watch out for is that you’re not performing side effects from your composable directly, where a “side effect” is basically changing any state that isn’t backed by a snapshot state object. I’m guessing focusManager is probably safe to do that with (assuming you are also requesting focus at some point), but not sure.

[escodro]

Thanks, Zach. 😊
Yeah, I’m having some difficulties to understand Effects and this is the only way I found to fix it.
I will let you know if I find a better approach.

[Zach Klippenstein (he/him) [MOD]]

Can you share more of your code? Not sure how all those pieces fit together

[escodro]

Sure!
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)
)

I needed to add the delay due to the BottomSheet reveal animation.

[Adam Powell]

The code in the OP looks suspicious and makes me wonder about the overall data flow

[Adam Powell]

it's using recomposition to generate edge events and it's not clear that it will be accurate/not generate duplicate events in some recompositions

[Adam Powell]

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"

[Adam Powell]

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

[Adam Powell]

(bah, slack's text editor)

[escodro]

I see…
I think that the code that I created is more complex than it should be, but it is the only way I found out to create the behavior I wanted… 😔

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.

The complete (also a little confusing) code is this:
https://github.com/igorescodro/alkaa/blob/ff0878e63730d70b60d6a3c74d01285fe03a323d/app/src/main/java/com/escodro/alkaa/presentation/home/Home.kt#L76

[escodro]

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.

[Adam Powell]

cc @Sean McQuillan [G] for the keyboard case in particular

[escodro]

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 Powell]

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

[Adam Powell]

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

[Adam Powell]

those events aren't generated by recomposition itself

[Adam Powell]

a direct transliteration of the code in the OP might do something like:

LaunchedEffect(modalSheetState, focusManager) {
  snapshotFlow { modalSheetState.isVisible }
    ...
    .collect {
       // take action
    }
}

so that the flow collection scoped to the composition via LaunchedEffect is what drives the action, rather than a recomposed effect itself. This will prevent the one frame delay/jank, but it won't resolve any other potential data flow/distribution of responsibility questions present here

[Adam Powell]

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

[Adam Powell]

and if UI goes away that had focus, it can't have focus anymore if it doesn't exist and the effect of clearing focus should be more or less a no-op

[escodro]

Thanks a lot, Adam. ❤️
The code you proposed worked as expected.

I know my code is probably over engineering the communication between the Scaffold and ModalBottomSheetLayout, but I need to study more before develop a better solution. 😅

[Sean McQuillan [G]]

It sounds like you got it already, but adding a quick note for future readers:

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).

[Sean McQuillan [G]]

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