raamcosta / compose-destinations

Annotation processing library for type-safe Jetpack Compose navigation with no boilerplate.
https://composedestinations.rafaelcosta.xyz
Apache License 2.0
3.22k stars 133 forks source link

Access to the parent navigation controller(DestinationsNavigator) #21

Closed BVantur closed 2 years ago

BVantur commented 2 years ago

First of all, let me say we really appreciate the effort put into this library. We really like how easy it is to do navigation with this library in Android Compose. Thank you very much. πŸ‘πŸ‘πŸ‘Œ

Nevertheless, we stumbled on some problems with our use case. We are planning to start developing with Jetpack Compose and we are now in the phase of researching which navigation approach should we use in our app. For better understanding, here is a github repository to the project with the below-mentioned problem.

What we need is to have multiple DestinationsNavHosts in our app because we are using one DestinationNavHost(root) that is used for the main flow of the app(SplashScreen -> HomeScreen,ExternalDetailsScreen) and another DestinationNavHost(nested) that is used as a container for Drawer menu items(ProfileScreen, InternalDetailsScreen) in the home screen.

OUR APPLICATION FLOW:

Screenshot 2021-12-01 at 09 22 38

So, our pain point in our application is ProfileScreen. There we have 2 buttons. Button with the text "Inside details screen" opens a new screen inside nested DestinationNavHost as is presented with InternalDetailsScreen above in the Image. For that, we use DestinationsNavigator from ProfileScreen and it works fine. But we have a problem with the button that has "External details screen" for a text, and here is expected to show a new screen in the root DestinationNavHost. If we use DestinationsNavigator from ProfileScreen, the screen is presented as InternalDetailsScreen. But we want to replace the content in root DestinationNavHost. Currently, we are solving this with a companion object on MainActivity which stores the DestinationsNavigator in it from HomeScreen and then we access it when we press on the "External details screen" button to navigate to the correct screen.

Is there a capability in this library that we are not seeing to do that kind of navigation with parent DestinationsNavigator? We would really appreciate it if you could point us on how to use that with the current state of the library?

raamcosta commented 2 years ago

Hi @BVantur πŸ‘‹

First of all, thank you so much for the very detailed explanation πŸ‘ And thank you for the kind words.

In my understanding of jetpack's compose navigation, we shouldn't use nested NavHosts. I also found this answer which goes in the same direction: answer link

So, if we follow that advice, you should put Scaffold at the top level and use only one DestinationsNavHost. Then, both "HomeScreen" and "InternalDetailsScreen" would be part of a nested navigation graph. After this, you would only have one NavHostController, and it (or the DestinationsNavigator wrapper) could be used to navigate anywhere. Your Scaffold could react to the current destination, and in case that destination is not part of the "nested nav graph", then we just hide the Scaffold elements.

I'll take your github project and make some changes because I also wanna try putting into reality what I'm saying here. Maybe I even open a PR there πŸ™‚ no promises though!

Thank you again, and please let me know if my answer helped you

raamcosta commented 2 years ago

Actually, it's easier to just post a patch here πŸ™‚ This is meant for your compose-destinations branch.

