Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement hierarchy access for Destination #186

Open
MaxMichel2 opened this issue Jul 18, 2022 · 6 comments
Open

Implement hierarchy access for Destination #186

MaxMichel2 opened this issue Jul 18, 2022 · 6 comments
Labels
enhancement New feature or request

Comments

@MaxMichel2
Copy link

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!

@raamcosta raamcosta added the enhancement New feature or request label Jul 21, 2022
@MaxMichel2
Copy link
Author

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
Copy link
Owner

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
Copy link
Owner

Or who knows, you can submit a PR 馃榿

@MaxMichel2
Copy link
Author

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
Copy link

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

@RyuuyaS
Copy link

RyuuyaS commented Sep 28, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants