From af8fb44d466b0627d4dc5f9026280422f0862b51 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 17:01:32 +0000 Subject: [PATCH 1/2] fix(nav): keep the bottom bar on screens opened from the drawer Since the April change that hid AppBottomBar on every canPop() entry, any section picked from the navigation drawer (Pictures, Articles, Wallet, Settings, the own profile, ...) lost the bottom bar, because the drawer pushes those screens like any in-app navigation. Fixes #4141. The drawer now opens its destinations through INav.navDrawer, which stamps the new back-stack entry with DRAWER_ROOT_KEY, and the bar asks INav.showsBottomBar() instead of !canPop(): tab roots, Home and drawer destinations show it, in-app pushes (a profile or chat opened from a note, etc.) still don't. The back arrow is unchanged, since drawer screens can still pop. FabBottomBarPadding follows the same rule so FABs don't float 50dp above the bar that now shows. navBottomBar already drops any non-tab- root entry before switching tabs, so tapping a tab from a drawer screen lands on the tab without saving the drawer screen into it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018otSD34gHeE1RT31UgzY1b --- .../ui/navigation/NavigationEffects.kt | 8 ++ .../ui/navigation/bottombars/AppBottomBar.kt | 8 +- .../ui/navigation/drawer/DrawerContent.kt | 12 +- .../amethyst/ui/navigation/navs/Nav.kt | 55 ++++++-- .../concord/ConcordChannelListScreen.kt | 2 +- .../concord/ConcordHomeScreen.kt | 2 +- .../PublicChatChannelScreen.kt | 2 +- .../relayGroup/RelayGroupChannelListScreen.kt | 2 +- .../relayGroup/RelayGroupChatScreen.kt | 2 +- .../ui/navigation/NavBottomBarStackTest.kt | 4 +- .../amethyst/ui/navigation/NavDrawerTest.kt | 118 ++++++++++++++++++ .../ui/layouts/DisappearingScaffold.kt | 2 +- .../bottombars/FabBottomBarPadding.kt | 6 +- .../commons/ui/navigation/navs/INav.kt | 17 +++ 14 files changed, 212 insertions(+), 28 deletions(-) create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/ui/navigation/NavDrawerTest.kt 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..ca068e3aba 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() } @@ -742,7 +742,7 @@ private fun ScheduledPostsNavigationRow( badgeCount = pendingCount, onClick = { nav.closeDrawer() - nav.nav { def.resolveRoute(accountViewModel) } + nav.navDrawer { def.resolveRoute(accountViewModel) } }, ) } @@ -850,7 +850,7 @@ fun NavigationRow( tint, onClick = { nav.closeDrawer() - nav.nav(route) + nav.navDrawer(route) }, ) } @@ -871,7 +871,7 @@ fun NavigationRow( tint, onClick = { nav.closeDrawer() - nav.nav(computeRoute) + nav.navDrawer(computeRoute) }, ) } @@ -890,7 +890,7 @@ fun NavigationRow( tint = tint, onClick = { nav.closeDrawer() - nav.nav(route) + nav.navDrawer(route) }, ) } @@ -909,7 +909,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/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/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( From 348f294e2655570dde13a0e0ffe56e74d58474c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 18:20:59 +0000 Subject: [PATCH 2/2] fix(nav): bottom bar on every pinnable tab, and FAB padding on drawer screens Audit follow-up to the drawer bottom-bar fix (#4141): - Napplets, Nsites, Marmot Groups, Cordn Groups, the NIP-46 signer and My Blossom data are all pinnable bottom-bar tabs (BottomBarCategories) and drawer destinations, but their screens never rendered AppBottomBar: tapping one of them as a tab made the bar vanish, the same bug the issue reports. They now host the bar like every other tab screen. - That also fixes a regression from the previous commit: showsBottomBar() is true on drawer-opened entries, so FabBottomBarPadded dropped its padding on Marmot Groups, which had no bar to sit above. For the same reason the drawer's Create rows (HLS video, the debug Chess lobby) now open as plain pushes: they are composer/tool screens without a bar. - Marmot Groups showed its back arrow unconditionally, including as a tab root; it now follows canPop() like the other tab screens. - Geocaches and Geocache Hunts are two pinnable tabs of one screen, but the screen always selected Route.Geocaches() and treated any Geocaches route as a re-tap: the Hunts tab never highlighted and tapping one tab from the other scrolled to top instead of switching. It now matches its exact route. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018otSD34gHeE1RT31UgzY1b --- .../mediaServers/BlossomBlobManagerScreen.kt | 6 +++++ .../ui/navigation/drawer/DrawerContent.kt | 22 +++++++++++++------ .../chats/cordnGroup/CordnGroupListScreen.kt | 6 +++++ .../marmotGroup/MarmotGroupListScreen.kt | 19 +++++++++++----- .../loggedIn/geocaches/GeocachesScreen.kt | 8 +++++-- .../loggedIn/napplets/NappletsScreen.kt | 7 ++++++ .../ui/screen/loggedIn/nsites/NsitesScreen.kt | 7 ++++++ .../settings/nip46/Nip46SignerScreen.kt | 6 +++++ 8 files changed, 67 insertions(+), 14 deletions(-) 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/drawer/DrawerContent.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/drawer/DrawerContent.kt index ca068e3aba..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 @@ -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) + }, ) } } 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/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 =