expand to see the full patch ``` Index: app/src/main/java/sp/bvantur/nestednavigation/ui/screens/HomeScreen.kt =================================================================== diff --git a/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/HomeScreen.kt b/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/HomeScreen.kt deleted file mode 100644 --- a/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/HomeScreen.kt (revision 7c9873dbdcb218a0aed93fb27106ca834a8fa10e) +++ /dev/null (revision 7c9873dbdcb218a0aed93fb27106ca834a8fa10e) @@ -1,106 +0,0 @@ -package sp.bvantur.nestednavigation.ui.screens - -import androidx.compose.foundation.background -import androidx.compose.foundation.clickable -import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.height -import androidx.compose.foundation.layout.padding -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.outlined.Menu -import androidx.compose.material.rememberScaffoldState -import androidx.compose.material3.ExperimentalMaterial3Api -import androidx.compose.material3.Icon -import androidx.compose.material3.IconButton -import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.Text -import androidx.compose.runtime.Composable -import androidx.compose.runtime.rememberCoroutineScope -import androidx.compose.ui.Alignment -import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.Color -import androidx.compose.ui.unit.dp -import androidx.navigation.NavHostController -import com.ramcosta.composedestinations.DestinationsNavHost -import com.ramcosta.composedestinations.NavGraphs -import com.ramcosta.composedestinations.ProfileScreenDestination -import com.ramcosta.composedestinations.annotation.Destination -import com.ramcosta.composedestinations.navigation.DestinationsNavigator -import com.ramcosta.composedestinations.rememberDestinationsNavController -import kotlinx.coroutines.launch -import sp.bvantur.nestednavigation.MainActivity - -@Destination -@OptIn(ExperimentalMaterial3Api::class) -@Composable -fun HomeScreen(navigator: DestinationsNavigator) { - MainActivity.homeScreenNavigator = navigator - val scaffoldState = rememberScaffoldState() - val scope = rememberCoroutineScope() - val navController: NavHostController = rememberDestinationsNavController() - - DestinationsScaffold( - scaffoldState = scaffoldState, - navController = navController, - drawerContent = { - DrawerRow("Profile", true) { - scope.launch { scaffoldState.drawerState.close() } - } - }, - topBar = { - AppBar { - scope.launch { scaffoldState.drawerState.open() } - } - }, - ) { - DestinationsNavHost( - navController = navController, - startDestination = ProfileScreenDestination - ) - } -} - -@Composable -fun DrawerRow(title: String, selected: Boolean, onClick: () -> Unit) { - val background = - if (selected) MaterialTheme.colorScheme.primary.copy(alpha = 0.12f) else Color.Transparent - val textColor = if (selected) MaterialTheme.colorScheme.primary else MaterialTheme.colorScheme.onSurface - - Text( - color = textColor, - text = title, - modifier = Modifier - .clickable(onClick = onClick) - .background(background) - .fillMaxWidth() - .padding(16.dp) - ) -} - -@Composable -private fun AppBar( - onDrawerClick: () -> Unit -) { - Box( - modifier = Modifier - .fillMaxWidth() - .height(60.dp) - .background(MaterialTheme.colorScheme.primary) - ) { - IconButton( - onClick = onDrawerClick, - modifier = Modifier.align(Alignment.CenterStart) - ) { - Icon( - imageVector = Icons.Outlined.Menu, - tint = Color.White, - contentDescription = "menu" - ) - } - Text( - text = "TEST", - modifier = Modifier.align(Alignment.Center), - color = Color.White - ) - } -} \ No newline at end of file Index: app/src/main/java/sp/bvantur/nestednavigation/ui/screens/SplashScreen.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/SplashScreen.kt b/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/SplashScreen.kt --- a/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/SplashScreen.kt (revision 7c9873dbdcb218a0aed93fb27106ca834a8fa10e) +++ b/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/SplashScreen.kt (date 1638379761588) @@ -12,8 +12,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.text.TextStyle -import androidx.navigation.NavHostController -import com.ramcosta.composedestinations.HomeScreenDestination +import com.ramcosta.composedestinations.NavGraphs import com.ramcosta.composedestinations.annotation.Destination import com.ramcosta.composedestinations.navigation.DestinationsNavigator @@ -26,7 +25,7 @@ Column(Modifier.fillMaxSize(), verticalArrangement = Arrangement.Center, horizontalAlignment = Alignment.CenterHorizontally) { Text("Splash screen", style = TextStyle(color = Color.White)) Button(onClick = { - navigator.navigate(HomeScreenDestination) + navigator.navigate(NavGraphs.home) }) { Text("To home screen") } Index: app/src/main/java/sp/bvantur/nestednavigation/ui/screens/ProfileScreen.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/ProfileScreen.kt b/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/ProfileScreen.kt --- a/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/ProfileScreen.kt (revision 7c9873dbdcb218a0aed93fb27106ca834a8fa10e) +++ b/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/ProfileScreen.kt (date 1638379716652) @@ -12,15 +12,12 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.unit.dp -import androidx.navigation.NavHostController import com.ramcosta.composedestinations.ExternalDetailsScreenDestination import com.ramcosta.composedestinations.InternalDetailsScreenDestination import com.ramcosta.composedestinations.annotation.Destination import com.ramcosta.composedestinations.navigation.DestinationsNavigator -import sp.bvantur.nestednavigation.MainActivity -//@Destination(navGraph = "profile", start = true) -@Destination +@Destination(navGraph = "home", start = true) @Composable fun ProfileScreen(navigator: DestinationsNavigator) { Box(modifier = Modifier @@ -28,8 +25,7 @@ .background(Color.Red)) { Column { Button(onClick = { - MainActivity.homeScreenNavigator?.navigate(ExternalDetailsScreenDestination) -// navigator.navigate(ExternalDetailsScreenDestination) + navigator.navigate(ExternalDetailsScreenDestination) }) { Text(text = "External details screen") } Index: app/src/main/java/sp/bvantur/nestednavigation/ui/screens/InternalDetailsScreen.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/InternalDetailsScreen.kt b/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/InternalDetailsScreen.kt --- a/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/InternalDetailsScreen.kt (revision 7c9873dbdcb218a0aed93fb27106ca834a8fa10e) +++ b/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/InternalDetailsScreen.kt (date 1638379716655) @@ -3,13 +3,22 @@ import androidx.compose.foundation.background import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import com.ramcosta.composedestinations.annotation.Destination -@Destination(navGraph = "profile", start = true) +@Destination(navGraph = "home") @Composable fun InternalDetailsScreen() { - Box(modifier = Modifier.fillMaxSize().background(Color.Red)) + Box( + modifier = Modifier + .fillMaxSize() + .background(Color.Red), + contentAlignment = Alignment.Center + ) { + Text("InternalDetailsScreen") + } } \ No newline at end of file Index: app/src/main/java/sp/bvantur/nestednavigation/ui/screens/ExternalDetailsScreen.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/ExternalDetailsScreen.kt b/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/ExternalDetailsScreen.kt --- a/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/ExternalDetailsScreen.kt (revision 7c9873dbdcb218a0aed93fb27106ca834a8fa10e) +++ b/app/src/main/java/sp/bvantur/nestednavigation/ui/screens/ExternalDetailsScreen.kt (date 1638379716649) @@ -3,7 +3,9 @@ import androidx.compose.foundation.background import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import com.ramcosta.composedestinations.annotation.Destination @@ -11,5 +13,12 @@ @Destination @Composable fun ExternalDetailsScreen() { - Box(modifier = Modifier.fillMaxSize().background(Color.Blue)) + Box( + modifier = Modifier + .fillMaxSize() + .background(Color.Blue), + contentAlignment = Alignment.Center + ) { + Text("ExternalDetailsScreen") + } } \ No newline at end of file Index: app/src/main/java/sp/bvantur/nestednavigation/MainActivity.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/app/src/main/java/sp/bvantur/nestednavigation/MainActivity.kt b/app/src/main/java/sp/bvantur/nestednavigation/MainActivity.kt --- a/app/src/main/java/sp/bvantur/nestednavigation/MainActivity.kt (revision 7c9873dbdcb218a0aed93fb27106ca834a8fa10e) +++ b/app/src/main/java/sp/bvantur/nestednavigation/MainActivity.kt (date 1638379761585) @@ -3,62 +3,114 @@ import android.os.Bundle import androidx.activity.ComponentActivity import androidx.activity.compose.setContent -import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.background +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.outlined.Menu +import androidx.compose.material.rememberScaffoldState +import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.Surface import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.tooling.preview.Preview -import androidx.navigation.NavHostController -import androidx.navigation.compose.NavHost -import androidx.navigation.compose.composable -import androidx.navigation.compose.rememberNavController +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.unit.dp +import com.ramcosta.composedestinations.Destination import com.ramcosta.composedestinations.DestinationsNavHost import com.ramcosta.composedestinations.NavGraphs -import com.ramcosta.composedestinations.navigation.DestinationsNavigator +import com.ramcosta.composedestinations.allDestinations import com.ramcosta.composedestinations.rememberDestinationsNavController -import sp.bvantur.nestednavigation.ui.screens.ExternalDetailsScreen -import sp.bvantur.nestednavigation.ui.screens.HomeScreen -import sp.bvantur.nestednavigation.ui.screens.SplashScreen +import kotlinx.coroutines.launch +import sp.bvantur.nestednavigation.ui.screens.DestinationsScaffold import sp.bvantur.nestednavigation.ui.theme.NestedNavigationTheme class MainActivity : ComponentActivity() { - companion object { - var homeScreenNavigator: DestinationsNavigator? = null - } + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) + setContent { + val scaffoldState = rememberScaffoldState() + val scope = rememberCoroutineScope() val navController = rememberDestinationsNavController() NestedNavigationTheme { - // A surface container using the 'background' color from the theme - Surface( - modifier = Modifier.fillMaxSize(), - color = MaterialTheme.colorScheme.background - ) { - DestinationsNavHost( - navController = navController, - startDestination = NavGraphs.root.startDestination - ) + DestinationsScaffold( + scaffoldState = scaffoldState, + navController = navController, + drawerContent = { dest: Destination -> + if (NavGraphs.home.allDestinations.contains(dest)) { + DrawerRow("Profile", true) { + scope.launch { scaffoldState.drawerState.close() } + } + } + }, + topBar = { dest: Destination -> + if (NavGraphs.home.allDestinations.contains(dest)) { + AppBar { + scope.launch { scaffoldState.drawerState.open() } + } + } + }, + ) { + DestinationsNavHost(navController = navController) } } } } } + @Composable -fun Greeting(name: String) { - Text(text = "Hello $name!") +private fun DrawerRow(title: String, selected: Boolean, onClick: () -> Unit) { + val background = + if (selected) MaterialTheme.colorScheme.primary.copy(alpha = 0.12f) else Color.Transparent + val textColor = if (selected) MaterialTheme.colorScheme.primary else MaterialTheme.colorScheme.onSurface + + Text( + color = textColor, + text = title, + modifier = Modifier + .clickable(onClick = onClick) + .background(background) + .fillMaxWidth() + .padding(16.dp) + ) } -@Preview(showBackground = true) @Composable -fun DefaultPreview() { - NestedNavigationTheme { - Greeting("Android") +private fun AppBar( + onDrawerClick: () -> Unit +) { + Box( + modifier = Modifier + .fillMaxWidth() + .height(60.dp) + .background(MaterialTheme.colorScheme.primary) + ) { + IconButton( + onClick = onDrawerClick, + modifier = Modifier.align(Alignment.CenterStart) + ) { + Icon( + imageVector = Icons.Outlined.Menu, + tint = Color.White, + contentDescription = "menu" + ) + } + Text( + text = "TEST", + modifier = Modifier.align(Alignment.Center), + color = Color.White + ) } } \ No newline at end of file ```
BVantur commented 2 years ago

