diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/mediaServers/BlossomBlobManagerScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/mediaServers/BlossomBlobManagerScreen.kt index cb59cc7a72..7be384edf5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/mediaServers/BlossomBlobManagerScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/mediaServers/BlossomBlobManagerScreen.kt @@ -136,6 +136,7 @@ import com.vitorpamplona.amethyst.commons.ui.stringRes import com.vitorpamplona.amethyst.commons.ui.theme.allGoodColor import com.vitorpamplona.amethyst.commons.ui.theme.grayText import com.vitorpamplona.amethyst.service.playback.composable.VideoViewInner +import com.vitorpamplona.amethyst.ui.navigation.bottombars.AppBottomBar import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip56Reports.ReportType @@ -221,6 +222,11 @@ fun BlossomBlobManagerScreen( }, ) }, + bottomBar = { + AppBottomBar(Route.ManageBlossomBlobs, nav, accountViewModel) { route -> + if (route != Route.ManageBlossomBlobs) nav.navBottomBar(route) + } + }, ) { padding -> Column( modifier = diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/NavigationEffects.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/NavigationEffects.kt index d7e44af2db..31ad12ee41 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/NavigationEffects.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/NavigationEffects.kt @@ -54,6 +54,14 @@ const val BOTTOM_NAV_ROOT_KEY = "bottomNavRoot" fun NavBackStackEntry.isBottomNavRoot(): Boolean = savedStateHandle.get(BOTTOM_NAV_ROOT_KEY) == true +// Per-entry hint stamped by Nav.navDrawer marking that the entry was opened +// from the navigation drawer. It sits on top of the stack like any push (back +// arrow, slide animation), but Nav.showsBottomBar keeps the bottom bar on it: +// a drawer destination is a top-level section, not a detail screen. +const val DRAWER_ROOT_KEY = "drawerRoot" + +fun NavBackStackEntry.isDrawerRoot(): Boolean = savedStateHandle.get(DRAWER_ROOT_KEY) == true + /** * The shell's current layout tier, mirrored for the transition specs below. Transition * lambdas run when a navigation starts — outside composition — so they can't read diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/AppBottomBar.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/AppBottomBar.kt index ec0ca2e514..efaf7c00ca 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/AppBottomBar.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/AppBottomBar.kt @@ -68,7 +68,7 @@ import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel /** Content height of the [AppBottomBar] (the 50.dp Column inside [RenderBottomMenu]), * exclusive of the system navigation-bar inset. Used by FAB callers that want to - * reserve the same vertical space when the bar hides itself on canPop entries. */ + * reserve the same vertical space when the bar hides itself on in-app pushes. */ val AppBottomBarHeight = 50.dp @Composable @@ -94,9 +94,9 @@ fun AppBottomBar( // permanently docked drawer (Expanded). if (LocalScreenLayout.current.isLargeScreen) return - // Hide the bar on entries that aren't a tab root (drawer or in-app - // pushes). Mirrors the back-arrow rule in canPop(). - if (nav.canPop()) return + // Hide the bar on in-app pushes. Tab roots, Home and screens opened from + // the drawer keep it, even though the drawer ones still show a back arrow. + if (!nav.showsBottomBar()) return val items by accountViewModel.account.settings.syncedSettings.navigation.bottomBarItems .collectAsStateWithLifecycle() diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/drawer/DrawerContent.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/drawer/DrawerContent.kt index be821834a5..91a4198d97 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/drawer/DrawerContent.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/drawer/DrawerContent.kt @@ -210,7 +210,7 @@ private fun DrawerContentBody( accountViewModel: AccountViewModel, ) { val onClickUser = { - nav.nav(routeFor(accountViewModel.userProfile())) + nav.navDrawer(routeFor(accountViewModel.userProfile())) nav.closeDrawer() } @@ -641,24 +641,32 @@ fun ListContent( } } -/** The Create section's rows — composer entry points, none of which is a catalog destination. */ +/** + * The Create section's rows — composer entry points, none of which is a catalog destination. They + * open as plain pushes rather than through [INav.navDrawer]: those screens host no bottom bar, and + * a drawer stamp would tell FabBottomBarPadding that one is showing. + */ @Composable private fun CreateRows(nav: INav) { - NavigationRow( + IconRow( title = Res.string.share_hls_video, icon = MaterialSymbols.SettingsInputAntenna, tint = MaterialTheme.colorScheme.onBackground, - nav = nav, - route = Route.NewHlsVideo, + onClick = { + nav.closeDrawer() + nav.nav(Route.NewHlsVideo) + }, ) if (isDebug) { - NavigationRow( + IconRow( title = Res.string.route_chess, icon = MaterialSymbols.ChessKnight, tint = MaterialTheme.colorScheme.onBackground, - nav = nav, - route = Route.Chess, + onClick = { + nav.closeDrawer() + nav.nav(Route.Chess) + }, ) } } @@ -742,7 +750,7 @@ private fun ScheduledPostsNavigationRow( badgeCount = pendingCount, onClick = { nav.closeDrawer() - nav.nav { def.resolveRoute(accountViewModel) } + nav.navDrawer { def.resolveRoute(accountViewModel) } }, ) } @@ -850,7 +858,7 @@ fun NavigationRow( tint, onClick = { nav.closeDrawer() - nav.nav(route) + nav.navDrawer(route) }, ) } @@ -871,7 +879,7 @@ fun NavigationRow( tint, onClick = { nav.closeDrawer() - nav.nav(computeRoute) + nav.navDrawer(computeRoute) }, ) } @@ -890,7 +898,7 @@ fun NavigationRow( tint = tint, onClick = { nav.closeDrawer() - nav.nav(route) + nav.navDrawer(route) }, ) } @@ -909,7 +917,7 @@ fun NavigationRow( tint = tint, onClick = { nav.closeDrawer() - nav.nav(computeRoute) + nav.navDrawer(computeRoute) }, ) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/navs/Nav.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/navs/Nav.kt index 0818cc8a42..41780eb4e3 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/navs/Nav.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/navs/Nav.kt @@ -36,7 +36,9 @@ import androidx.navigation.NavHostController import com.vitorpamplona.amethyst.commons.model.navigation.Route import com.vitorpamplona.amethyst.commons.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.navigation.BOTTOM_NAV_ROOT_KEY +import com.vitorpamplona.amethyst.ui.navigation.DRAWER_ROOT_KEY import com.vitorpamplona.amethyst.ui.navigation.isBottomNavRoot +import com.vitorpamplona.amethyst.ui.navigation.isDrawerRoot import com.vitorpamplona.amethyst.ui.navigation.routes.getRouteWithArguments import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -89,6 +91,31 @@ class Nav( } } + override fun navDrawer(route: Route) { + navigationScope.launch { + ime.settle() + navigateFromDrawer(route) + } + } + + override fun navDrawer(computeRoute: suspend () -> Route?) { + navigationScope.launch { + ime.settle() + computeRoute()?.let { navigateFromDrawer(it) } + } + } + + /** + * Same push as [nav], then stamps the new entry [DRAWER_ROOT_KEY] so [showsBottomBar] keeps + * the bottom bar on it. The stamp lands in the same frame as the navigate, before the entry + * composes, the way [navBottomBar] stamps its tab roots. + */ + private fun navigateFromDrawer(route: Route) { + if (getRouteWithArguments(route::class, controller) == route) return + controller.navigate(route) + controller.currentBackStackEntry?.savedStateHandle?.set(DRAWER_ROOT_KEY, true) + } + override fun newStack(route: Route) { navigationScope.launch { ime.settle() @@ -107,9 +134,10 @@ class Nav( // A nav-bar tap asks for a tab, never for whatever the user pushed on top of one. Drop // those pushes first, and without saving them, so the restoreState below can never hand - // a deep stack back. On phones this is always a no-op — AppBottomBar hides itself off - // tab roots, so the bar is only ever tapped from one — but the large-screen rail stays - // on screen the whole time and is routinely tapped from three screens deep. + // a deep stack back. On phones the bar shows only on tab roots and on screens opened + // from the drawer (dropped here, back to the tab they were opened over), but the + // large-screen rail stays on screen the whole time and is routinely tapped from three + // screens deep. popPushesAboveTabRoot() // Dropping those pushes is often the whole job — re-tapping the tab the user is inside, @@ -211,11 +239,24 @@ class Nav( // Outside a NavHost destination (shell chrome, drawer) the current owner // is the account-scoped ViewModelStoreOwner, not an entry; fall back to // the globally-current entry so those callers keep their prior behavior. - val entry = - (LocalViewModelStoreOwner.current as? NavBackStackEntry) - ?: controller.currentBackStackEntry - ?: return false + val entry = ownEntry() ?: return false + return canPop(entry) + } + @Composable + override fun showsBottomBar(): Boolean { + val entry = ownEntry() ?: return true + // Drawer destinations can pop (they sit on whatever screen the drawer was opened over), + // but they are top-level sections, so they keep the bar the tab roots have. + return entry.isDrawerRoot() || !canPop(entry) + } + + @Composable + private fun ownEntry(): NavBackStackEntry? = + (LocalViewModelStoreOwner.current as? NavBackStackEntry) + ?: controller.currentBackStackEntry + + private fun canPop(entry: NavBackStackEntry): Boolean { // Hidden on tab roots (reached via the bottom nav) and on Home (the // graph's start destination): nothing sits below either that a back // arrow could return to. Every other entry is a push on top of Home, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/cordnGroup/CordnGroupListScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/cordnGroup/CordnGroupListScreen.kt index 13876ffbe1..d29ffb280a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/cordnGroup/CordnGroupListScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/cordnGroup/CordnGroupListScreen.kt @@ -64,6 +64,7 @@ import com.vitorpamplona.amethyst.commons.ui.navigation.navs.INav import com.vitorpamplona.amethyst.commons.ui.navigation.topbars.TopBarWithBackButton import com.vitorpamplona.amethyst.commons.ui.theme.DividerThickness import com.vitorpamplona.amethyst.model.cordn.CordnRuntime +import com.vitorpamplona.amethyst.ui.navigation.bottombars.AppBottomBar import com.vitorpamplona.amethyst.ui.pluralStringRes import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.rooms.CordnGroupRoomCompose @@ -125,6 +126,11 @@ fun CordnGroupListScreen( } } }, + bottomBar = { + AppBottomBar(Route.CordnGroupList, nav, accountViewModel) { route -> + if (route != Route.CordnGroupList) nav.navBottomBar(route) + } + }, ) { padding -> if (runtime == null) { EmptyState( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt index d94e18af51..ec72529de9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt @@ -91,6 +91,7 @@ import com.vitorpamplona.amethyst.commons.ui.pluralStringRes import com.vitorpamplona.amethyst.commons.ui.stringRes import com.vitorpamplona.amethyst.commons.ui.theme.Size55dp import com.vitorpamplona.amethyst.service.relayClient.reqCommand.user.observeUserInfo +import com.vitorpamplona.amethyst.ui.navigation.bottombars.AppBottomBar import com.vitorpamplona.amethyst.ui.note.NonClickableUserPictures import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.types.hasEncryptedMediaV2 @@ -136,16 +137,24 @@ fun MarmotGroupListScreen( topBar = { TopAppBar( navigationIcon = { - IconButton(onClick = { nav.popBack() }) { - Icon( - symbol = MaterialSymbols.AutoMirrored.ArrowBack, - contentDescription = stringRes(Res.string.back), - ) + // No arrow as a bottom-nav tab root: there is nothing below it to return to. + if (nav.canPop()) { + IconButton(onClick = { nav.popBack() }) { + Icon( + symbol = MaterialSymbols.AutoMirrored.ArrowBack, + contentDescription = stringRes(Res.string.back), + ) + } } }, title = { Text(stringRes(Res.string.marmot_groups_title)) }, ) }, + bottomBar = { + AppBottomBar(Route.MarmotGroupList, nav, accountViewModel) { route -> + if (route != Route.MarmotGroupList) nav.navBottomBar(route) + } + }, floatingActionButton = { FabBottomBarPadded(nav) { FloatingActionButton(onClick = { nav.nav(Route.CreateMarmotGroup) }, shape = CircleShape) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordChannelListScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordChannelListScreen.kt index d95788cda4..8e12603fd3 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordChannelListScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordChannelListScreen.kt @@ -353,7 +353,7 @@ fun ConcordChannelListScreen( ) }, bottomBar = { - // Renders only when this is a bottom-nav root (AppBottomBar hides itself when canPop), + // Hidden on in-app pushes (AppBottomBar renders only when nav.showsBottomBar()), // so a pinned Concord community works both as a pushed detail and as a bottom-nav tab. AppBottomBar(Route.ConcordServer(communityId), nav, accountViewModel) { route -> if (route != Route.ConcordServer(communityId)) nav.navBottomBar(route) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordHomeScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordHomeScreen.kt index 95a86e69f7..5852cb0061 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordHomeScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordHomeScreen.kt @@ -137,7 +137,7 @@ fun ConcordHomeScreen( ) }, bottomBar = { - // Renders only when this is a bottom-nav root (AppBottomBar hides itself when canPop), + // Hidden on in-app pushes (AppBottomBar renders only when nav.showsBottomBar()), // so the same screen works both as a pushed destination and a bottom-nav tab. AppBottomBar(Route.Concords, nav, accountViewModel) { route -> if (route != Route.Concords) nav.navBottomBar(route) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/nip28PublicChat/PublicChatChannelScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/nip28PublicChat/PublicChatChannelScreen.kt index da19432246..10e544030b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/nip28PublicChat/PublicChatChannelScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/nip28PublicChat/PublicChatChannelScreen.kt @@ -56,7 +56,7 @@ fun PublicChatChannelScreen( PublicChatTopBar(it, accountViewModel, nav) } }, - // Renders only when this is a bottom-nav root (AppBottomBar hides itself when canPop), + // Hidden on in-app pushes (AppBottomBar renders only when nav.showsBottomBar()), // so a pinned public chat works both as a pushed detail and as a bottom-nav tab. bottomBar = { AppBottomBar(selfRoute, nav, accountViewModel) { route -> diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChannelListScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChannelListScreen.kt index 30efeb7f3b..0bd74d7ba7 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChannelListScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChannelListScreen.kt @@ -439,7 +439,7 @@ fun RelayGroupChannelListScreen( ) }, bottomBar = { - // Renders only when this is a bottom-nav root (AppBottomBar hides itself when canPop), + // Hidden on in-app pushes (AppBottomBar renders only when nav.showsBottomBar()), // so a pinned NIP-29 relay works both as a pushed detail and as a bottom-nav tab. AppBottomBar(selfRoute, nav, accountViewModel) { route -> if (route != selfRoute) nav.navBottomBar(route) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChatScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChatScreen.kt index d6b6f376a1..e938259b76 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChatScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChatScreen.kt @@ -58,7 +58,7 @@ fun RelayGroupChatScreen( RelayGroupTopBar(it, inviteCode, accountViewModel, nav) } }, - // Renders only when this is a bottom-nav root (AppBottomBar hides itself when canPop), + // Hidden on in-app pushes (AppBottomBar renders only when nav.showsBottomBar()), // so a pinned relay group works both as a pushed detail and as a bottom-nav tab. bottomBar = { AppBottomBar(selfRoute, nav, accountViewModel) { route -> diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/geocaches/GeocachesScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/geocaches/GeocachesScreen.kt index 6a271c49e2..fb41b9f2e1 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/geocaches/GeocachesScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/geocaches/GeocachesScreen.kt @@ -107,8 +107,12 @@ fun GeocachesScreen( } }, bottomBar = { - AppBottomBar(Route.Geocaches(), nav, accountViewModel) { route -> - if (route is Route.Geocaches) { + // Geocaches and Hunts are two separate pinnable tabs of this one screen, so match the + // exact route: a class check highlighted the wrong one and turned a tap on the other + // into a scroll-to-top instead of a tab switch. + val selfRoute = Route.Geocaches(initialTab) + AppBottomBar(selfRoute, nav, accountViewModel) { route -> + if (route == selfRoute) { nearby.sendToTop() } else { nav.navBottomBar(route) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/napplets/NappletsScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/napplets/NappletsScreen.kt index 385ed65e3e..bea2b1ae8f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/napplets/NappletsScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/napplets/NappletsScreen.kt @@ -37,11 +37,13 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.vitorpamplona.amethyst.Amethyst +import com.vitorpamplona.amethyst.commons.model.navigation.Route import com.vitorpamplona.amethyst.commons.resources.Res import com.vitorpamplona.amethyst.commons.resources.napplet_none_found import com.vitorpamplona.amethyst.commons.ui.navigation.navs.INav import com.vitorpamplona.amethyst.commons.ui.stringRes import com.vitorpamplona.amethyst.napplet.NappletLauncher +import com.vitorpamplona.amethyst.ui.navigation.bottombars.AppBottomBar import com.vitorpamplona.amethyst.ui.note.NoteCompose import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.screen.loggedIn.napplets.datasource.NappletsFilterAssemblerSubscription @@ -90,6 +92,11 @@ fun NappletsScreen( Scaffold( topBar = { NappletsTopBar(accountViewModel, nav) }, + bottomBar = { + AppBottomBar(Route.Napplets, nav, accountViewModel) { route -> + if (route != Route.Napplets) nav.navBottomBar(route) + } + }, ) { padding -> if (visible.isEmpty()) { Box(Modifier.fillMaxSize().padding(padding), contentAlignment = Alignment.Center) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nsites/NsitesScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nsites/NsitesScreen.kt index 4bee779bf0..5ef15b0073 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nsites/NsitesScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nsites/NsitesScreen.kt @@ -37,10 +37,12 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.vitorpamplona.amethyst.Amethyst +import com.vitorpamplona.amethyst.commons.model.navigation.Route import com.vitorpamplona.amethyst.commons.resources.Res import com.vitorpamplona.amethyst.commons.resources.nsite_none_found import com.vitorpamplona.amethyst.commons.ui.navigation.navs.INav import com.vitorpamplona.amethyst.commons.ui.stringRes +import com.vitorpamplona.amethyst.ui.navigation.bottombars.AppBottomBar import com.vitorpamplona.amethyst.ui.note.NoteCompose import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.screen.loggedIn.nsites.datasource.NsitesFilterAssemblerSubscription @@ -88,6 +90,11 @@ fun NsitesScreen( Scaffold( topBar = { NsitesTopBar(accountViewModel, nav) }, + bottomBar = { + AppBottomBar(Route.Nsites, nav, accountViewModel) { route -> + if (route != Route.Nsites) nav.navBottomBar(route) + } + }, ) { padding -> if (visible.isEmpty()) { Box(Modifier.fillMaxSize().padding(padding), contentAlignment = Alignment.Center) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/nip46/Nip46SignerScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/nip46/Nip46SignerScreen.kt index df18f48a12..569c6c6ab9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/nip46/Nip46SignerScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/nip46/Nip46SignerScreen.kt @@ -125,6 +125,7 @@ import com.vitorpamplona.amethyst.commons.ui.navigation.topbars.TopBarWithBackBu import com.vitorpamplona.amethyst.commons.ui.pluralStringRes import com.vitorpamplona.amethyst.commons.ui.stringRes import com.vitorpamplona.amethyst.model.nip46Signer.Nip46SignerState +import com.vitorpamplona.amethyst.ui.navigation.bottombars.AppBottomBar import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.screen.loggedIn.qrcode.QrCodeDrawer import com.vitorpamplona.amethyst.ui.screen.loggedIn.qrcode.SimpleQrCodeScanner @@ -193,6 +194,11 @@ fun Nip46SignerScreen( Scaffold( topBar = { TopBarWithBackButton(stringRes(Res.string.nip46_signer_title), nav) }, + bottomBar = { + AppBottomBar(Route.Nip46Signer(), nav, accountViewModel) { route -> + if (route != Route.Nip46Signer()) nav.navBottomBar(route) + } + }, ) { padding -> Column( modifier = diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/navigation/NavBottomBarStackTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/navigation/NavBottomBarStackTest.kt index edc44460a9..cac7218dfd 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/navigation/NavBottomBarStackTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/navigation/NavBottomBarStackTest.kt @@ -42,8 +42,8 @@ import org.junit.Test /** * A nav-bar tap must land on the tab itself, never on whatever the user had pushed on top of one. * - * The phone bottom bar gets this for free — it hides itself off tab roots, so it can only ever be - * tapped from one. The large-screen navigation rail stays on screen the whole time, which is where + * The phone bottom bar shows only on tab roots and drawer destinations, so it is tapped from at most + * one screen above a tab. The large-screen navigation rail stays on screen the whole time, which is where * the gap showed: tapping Home from a thread three screens deep came back to that thread instead of * the feed. * diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/navigation/NavDrawerTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/navigation/NavDrawerTest.kt new file mode 100644 index 0000000000..580b1f63b6 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/navigation/NavDrawerTest.kt @@ -0,0 +1,118 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.ui.navigation + +import androidx.navigation.NavBackStackEntry +import androidx.navigation.NavHostController +import com.vitorpamplona.amethyst.commons.model.navigation.Route +import com.vitorpamplona.amethyst.ui.navigation.navs.Nav +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.Test + +/** + * A screen opened from the navigation drawer is a top-level section: it keeps the bottom bar + * (issue #4141, where Pictures and every other drawer destination lost it). It still sits on top of + * the screen the drawer was opened over, so it keeps its back arrow too; the bar is told apart + * from an in-app push by the [DRAWER_ROOT_KEY] stamp these tests pin down. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class NavDrawerTest { + private fun entry( + isTabRoot: Boolean = false, + isDrawerRoot: Boolean = false, + ): NavBackStackEntry = + mockk(relaxed = true) { + every { savedStateHandle.get(BOTTOM_NAV_ROOT_KEY) } returns isTabRoot + every { savedStateHandle.get(DRAWER_ROOT_KEY) } returns isDrawerRoot + } + + /** A controller whose current entry becomes [pushed] once [route] is navigated to. */ + private fun controllerPushing( + route: Route, + pushed: NavBackStackEntry, + ): NavHostController { + var current = entry(isTabRoot = true) + return mockk(relaxed = true) { + every { currentBackStackEntry } answers { current } + every { navigate(route) } answers { current = pushed } + } + } + + @Test + fun stampsTheEntryItOpens() = + runTest { + val pushed = entry() + val controller = controllerPushing(Route.Pictures(), pushed) + + Nav(controller, this).navDrawer(Route.Pictures()) + advanceUntilIdle() + + verify(exactly = 1) { controller.navigate(Route.Pictures()) } + verify(exactly = 1) { pushed.savedStateHandle[DRAWER_ROOT_KEY] = true } + } + + @Test + fun stampsTheEntryOfAResolvedRoute() = + runTest { + val pushed = entry() + val controller = controllerPushing(Route.Articles, pushed) + + Nav(controller, this).navDrawer { Route.Articles } + advanceUntilIdle() + + verify(exactly = 1) { pushed.savedStateHandle[DRAWER_ROOT_KEY] = true } + } + + @Test + fun plainPushesAreNotStamped() = + runTest { + val pushed = entry() + val controller = controllerPushing(Route.Pictures(), pushed) + + Nav(controller, this).nav(Route.Pictures()) + advanceUntilIdle() + + verify(exactly = 0) { pushed.savedStateHandle[DRAWER_ROOT_KEY] = any() } + } + + @Test + fun aTabTappedFromADrawerScreenDropsItFirst() = + runTest { + // Home > Pictures (from the drawer): the bar now shows on Pictures, so it can be tapped + // from there. The drawer entry is not a tab root and must not be saved into the tab. + val controller = + mockk(relaxed = true) { + every { currentBackStackEntry } returnsMany + listOf(entry(isDrawerRoot = true), entry(isTabRoot = true)) + every { popBackStack() } returns true + } + + Nav(controller, this).navBottomBar(Route.Message) + advanceUntilIdle() + + verify(exactly = 1) { controller.popBackStack() } + } +} diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/layouts/DisappearingScaffold.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/layouts/DisappearingScaffold.kt index 972ba2c8b0..7db01dd845 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/layouts/DisappearingScaffold.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/layouts/DisappearingScaffold.kt @@ -199,7 +199,7 @@ private fun ScaffoldLayout( }.firstOrNull()?.measure(looseConstraints) } // When the bar lambda is provided but its content emits nothing (e.g. AppBottomBar - // hides itself on canPop entries, or while the keyboard is up), reserve the + // hides itself on in-app pushes, or while the keyboard is up), reserve the // system-nav-bar inset so the FAB and content stay clear of the navigation bar instead // of sliding under it. Subtract the IME inset: the root imePadding has already lifted the // whole scaffold above the keyboard, and the IME inset spans the nav-bar band, so diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/navigation/bottombars/FabBottomBarPadding.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/navigation/bottombars/FabBottomBarPadding.kt index 723231f179..592ee4cd94 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/navigation/bottombars/FabBottomBarPadding.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/navigation/bottombars/FabBottomBarPadding.kt @@ -31,7 +31,7 @@ import com.vitorpamplona.amethyst.commons.ui.navigation.navs.INav /** * Reserves the visual space the `AppBottomBar` occupies on root tab entries so a * FloatingActionButton stays at the same vertical position whether or not the bar is - * rendered. `AppBottomBar` hides itself on canPop entries (drawer pushes, in-app + * rendered. `AppBottomBar` hides itself off [INav.showsBottomBar] entries (in-app * navigations) — without this padding the FAB drops by `AppBottomBarHeight` there. * * The system-navigation-bar inset is already handled by the surrounding Scaffold, so @@ -41,8 +41,8 @@ val FABPaddingFromBottom = 30.dp @Composable fun Modifier.fabBottomBarPadding(nav: INav): Modifier = - if (nav.canPop() || LocalScreenLayout.current.isLargeScreen) { - // canPop entries hide the bar on phones; large screens never render it at all. + if (!nav.showsBottomBar() || LocalScreenLayout.current.isLargeScreen) { + // In-app pushes hide the bar on phones; large screens never render it at all. padding(bottom = FABPaddingFromBottom) } else { this diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/navigation/navs/INav.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/navigation/navs/INav.kt index 08261b7691..9a3a5d3261 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/navigation/navs/INav.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/navigation/navs/INav.kt @@ -52,9 +52,26 @@ interface INav { fun navBottomBar(route: Route) + /** + * Opens [route] as a top-level destination picked from the navigation drawer. It still + * stacks on top of the current screen (so it [canPop]), but unlike an in-app push it keeps + * the bottom bar — see [showsBottomBar]. + */ + fun navDrawer(route: Route) = nav(route) + + /** [navDrawer], for a route that has to be resolved first. */ + fun navDrawer(computeRoute: suspend () -> Route?) = nav(computeRoute) + @Composable fun canPop(): Boolean + /** + * Whether this screen renders the bottom bar: tab roots, the start destination and + * destinations opened from the drawer ([navDrawer]) do; in-app pushes don't. + */ + @Composable + fun showsBottomBar(): Boolean = !canPop() + fun popBack() fun popUpTo(