Could use some code review on a "wrapper" composab...
# compose
c
Could use some code review on a "wrapper" composable that I want to use in order to signify that a screen requires a logged in state. Thoughts on this?
Copy code
@Composable
fun RequireSignedInUser(
  navController: NavController,
  appStateHolder: AppStateHolder,
  content: @Composable () -> Unit
) {
  if (appStateHolder.isLoggedIn) { content() } else { LaunchedEffect(Unit) { navController.navigate(route = Screen.LoginScreen.route) }
  }
}
Essentially I have this currently:
Copy code
composable(Screen.Tab1.route) { Tab1Screen() }
composable(Screen.Tab2.route) { Tab2Screen() }
composable(Screen.Tab3.route) { Tab3Screen() }
but I want tab 1, 2, and 3 to all kick the user to the loginScreen if the user is logged out. So I created RequireSignedInUser and then use it as such
Copy code
composable(Screen.Tab1.route) { RequireSignedInUser(navController, appStateHolder){Tab1Screen()} }
composable(Screen.Tab2.route) { RequireSignedInUser(navController, appStateHolder){Tab2Screen()} }
composable(Screen.Tab3.route) { RequireSignedInUser(navController, appStateHolder){Tab3Screen()} }
The two things I'm "worried" most about is just my general usage of this wrapper for convenience. Maybe it's just kinda bad to follow this pattern? Second thing I'm worried about is the LaucnhedEffect(Unit) inside of an if statement. Kinda feels smelly?
t
I would perform the signed-in-user logic in the layer that is hosting these tabs, then you don't need the side effect in each destination composable
z
Better to just compose the login screen directly instead of trampolining a navigation call.
c
I def feel this way as well, but my team is going with the direction that the arch events docs state. Which basically uses nav calls.
i.e.
t
is
AppStateHolder
stable/immutable? i wonder if there’s overhead in adding the wrapper for each destination. Agree with Tad, the login check should happen outside of the nav graph (i.e you dont see that nav graph unless you are logged in)
Curious, what did u end up going with?
c
At the moment I'm going for basically what the docs show. So the user technically always sees the nav graph and the check is done per destination.