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

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

@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")

}

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

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

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

@Bryan L thanks! Sealed classes makes sense. I'll update.

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

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.

Don't mutate the nav controller directly in a composable function

Adam, so for like this line https://github.com/ColtonIdle/ComposeSignInSample/blob/main/app/src/main/java/com/example/composesigninsample/MainActivity.kt#L118 You're recommending to just pass in a lambda?

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

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?

two orthogonal topics: what to call vs. where/how/when to call it.

I'm going to make a commit or two to this repo, to see if I'm understanding correctly. 🙃

that'll ensure that it only happens once when the effect enters the composition, yes

Okay. So if my composables recomposes a 100000 times, then using LaunchedEffect makes sure that it only happens once?

well, now we're back to product requirements, but in this case the product is your API surface here 🙂

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"

by "enter composition" I mean that it was not present before and it is present now

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

if you're curious, the whole thing works based on RememberObserver

Desparately need a lint rule to somehow tell people they are doing something bad during recomposition. lol

if you

remember { }

any object that implements

RememberObserver

, compose will invoke its callbacks during these events of the composition lifecycle

Okay. Updated the repo once more... I think I'm getting closer to something not completely terrible? https://github.com/ColtonIdle/ComposeSignInSample/commit/e648a8839f6bfd3afaa8425a482acbbfd7442326

yep, seems like a solid place to start from