Thank you very much @raamcosta ;) We will use your proposed solution for our use case. I didn't know that there should be only 1 NavHost for the whole app. I am used to using more NavHosts in the old Navigation component, so I thought I can do this with Compose as well. To be honest, I am not a fan of this kind of solution that hides or shows something at the top of the whole UI hierarchy, depending on what is present somewhere down in the tree. But is working for us, so thank you! ;) Is not a hard solution to implement and to understand. You can go ahead and close the issue if you like. πŸ™ŒπŸ˜Š

raamcosta commented 2 years ago

Glad that helped πŸ™‚

I guess is one of those things that are much more ok to do with compose, since it's all about reacting to stuff. The next version of the library is going to change some things and add some features. I have been doing major internal refactor and such. I noticed you had nested NavHosts and in that sample app and it seemed to work fine. To be honest there is a clear lack of documentation around some things in compose navigation. That is why I am working on it being possible to have multiple NavHost calls in my library. Even though I didn't think it would be correct, now I don't know anymore and I prefer to leave that decision up to the developers.

With this new version it will be easier to import dependencies, you will be able to pass Parcelable, enums and Serializable as navigation arguments and more. But some APIs will change a bit. Stay tuned for that and let me know if you have any issue migrating. Also I will be having two versions one with beta dependencies (beta compose, beta accompanist) and one for stable dependencies.

