AppBarConfiguration(Menu) constructor sets graph IDs, not graph start destination IDs, as top level destinations [238496771]

Fixed

Bug

Status Update

No update yet.

Description

st...@gmail.com

created issue #1

Jul 10, 2022 02:44PM

Component used: Navigation Version used: 2.4 Devices/Android versions reproduced on: all

AppCompatActivity::setupActionBarWithNavController can be called to automatically update app bar content when navigation destination changes. In particular, setupActionBarWithNavController promises to automatically hide the Up arrow for "top-level" destinations and show the Up arrow for all other destinations. To achieve this, setupActionBarWithNavController needs to infer or be told which destinations are "top-level".

When using a BottomNavigationView, I wanted

the root screen of each tab to have no Up arrow, and
all other screens to have an Up arrow.

I tried calling setupActionBarWithNavController and passing a custom AppBarConfiguration constructed using the Menu instance from the BottomNavigationView:

AppBarConfiguration.Builder(
  topLevelMenu = bottomNavigationViewMenu
).build()

Unfortunately, when passed a Menu whose entries correspond to navigation graph IDs, AppBarConfiguration::Builder sets the graph IDs as top-level destination IDs, rather than the setting the graph start destination IDs as top-level destination IDs. The result is that the AbstractAppBarOnDestinationChangedListener applied by setupActionBarWithNavController mistakenly classifies every destination as top-level and no screens have an Up arrow.

This can be worked around by explicitly passing the IDs of the start destinations of each tab's graph when constructing the AppBarConfiguration builder:

AppBarConfiguration.Builder(
  topLevelDestinationIds = setOf(
    tab1StartDestinationId,
    tab2StartDestinationId,
    tab3StartDestinationId,
  )
).build()

However, the behavior of AppBarConfiguration.Builder(Menu) remains surprising and it would be good to investigate whether that constructor could be made to behave as I anticipated!

Android Study Group Slack thread: https://androidstudygroup.slack.com/archives/CAZCL8AVB/p1656470310000929?thread_ts=1655997032.467509&cid=CAZCL8AVB

Comments

il...@google.com <il...@google.com> Jul 13, 2022 11:54PM

Assigned to jb...@google.com.

il...@google.com <il...@google.com> Sep 13, 2022 07:50PM

Reassigned to cl...@google.com.

ap...@google.com <ap...@google.com> #2Sep 23, 2022 03:56PM

Oh I forgot to mention that I thought it might be connected to this issue: https://issuetracker.google.com/issues/243778589. But after trying 2.5.2 I can say that the issue persists.

cl...@google.com <cl...@google.com> #3Sep 26, 2022 05:12PM

Marked as fixed.

Please provide a minimal sample project along with the minimal steps to recreate the issue in the project.

st...@gmail.com <st...@gmail.com> #4Sep 26, 2022 06:11PM

Sorry for the delay. I got a working example here: https://github.com/Del-S/issues. The project is set up to crash when navigating to ThirdFragment (nested nav graph) with poping the graph.

I poked into it a little bit and it seems to be connected to SafeArgs gradle plugin. When safe args is used to navigate to ThirdFragment (with nested graph poped) it will try to recreate te parent but there were no arguments passed so it crashes.

Steps to reproduce:

Navigate from Second Fragment to Child nav graph (with non-nullable parameters).
Navigate to Third fragment using SafeArgs and the app crashes.

Crash log:

Process: cz.dels.issues, PID: 1743
java.lang.NullPointerException: null cannot be cast to non-null type kotlin.Long
at androidx.navigation.NavType$Companion$LongType$1.get(NavType.kt:352)
at androidx.navigation.NavType$Companion$LongType$1.get(NavType.kt:342)
at androidx.navigation.NavArgument.verify(NavArgument.kt:76)
at androidx.navigation.NavDestination.addInDefaultArgs(NavDestination.kt:502)
at androidx.navigation.NavController.addEntryToBackStack(NavController.kt:1865)
at androidx.navigation.NavController.addEntryToBackStack$default(NavController.kt:1813)
at androidx.navigation.NavController$navigate$4.invoke(NavController.kt:1721)
at androidx.navigation.NavController$navigate$4.invoke(NavController.kt:1719)
at androidx.navigation.NavController$NavControllerNavigatorState.push(NavController.kt:287)
at androidx.navigation.fragment.FragmentNavigator.navigate(FragmentNavigator.kt:246)
at androidx.navigation.fragment.FragmentNavigator.navigate(FragmentNavigator.kt:162)
at androidx.navigation.NavController.navigateInternal(NavController.kt:260)
at androidx.navigation.NavController.navigate(NavController.kt:1719)
at androidx.navigation.NavController.navigate(NavController.kt:1545)
at androidx.navigation.NavController.navigate(NavController.kt:1472)
at androidx.navigation.NavController.navigate(NavController.kt:1930)
at cz.dels.issues.SecondFragment.onViewCreated$lambda-0(SecondFragment.kt:38)
at cz.dels.issues.SecondFragment.$r8$lambda$XDYnOS_cYrafiNQ5rcCu1WCn0IE(Unknown Source:0)
at cz.dels.issues.SecondFragment$$ExternalSyntheticLambda0.onClick(Unknown Source:2)

Note: If in step 2 SaveArgs is not used then navigation works correctly. More information is here: https://github.com/Del-S/issues/blob/master/app/src/main/java/cz/dels/issues/SecondFragment.kt#L37.

na...@google.com <na...@google.com> #5Oct 5, 2022 05:14PM

Ups a typo: Navigate from ~~Second~~ First Fragment to Child nav graph (with non-nullable parameters).

Note: sorry for the spam but I am not able to edit my own comment.

Issue 238496771