https://kotlinlang.org logo
#compose
Title
# compose
t

Tin Tran

07/28/2022, 4:34 AM
Is it a good practice passing
ModalBottomSheetState
to ViewModel?
o

oianmol

07/28/2022, 5:14 AM
The ViewModel doesn't need to know if you have a bottom sheet or not 🤷 so i would choose not to provide the sheet state to VM.
t

Tin Tran

07/28/2022, 6:06 AM
So if i want to control the sheet state from VM I’ll have to add a boolean to the view model and emit it to compose and have something like this in compose
Copy code
LaunchedEffect(key1 = state.showBottomSheet) {
        if (state.showBottomSheet) {
            bottomSheetState.animateTo(ModalBottomSheetValue.Expanded)
        } else {
            bottomSheetState.animateTo(ModalBottomSheetValue.Hidden)
        }
    }
But if the user touch the scrim or swipe the bottom sheet down i’ll have to do this to notify the VM which I think is kind of cumbersome
Copy code
val bottomSheetState = rememberModalBottomSheetState(
        initialValue = ModalBottomSheetValue.Hidden,
        confirmStateChange = {
            if (!state.bottomSheetCancelable) {
                return@rememberModalBottomSheetState false
            }
            if (it == ModalBottomSheetValue.Hidden) {
                Timber.d("test test test")
                keyboardController?.hide()
                viewModel.reduce(LoginEvent.HideBottomSheet)
            }
            true
        }
    )
a

Albert Chang

07/28/2022, 10:22 AM
It’s perfectly valid. View models hold UI states and
ModalBottomSheetState
is a UI state.
o

oianmol

07/28/2022, 10:39 AM
The sealed class of UIState which you construct for your views is less complex, but bottomSheetState is a complex object. If in future you would want to move away from Bottom sheet's you would end up refactoring viewModels also, I think VM's should be simple enough and should have minimum android dependencies. It's like method injection when you tell your VM about the bottomSheetState which is also a composable and not just some kotlin data class for UIstate.
a

Albert Chang

07/28/2022, 10:51 AM
the bottomSheetState which is also a composable
No,
ModalBottomSheetState
is not a composable. It’s a UI state class.
If in future you would want to move away from Bottom sheet’s you would end up refactoring viewModels
What’s the problem of refactoring view models? Do you design your view model so that any future UI change doesn’t need any modification of view model? Is that possible anyway? Creating this kind of unnecessary restriction for yourself only makes your code more verbose and more complicated. We usually call this over-engineering.
o

oianmol

07/28/2022, 10:54 AM
It's a composable function form what I see, I would not write the bottom sheet interaction logic within the viewModel, it's like adding onClickListener in the VM
a

Albert Chang

07/28/2022, 10:56 AM
But we are talking about
ModalBottomSheetState
instead of
rememberModalBottomSheetState()
😄
o

oianmol

07/28/2022, 11:02 AM
It's a rememberSaveable, and if config changes happen then it will reconstruct the
ModalBottomSheetState
for you! So what you essentially passed to your VM will not be the same instance. Does that make sense ? lmk if something is not clear or if I am wrong?
Your VM's
ModalBottomSheetState
will be different after config change. that's why you should not pass it to your VM. It's an anti pattern. We are repeating the same mistakes again, it's like passing Activities to your AsyncTasks! 🤦
a

Albert Chang

07/28/2022, 11:06 AM
Oh maybe you didn’t notice that
ModalBottomSheetState
has a public constructor which you can directly use to initialize the class in the view model.
o

oianmol

07/28/2022, 11:08 AM
Why would you deal with all that when compose gives you
rememberModalBottomSheetState()
and take care of config changes with rememberSaveable ?
@Ian Lake or @Manuel Vivo might help here for best practices, or @theapache64 🙂
a

Albert Chang

07/28/2022, 11:11 AM
I don’t understand your question. What do I need to deal with when I can directly create a
ModalBottomSheetState
as a member of view model and the view model survives configuration changes?
o

oianmol

07/28/2022, 11:40 AM
I see of course you can write a
ModalBottomSheetState
and use in the VM, but by moving it there you give your VM the responsibility to deal with UI interactions, for me I think of it as if I am writing click listeners within the VM, but you are right we just end up calling
expand
and
hide
so it works. I am just thinking out loud, if I follow what you are mentioning here would it lead to memory leaks or some unexpected behaviour 🤷
f

fengdai

07/28/2022, 1:06 PM
Albert is right. All state objects are fine to be hoisted to view model. And no memory leak or unexpected behavior will happen.
Simple state hoisting can be managed in the composable functions itself. However, if the amount of state to keep track of increases, or the logic to perform in composable functions arises, it’s a good practice to delegate the logic and state responsibilities to other classes: state holders.
&
a ViewModel is a special type of state holder
c

Colton Idle

07/29/2022, 4:14 AM
Yeah. I do what Albert says as per @Manuel Vivos help on twitter.
j

jossiwolf

07/29/2022, 9:01 AM
Using
rememberModalBottomSheetState
means that you get saving for free, yes. But it's perfectly fine for the state to live in the VM as part of your UI state🙂 Follow the same state saving best practices for that as with the rest of your UI state.
t

Tin Tran

07/29/2022, 9:37 AM
Thanks guys
m

Manuel Vivo

08/01/2022, 9:10 AM
Our recommendation is to keep the state closest to where it’s needed. If the bottom sheet state isn’t required in the VM (i.e. it isn’t necessary to derive other UI state nor requires business logic), then don’t hoist it in the VM. It’d be better placed in the UI close to its component. As Jossi was saying,
rememberMBSS
saves the state across process and activity recreation. If you hoist it in the VM, you’d need to handle that yourself using the
SavedStateHandle
153 Views