If there is anything else I can help you with, don't hesitate to reach out. Even with a Twitter DM for quick questions πŸ™‚

rubinbasha commented 2 years ago

@raamcosta we are currently in the POC phase of our new project and really do like your lib and would like to use it as it does remove so much boilerplate and makes the code so much more readable. We did implement Drawer and bottom navigation at the top level of our composable hierarchy, but that is starting to feel a bit hacky, we would really love it to have the drawer and bottom navi only where needed as subgraphs, so to have a separation of concerns. Would you share with us please when would you expect to offer this possibility with your lib?

raamcosta commented 2 years ago

Hi @rubinbasha πŸ‘‹

Very soon. I have been working on the next release and it is pretty much ready. The only thing holding me back from releasing it today is that I would like to update all the docs in the wikis before I do that.

Even so, I'm expecting to release it before the end of this week. If you want to try the new version out even if there will be some documentation lacking, I can maybe release it and simply don't advertise the new version on GitHub. If that would help, do let me know.

rubinbasha commented 2 years ago

@raamcosta that is amazing news already, if it is a matter of days or just a week or so , then I would by no means want to rush you. I deeply thank you for the amazing job. Honestly this lib was the first thing I searched for when I wanted to enter the @Compose world πŸ˜„

BVantur commented 2 years ago

