https://kotlinlang.org logo
Join Slack
Powered by
do your composables look like this or am I doing s...
# compose
f
do your composables look like this or am I doing something wrong? Especially the AlertDialogs are adding a lot of arguments (shown boolean, click handlers for both buttons). Edit: To add more context, only 1 composable per screen looks like this, and it's basically an intermediary composable that contains all the smaller composables of the screen. I added this to keep my top-most screen composable smaller. But therefore it needs to handle all the data flow between the ViewModel and the rest of the screen.
goberserk 1
🎰 1
☝️ 2
r
You can pass component interface instead of lambdas
➕ 1
u
you mean like Listeners intertace containing all those methods?
r
Like this
d
You're doing something wrong according to Leland. You should use more slot APIs.
m
I would say theres definitely something wrong. that composable is doing too much imho. you can, as Dominic pointed out, utilise slots APIs better
composable function is still a function so too many arguments is not good
n
hey @Florian, I ran into this too on Friday. I discovered this which might help (and suggests what Rustam was saying): https://github.com/androidx/androidx/blob/androidx-main/compose/docs/compose-api-guidelines.md#hoisted-state-types
I have yet to try this though.
r
IMHO it does look like too much for a single composable. Not sure how’s your UI but I’d assume all these actions are triggered by different UI sections? I’d use slots APIs as suggested above. I’d also probably create separate interfaces for each UI section (for example 
interface AlertDialogActions
, 
interface ToolbarActions
). I’d make this root composable would only receive a single interface. Let’s say a parent interface that would derive from all the others (
interface ParentActions: AlertDialogActions, ToolbarActions
), so it looks cleaner and with less params
➕ 3
f
The Composable in my image is sitting between the ViewModel and the rest of the screen and contains all the smaller composables.
I could remove it and them I'm left only with smaller functions, but the top-most screen composable is smaller this way and gives me a better overview
@Nick Thank you I was looking for something like this
👍 1
but it seems like I can just get rid of this intermediary Body composable
I don't think a slot API would really change anything here
n
This is exactly what I’m running into. I have screen with a lot of functionality/callbacks. I can pass the viewmodel in, but then the previews don’t render. I wish there was a good solution that I knew of 😞
u
whats a slot api? @Composable lambdas params?
f
@ursus yes
@Ricardo García But for example, in a Scaffold it's more organized if you don't put all the Composables directly into the body slot and instead create a separate "BodyContent" composable. But then you can't avoid passing all the state and events through this single function.
if you guys don't have such a wrapper around your screen, what do you use to show the whole screen in the preview?
s
f
@Shakil Karim I do
but you need another composable between them otherwise you can't display a preview for that screen
as far as I know preview can't take a ViewModel arg
s
@Florian I am using this approach, copied for TIVI app
Copy code
@ExperimentalCoilApi
@ExperimentalMaterialApi
@Composable
fun Discover(
    viewModel: DiscoverViewModel,
    goToProfile: () -> Unit
) {

    val viewState by rememberFlowWithLifecycle(viewModel.state)
        .collectAsState(initial = DiscoverViewState.Empty)

    Discover(viewState, goToProfile, selectPack = { pack ->
        if (pack.badge == BADGE.GET) {
            viewModel.submitAction(DiscoverAction.AddOrUpdatePackToDatabase(pack))
        }

    }, onLectionSelected = {

    }, selectCategory = {

    }) { action ->
        viewModel.submitAction(action)
    }
}

@ExperimentalCoilApi
@ExperimentalMaterialApi
@Composable
internal fun Discover(
    state: DiscoverViewState,
    goToProfile: () -> Unit,
    selectPack: (Pack) -> Unit,
    onLectionSelected: (Lection) -> Unit,
    selectCategory: (String) -> Unit,
    actioner: (DiscoverAction) -> Unit,
) {}
All the action handle by ViewModel is provided with 
actioner: (DiscoverAction) -> Unit,
f
@Shakil Karim thank you very much, I'll take a closer look at this 👍
👍 1
ok I think the tivi approach is what I need
but @zhuinden recommended to me to use an interface instead of a sealed class, which makes sense to me
n
I started using an interface in my project for the first time, it really does simplify it. There’s no need to modify the params up the chain anymore which really saves some time.
f
yep
I think this is what @rsktash also initially suggested
@Shakil Karim you might wanna try an interface instead of the sealed class, I found it way simpler
and it seems to achieve exactly the same
s
@Florian Yeah, Interface will work as well, but I am using Action Class with when which helps in autocomplete and handle the event with MutableSharedFlow But Interface is also a good option, Do you have any example with Interface?
f
@Shakil Karim I don't have an example since my repo is private, but it's pretty straight forward
👍 1
the ViewModel implements the interface and then you can pass that interface instead of the "action" object in tivi
🙏 1
🙏 1
use it the same way
and in the preview you can just pass an empty Kotlin object
r
That was actually the approach I was suggesting. I think it looks cleaner just passing that interface. I’m having trouble with the preview though. When you mean an empty Kotlin object, you mean an anonymous object implementing that interface? I find it a bit cumbersome to implement the whole interface just for the preview, specially with large interfaces. Not sure if you mean something different or if there’s a workaround for it
f
@Ricardo García yea that's what I meant. It's verbose but I just add all the methods with empty bodies. I guess you can also put an empty implementation somewhere else for organization
👍 1
r
What about variables? If I have a huge list of variables like below in viewModel. Which approach would be appropriate ? A data class and an Interface?
Copy code
@Composable
fun AddUser(
    fullName: String,
    userName: String,
    phoneNumber: String,
    email: String,
    password: String,
    pin: String,
    myPin: String,
    onFullNameChange: (String) -> Unit,
    onUserNameChane: (String) -> Unit,
    onPhoneNumberChange: (String) -> Unit,
    onEmailChange: (String) -> Unit,
    onPasswordChange: (String) -> Unit,
    onPinChange: (String) -> Unit,
    onMyPinChange: (String) -> Unit,
)
f
@Rafiul Islam I thin you create State objects that encapsulate them
between the viewmodel and the outermost screen composable I put it in a data class called UiState
r
Do I have to use like UiState.copy(username = "something") every time I want to change a variable.
f
@Rafiul Islam no, I use the Flow 
combine
operator
l
I guess he is looking for somethign else. @Rafiul Islam I'm aware of 2 options you have: 1. You create a new object everytime, either with your approach or with 
combine
like Florian mentioned 2. you make the fields of UiState mutable, e.g. 
data class UiState(var isLoaded: Boolean, ..)
. Annotate the data class with 
@Stable
to benfit from compiler optimizations. This is explained in detail in compose guidelines
r
So Is option 2 is more suitable than option 1 in this case?
l
Yes option 2 would be better. It depends on if the fields are considered "stable". Which types are considered stable is explained here: https://github.com/androidx/androidx/blob/androidx-main/compose/docs/compose-api-guidelines.md#hoisted-state-types
👍 1
Check also the "Stable Types" section
r
Ok, Thanks. @Lilly Actually, I have read those sections you mentioned. But I guess I have to read again more carefully. Thanks again for your effort.
z
I'm back and yes my Composables are just like this in real project 😅
😆 1
j
Hey guys. As tivi project change a lot from last year, does somebody have any other example how to reduce number of callbacks that one composable can receive?
9 Views
Open in Slack
PreviousNext
Powered by