https://kotlinlang.org logo
Join Slack
Powered by
Code review request! Please be nice as compose + n...
# compose
c
Code review request! Please be nice as compose + navigation makes my brain hurt. Project = login/logout screens + compose + AAC compose nav + Hilt + AAC ViewModel These ~100 lines of code will serve as the base of my project. Appreciate any input you may have. https://github.com/ColtonIdle/ComposeSignInSample/blob/main/app/src/main/java/com/example/composesigninsample/MainActivity.kt
b
I prefer making sealed classes for my navigation. Easy to give each screen different parameters. For example, in the event you use it for a bottom nav bar.. you can call the icon or the title of the screen's name
Copy code
@Composable
fun AppRouter() {
    val navController = rememberNavController()
    NavHost(navController = navController, startDestination = Screen.HomeScreen.route) {
        composable(Screen.HomeScreen.route) { HomeScreen(navController = navController) }
        composable(Screen.SignInScreen.route) { SignInScreen(navController = navController) }
    }
}

sealed class Screen(val route: String) {
    object HomeScreen : Screen("home")
    object SignInScreen : Screen("signin")

}
Copy code
@Composable
fun AppRouter() {
    val navController = rememberNavController()
    NavHost(navController = navController, startDestination = Screen.HomeScreen.route) {
        composable(Screen.HomeScreen.route) { HomeScreen(navController = navController) }
        composable(Screen.LoginScreen.SignInScreen.route) { SignInScreen(navController = navController) }
    }
}

sealed class Screen(val route: String) {
    object HomeScreen : Screen("home")

    sealed class LoginScreen {

        object SignInScreen : Screen("signin")
        object RegisterScreen : Screen("register")
    }
}
You could also do something like this if you'd prefer to show it as subroute. Someone else will give better insight. But just some food for thought! (still only calls "signin". You can play around with it. 🙂 )
Copy code
fun AppRouter() {
    val navController = rememberNavController()
    NavHost(navController = navController, startDestination = Screen.HomeScreen.route) {
        composable(Screen.HomeScreen.route) { HomeScreen(navController = navController) }
        composable(Screen.LoginScreen.SignInScreen.subRoute) { SignInScreen(navController = navController) }
    }
}

sealed class Screen(val route: String) {
    object HomeScreen : Screen("home")

    sealed class LoginScreen(val subRoute: String) : Screen("login") {

        object SignInScreen : LoginScreen("signin")
        object RegisterScreen : LoginScreen("register")
    }
➕ 1
a
I think perhaps passing lambdas instead of passing the entire navigation controller might be better idea. It isolates you from debugging navigation related down the stack in the future when the app becomes more complicated
➕ 6
c
@Bryan L thanks! Sealed classes makes sense. I'll update.
@Abhishek Dewan makes sense to me too, but I think when listening to the latest Q&A session with Murat and Ian, I think Ian mentioned that it should be safe to only pass in a navController into the top level screen, but he wouldn't rec passing it down any deeper than that.
💯 2
a
if its a single developer on a project it shouldn't be a problem at all since you can control everything. My project I pass lambdas just to make sure I kinda get used to using them like that so that when my work adapts compose I can impart it to our team :)
c
Does anyone here see anything wrong with how the sign in/out logic is handled? For example, if you run the repo, it doesn't actually work correctly.
a
Don't mutate the nav controller directly in a composable function
➕ 2
c
a
your call, you could go either way depending on the shape of the rest of your code. The lambda is more generic/probably more testable. And like in the other thread, don't call it directly in composition. 🙂
c
you could go either way depending on the shape of the rest of your code.
What's either way? I figured the only option is to pass in a lambda, or do what I'm doing currently (calling navigate directly). Is there another option?
a
two orthogonal topics: what to call vs. where/how/when to call it.
c
I'm going to make a commit or two to this repo, to see if I'm understanding correctly. 🙃
@Bryan L made your change recommendation here! https://github.com/ColtonIdle/ComposeSignInSample/commit/2df04c93cd17a6153f30e0d4486a02a601cefdf2 Let me know if that's what you were thinking of. Now moving onto Adams recommendation.
@Adam Powell was this what you were referring to in terms of using LaunchedEffect? Or am I still wrong. https://github.com/ColtonIdle/ComposeSignInSample/commit/adef3b2897d794a9e20cc96ca0fa09acb891148e
a
that'll ensure that it only happens once when the effect enters the composition, yes
there's some details if you wanted to be 100% correct like probably use 
navController
instead of 
Unit
as the key parameter to 
LaunchedEffect
c
Okay. So if my composables recomposes a 100000 times, then using LaunchedEffect makes sure that it only happens once?
And what if I didn't want to pass in navController, and instead wanted to pass in a lambda. What would go into the launched effect as the key?
a
well, now we're back to product requirements, but in this case the product is your API surface here 🙂
strictly speaking 
LaunchedEffect
launches a coroutine to run the trailing lambda block when it first enters composition and cancels it when it leaves the composition. If any of the key parameters change while the 
LaunchedEffect
is still present, then it will cancel the coroutine and start a new one again.
now, lambda captures can change. 
LaunchedEffect
won't change its lambda capture out from under the running coroutine, at least not on its own.
You could even argue that changing for the 
navController
here is wrong and 
Unit
is more correct in this situation.
🤯 1
c
first enters composition and cancels it when it leaves the composition.
Hm. Let me make sure I understand. "first enters composition" is like when a HomeScreen composable is added to the NavHost? or by "enter composition" do you mean everytime it recomposes it "enters composition"
a
by "enter composition" I mean that it was not present before and it is present now
c
Got it. That's what I thought. It's kinda like the lifecycle of the composable. So even thought a composable can recompose 10000 times... it typically only enters composition and exits composition once during a use case of a composable being an entire screen. Hence why launchedEffect works nicely for what I want to do
a
if you're curious, the whole thing works based on RememberObserver
c
Desparately need a lint rule to somehow tell people they are doing something bad during recomposition. lol
a
if you 
remember { }
any object that implements 
RememberObserver
, compose will invoke its callbacks during these events of the composition lifecycle
👍 1
c
Okay. Updated the repo once more... I think I'm getting closer to something not completely terrible? https://github.com/ColtonIdle/ComposeSignInSample/commit/e648a8839f6bfd3afaa8425a482acbbfd7442326
a
yep, seems like a solid place to start from
👍 1
Open in Slack
PreviousNext
Powered by