Hey @raamcosta 😁

If you will add support for multiple NavHost calls, that would be awesome. ;) In our sample app, we are getting really complex MainActivity with handling AppBar and additionally handling BottomBar for some screens in it. Another thing that we are currently having trouble with because of a lack of support for multiple NavHosts is with drawing under the system. Sometimes we show BottomBar/AppBar and when we don't show it, we currently need to handle inset padding separately because contentPadding from Scaffold doesn't always return expected content padding. Do you have any timeline on when you will publish those changes?

raamcosta commented 2 years ago

Hi @BVantur !

Yeah, as I told @rubinbasha I do have a version ready to be released with that and other changes and features. I'll call it 1.0-beta cause I feel this version deserves to be the 1.0 version as it changes quite a few things πŸ™‚ The only thing I am doing now is updating the wikis.

But you know what, I have used your repository to test the new version, so, what I can do is I can release this new version to maven central, and make a PR in your repository with my changes, so you can see how things work and you can start working immediately.

Later when I finish the documentation I can update the Readme and make the release official.

I will do this later today after work πŸ˜‰ I'll reply here to let you know.

BVantur commented 2 years ago

Ohh I see now. I didn't refresh the page and didn't see new comments added here. Sorry for bothering you with more or less the same question. But great news that you have already made those changes. Can't wait to try out your new version of the library. Looking forward to it!! πŸ˜‰πŸ˜

raamcosta commented 2 years ago

@BVantur

There you go: https://github.com/BVantur/ComposeNestedGraph/pull/1

Any questions about the changes, let me know :)

Although the version is not "public" in the sense that I still want to update wikis before that, you can totally use it :) (btw I forgot to remove the dependency in the PR, but you don't need to import "androidx.navigation:navigation-compose" as it is now a transitive dependency of my library)

BVantur commented 2 years ago

@raamcosta Thanks a lot. We've checked your changes and we have a couple of problems on which we couldn't find the solution on how to solve.

We've created a new branch, where we adapt your changes with the new version of the library. I also added TODO comments in the code on where we had troubles. Here you can check the changes we made.

(This part is tagged in our code with number 1# next to TODO comments) The main problem we are currently having is how to use DestinationsNavigator for our nested NavHost in combination with Drawer items. So we went a step further with the code that was provided to us by you through that Pull request. We said that we want to have more items in the Drawer menu and we want to switch content in nested NavHost, by selecting items from the Drawer menu. The first thing that we noticed was that we don't have DestinationsNavigator from nested NavHost available, so for this purpose, we've created a lateinit variable called childNavigator in our HomeScreen composable function. This childNavigator variable gets filled after we set up DestinationNavHost composable for ProfileScreen. After that, we use that childNavigator to do the navigation for our drawer items, but we've got some problems with this approach. Navigation for us happens only for the first time. After that, the navigation doesn't work anymore on any of the items from the Drawer menu. But if we navigate back with the system back button, for example, we can do another navigation from the Drawer menu. We've played around to try to make it work, but we didn't find a solution for this. Can you maybe help us with this use case? Are we doing something wrong in our code?

OUR DRAWER CONTENT:

Screenshot 2021-12-11 at 08 41 03

(This part is tagged in our code with number 2# next to TODO comments) Additionally to the above problem would be great if there would be the possibility to send some optional arguments to the starting screen of a nested NavHost's screen. I am not sure how to do that with the current way of having that composable call for the start destination inside of nested NavHost.

raamcosta commented 2 years ago

Hi @BVantur !

Check this: https://github.com/BVantur/ComposeNestedGraph/pull/2

Let me know if there is any clarification or anything else I can help you guys with!

BVantur commented 2 years ago

Thank you very much. We didn't know that there is such an easy possibility to access the parent navigation controller. This looks really nice. We will keep you posted if we will need any clarification on anything else.