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

Implement hierarchy access for Destination #186

Open MaxMichel2 opened 2 years ago

MaxMichel2 commented 2 years ago

Hello,

I've been converting an existing app's navigation to use this library (60+ screens and 6+ nested nav graphs, should be a good test for the library 😉) and have found a nice-to-have that could be easily integrated in the existing system.

Currently, the NavDestination class contains a property called hierarchy which simply gives access to a list of destinations from the given NavDestination up to the root navigation graph. This is particularly useful when using a bottom navigation to identify if nested screens (belonging to a particular bottom navigation item) should display said navigation item as selected or not.

I cloned the repository for easier access and noticed that you use the following variable to achieve a similar result:

val isCurrentDestOnBackStack = navController.isRouteOnBackStack(destination.direction)

Lets assume we have a bottom bar with 3 nav graphs containing a certain number of screens: A, B, C

The issue I see with this (I could be wrong though) is that if a specific screen (belonging to nav graph B for example) can be navigated to from a set of screens in nav graph A, then both navigation item A and B would be on the back stack potentially, which would cause both to be visually selected. Another issue could be that said screen belonging to B, navigation item B should be selected but having navigated to it from A, B wouldn't be on the back stack and therefore wouldn't appear as selected (which could be the desired behaviour but it also could be considered an anomaly).

A solution I implemented is the following (taking into account that I cannot add a parent property to a Destination myself):

val Destination.hierarchy: Sequence<Destination>
    get() = generateSequence(this) { it.parent }

val Destination.parent: Destination?
    get() = NavGraphs.root.findParent(this)

fun NavGraph.findParent(destination: Destination): Destination? {
    val startDestination = this.startRoute as Destination
    return if (destination in this.destinations) {
        startDestination
    } else {
        this.nestedNavGraphs.find { navGraph ->
            navGraph.findParent(destination) != null
        }?.findParent(destination)
    }
}

// Usage as a selection variable would look like the following
/* */
selected = currentDestination.hierarchy.any { destination ->
    destination == screen.destination // screen.destination is the destination associated to a bottom navigation item
}
/* */

Feel free to let me know if this is an interesting idea or if the current behaviour is totally expected and isn't something that is likely to change!

Thanks again for all your work!

MaxMichel2 commented 2 years ago

Small note, the current findParent recursion is broken (cyclic recursion). I'm working on correcting this so I'll post a correction once I have it :)

raamcosta commented 2 years ago

Cool! If you re ok to wait, i can probably come up with a different way of doing this using the actual hierarchy from compose navigation (which you can also use and then just convert its entries to Destination as needed).

raamcosta commented 2 years ago

Or who knows, you can submit a PR 😁

MaxMichel2 commented 2 years ago

Hello, yes, a PR could be a good idea (I'll try to spend some time understanding everything going on first 😉)

As a temporary solution, I found that simply extracting the parent NavGraph for a DestinationSpec would be enough with the following function: (I was unable to find a way to

val DestinationSpec<*>.parent: NavGraph 
    get() {
        val parent =  NavGraphs.all.find {
            it.destinations.contains(this)
        }

        return parent ?: throw InvalidObjectException("The calling object should always have a parent!")
    }

Note that the NavGraphs.all is something I added to the codegen for the NavGraphs object (being an Object, it's a little complex to retrieve a list of it's variables so I added it in the codegen for simplicity's sake)

The major difference between your system and the Android system is that here, NavGraphSpec isn't a DirectionSpec and hence you can't recursively find a parent when starting from a DestinationSpec (since a DestinationSpec parent would be a NavGraphSpec)

I haven't spent much time looking through the structure to see if there's a way to link both together in a similar manner.

I'll continue playing around and try to implement something more robust

marosseleng commented 1 year ago

Hi, is there any progress regarding adding the access to the "hierarchy"?

RyuuyaS commented 1 year ago

Hello @raamcosta, I have seen in the pull request about this enhancement that you have another way for this. Can you update us on the information?