Search code examples
androidkotlinandroid-jetpack-composekotlin-flowjetpack-compose-navigation

Issue with Composable Navigation and ViewModel Updates


I have viewmodel on that base I want to navigate through screens. I am adding very basic example to explain my problem.

MainActivity

class MainActivity : ComponentActivity() {

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContent {
            NavigationScreen()
        }
    }
}

MainViewModel

class MainViewModel(private val navigationHandler: NavigationHandler) : ViewModel() {

    val currentDestination: StateFlow<ScreenName?> =
        navigationHandler.destination.stateIn(
            viewModelScope,
            SharingStarted.WhileSubscribed(),
            initialValue = null
        )

    fun navigateToDeviceSelection(isValid: Boolean) {
        val destination = if (isValid) {
            ScreenName.ScreenOne
        } else {
            ScreenName.ScreenTwo
        }
        println(">> destination ${destination.route}")
        navigationHandler.navigate(destination)
    }
}

I created a custom navigation class which handle the logic and navigate the screen accordingly using StateFlow.

NavigationDestination

interface NavigationDestination {
    val route: String
}

NavigationHandler

interface NavigationHandler {
    val destination: StateFlow<ScreenName?>
    fun navigate(navigationDestination: ScreenName)
}

Navigator

class Navigator : NavigationHandler {

    private val _destination: MutableStateFlow<ScreenName?> = MutableStateFlow(null)

    override val destination: StateFlow<ScreenName?> = _destination.asStateFlow()

    override fun navigate(navigationDestination: ScreenName) {
        _destination.value = navigationDestination
    }
}

ScreenName

sealed class ScreenName(override val route: String) : NavigationDestination {

    object ScreenOne : ScreenName("ScreenOne") {
        object ChildOne : ScreenName("ChildOne")
        object ChildTwo : ScreenName("ChildTwo")
    }

    object ScreenTwo : ScreenName("ScreenTwo")
}

Now when I consume this MainViewModel inside composable like below code

@Composable
fun NavigationScreen(
    viewModel: MainViewModel = koinViewModel(),
    navController: NavHostController = rememberNavController()
) {

    val destinationState by viewModel.currentDestination.collectAsStateWithLifecycle()

    LaunchedEffect(destinationState) {
        destinationState?.let {
            navController.navigate(it.route) {
                popUpTo(navController.graph.startDestinationId) {
                    inclusive = true
                }
                launchSingleTop = true
            }
        }
    }

    LaunchedEffect(viewModel) {
        viewModel.navigateToDeviceSelection(true)
    }

    NavHost(
        navController = navController,
        startDestination = ScreenName.ScreenOne.route,
        route = "parentRoute"
    ) {

        nestedGraphSample(navController, viewModel)

        composable(ScreenName.ScreenTwo.route) {
            Text(text = "Screen Two Route")
        }
    }
}

private fun NavGraphBuilder.nestedGraphSample(
    navController: NavHostController,
    viewModel: MainViewModel
) {
    navigation(
        startDestination = ScreenName.ScreenOne.ChildOne.route,
        route = ScreenName.ScreenOne.route
    ) {
        composable(ScreenName.ScreenOne.ChildOne.route) {

            LaunchedEffect(Unit) {
                delay(5.seconds)
                navController.navigate(ScreenName.ScreenOne.ChildTwo.route) {
                    popUpTo(navController.graph.startDestinationId) {
                        inclusive = true
                    }
                    launchSingleTop = true
                }
            }

            Text(text = "Screen Child One Route")
        }

        composable(ScreenName.ScreenOne.ChildTwo.route) {
            LaunchedEffect(Unit) {
                delay(5.seconds)
                viewModel.navigateToDeviceSelection(true)
            }
            Text(text = "Screen Child Two Route")
        }
    }
}

My primary issue arises when the user initially invokes navigateToDeviceSelection, leading to redirection to ScreenName.ScreenOne and the storage of this value within destinationState. Subsequently, when navigating via navController, the destination state remains unchanged, resulting in redirection to another screen. Additionally, upon invoking navigateToDeviceSelection within ScreenName.ScreenOne.ChildTwo.route, the destination state fails to update, rendering the LaunchedEffect ineffective, and causing the user to remain stuck in the ScreenName.ScreenOne.ChildTwo.route screen. To address this challenge, I seek a solution that avoids accessing destinationState within nested composable functions, as this example is basic, and I prefer not to pass this variable to nested children.


Solution

  • I think to update your code to the Android developer guidance, the shortest path is:

    1. Add a function to report the navigation event has been handled to your NavigationHandler:
    class Navigator : NavigationHandler {
    
        private val _destination: MutableStateFlow<ScreenName?> = MutableStateFlow(null)
    
        override val destination: StateFlow<ScreenName?> = _destination.asStateFlow()
    
        override fun navigate(navigationDestination: ScreenName) {
            _destination.value = navigationDestination
        }
    
        /** Must be called by collecting class when navigation event has been responded to. */
        override fun consumeNavigationEvent() {
            _destination.update { null } 
        }
    }
    
    1. It seems that currently your NavigationHandler layer is redundant complexity because it doesn't abstract away anything for the ViewModel. Any change made to the NavigationHandler also has to be made to the ViewModel to be able to handle it. It's also very weird that you are using stateIn on a StateFlow, creating a redundant StateFlow. I would just delete the entire NavigationHandler and Navigator, but if you don't, you need to at least remove the redundant StateFlow and need to pass through this new function.
    class MainViewModel(private val navigationHandler: NavigationHandler) : ViewModel() {
    
        val currentDestination: StateFlow<ScreenName?> get() =
            navigationHandler.destination
    
        fun navigateToDeviceSelection(isValid: Boolean) {
            val destination = if (isValid) {
                ScreenName.ScreenOne
            } else {
                ScreenName.ScreenTwo
            }
            println(">> destination ${destination.route}")
            navigationHandler.navigate(destination)
        }
    
        /** Must be called by collecting class when navigation event has been responded to. */
        override fun consumeNavigationEvent() {
           navigationHandler.destination.consumeNavigationEvent()
        }
    }
    
    1. Where you are responding to events, you must consume the event when you use it.
        LaunchedEffect(destinationState) {
            destinationState?.let {
                viewModel.consumeNavigationEvent()
                navController.navigate(it.route) {
                    popUpTo(navController.graph.startDestinationId) {
                        inclusive = true
                    }
                    launchSingleTop = true
                }
            }
        }
    

    But like I said, I think you can simplify your code quite a bit by eliminating the NavigationHandler and Navigator.

    You might also consider clarifying some of your function names. For example NavigationHandler.navigate() and MainViewModel.navigateToDestination() do not actually perform any navigation. They only set the desired new destination. It makes it confusing that your Composable has to tell the ViewModel to navigate and then it also has to tell the NavHost to navigate, but these are doing two different things.