feat: give the app a navigation component, and stop the app bar duplicating it
Phase 6, step 3. The app had no navigation component of any kind: 43 screens reached by pushing a route, and one home screen whose top app bar carried the only two peer surfaces -- a profile avatar in the leading slot, a search icon in the trailing one. **This is an information-architecture change and was taken as one.** With a single top-level destination, a navigation bar would have held one item and been strictly worse than the app bar it replaced -- M3's caution is to swap only functionally equivalent components. Promoting search and profile to peer destinations is what makes a navigation component mean anything here, and it was put to the product owner rather than inferred. Answered: promote them. The consequence is in `HomeScreen`: the app bar now carries a title and nothing else. Two routes to one destination is the thing the caution is about, and the navigation component is now the one route, at every breakpoint. **Which component, at which breakpoint**, straight from the layout foundation: | compact | navigation bar | | medium, expanded | collapsed rail | | large, extra-large | expanded rail | `NavigationSuiteScaffoldDefaults.navigationSuiteType` is not used, and the difference is the last row -- it stops at the collapsed rail, because it classifies with the three-value window size class rather than the five breakpoints the May 2026 revision published. Deriving from `Breakpoint` reaches the row the library's default cannot, and keeps one source of truth for window width in the app. `NavigationSuiteType.None` on everything else. A navigation bar belongs on the destinations it switches between; on a chat room, a signing screen or an onboarding step -- pushed to and left by coming back -- it is a permanent invitation to lose your place. **Two things the wiring needed.** `ActiveProfileRoute` is addressed by metadata event id, not by public key, and only the home screen ever had one. The nav host now observes it for as long as a key is signed in, and the profile item is *disabled* until it arrives rather than absent -- an item that appears late moves the two beside it, and a bar whose items move under a thumb is worse than one briefly unavailable. The item click pops to `HomeRoute`, not to the graph's start destination. The android docs give the second shape and it would be wrong here: this graph starts at `LoadingRoute`, and onboarding clears the stack with `popUpTo(0)` on its way to home, so by the time these items exist the start destination is not on the stack at all -- popping to it would leave the loading screen underneath as the thing back returns to. **Tests, and one that could not be written.** The breakpoint-to-component table is a pure function so all five rows are asserted; the two rail rows differ only in whether labels are drawn, and nobody opens a 1200dp window on purpose. Four more compose the component around a real nav graph, because `TopLevelDestination.of` matches by `hasRoute` -- reflection over the serialized route -- and a renamed route would fail by never showing the component at all. Navigation in those is driven through the controller rather than by tapping an item. That is a harness limitation, established rather than assumed: a click handler that navigates trips navigation-compose's own main-thread assertion under `runDesktopComposeUiTest`, reproducible in twenty lines containing no app code -- a `NavHost`, two routes and a `TextButton`. What an item's `onClick` builds is asserted where it is a pure function instead. Also `material3-adaptive-navigation-suite`, versioned with material3 rather than with the adaptive library: it is published by the material3 group, and its 1.10.0-alpha05 is what names adaptive 1.2.0 in the first place. Most of the `MantraNavHost` diff is indentation -- the `NavHost` call gained an enclosing composable. `git diff -w` shows the 38 lines that are not. 626 jvm tests green; android compiles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kgothatso Ngako
@@ -2,19 +2,16 @@ package press.mantra.compose.ui.composable
|
||||
|
||||
import androidx.compose.foundation.layout.Arrangement
|
||||
import androidx.compose.foundation.layout.Column
|
||||
import androidx.compose.foundation.layout.Spacer
|
||||
import androidx.compose.foundation.layout.fillMaxSize
|
||||
import androidx.compose.foundation.layout.padding
|
||||
import androidx.compose.foundation.pager.HorizontalPager
|
||||
import androidx.compose.foundation.pager.rememberPagerState
|
||||
import androidx.compose.material.icons.Icons
|
||||
import androidx.compose.material.icons.filled.Add
|
||||
import androidx.compose.material.icons.filled.Search
|
||||
import androidx.compose.material3.ExperimentalMaterial3Api
|
||||
import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi
|
||||
import androidx.compose.material3.ExtendedFloatingActionButton
|
||||
import androidx.compose.material3.Icon
|
||||
import androidx.compose.material3.IconButton
|
||||
import androidx.compose.material3.MaterialTheme
|
||||
import androidx.compose.material3.PrimaryScrollableTabRow
|
||||
import androidx.compose.material3.Scaffold
|
||||
@@ -22,16 +19,14 @@ import androidx.compose.material3.Surface
|
||||
import androidx.compose.material3.Tab
|
||||
import androidx.compose.material3.Text
|
||||
import androidx.compose.material3.TopAppBar
|
||||
import androidx.compose.material3.TopAppBarDefaults
|
||||
import androidx.compose.material3.rememberModalBottomSheetState
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.key
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.rememberCoroutineScope
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.text.style.TextOverflow
|
||||
import androidx.compose.ui.tooling.preview.Preview
|
||||
@@ -42,12 +37,10 @@ import press.mantra.compose.database.model.Profile
|
||||
import press.mantra.compose.database.model.intermdiate.LocalProfileWithFollowing
|
||||
import press.mantra.compose.repository.ChatRepository
|
||||
import press.mantra.compose.repository.NostrRepository
|
||||
import press.mantra.compose.ui.composable.navigation.routes.ActiveProfileRoute
|
||||
import press.mantra.compose.ui.composable.navigation.routes.Route
|
||||
import press.mantra.compose.ui.composable.widgets.LoadingDataIndicator
|
||||
import press.mantra.compose.ui.composable.widgets.dialogs.NewChatBottomSheetDialog
|
||||
import press.mantra.compose.ui.composable.widgets.dialogs.StartDirectMessageToNpubOrNip05Dialog
|
||||
import press.mantra.compose.ui.composable.widgets.profile.ProfileAvatar
|
||||
import press.mantra.compose.ui.theme.TorchTheme
|
||||
import press.mantra.compose.ui.view.model.ChatRoomListViewModel
|
||||
import press.mantra.compose.ui.view.model.HomeScreenType
|
||||
@@ -61,7 +54,6 @@ import mantra.composeapp.generated.resources.Res
|
||||
import org.jetbrains.compose.resources.stringResource
|
||||
import mantra.composeapp.generated.resources.mantra
|
||||
import mantra.composeapp.generated.resources.new_chat
|
||||
import mantra.composeapp.generated.resources.something_went_wrong
|
||||
import press.mantra.compose.ui.composable.widgets.ErrorState
|
||||
import androidx.compose.material3.SnackbarHost
|
||||
import press.mantra.compose.ui.composable.widgets.LocalSnackbarHostState
|
||||
@@ -75,7 +67,6 @@ fun HomeScreen(
|
||||
onNavigateToRoute: (Route) -> Unit,
|
||||
onNavigateToDirectMessageDetail: (Route) -> Unit,
|
||||
onNavigateToChatRoomCreation: () -> Unit,
|
||||
onNavigateToSearch: () -> Unit,
|
||||
nostrRepository: NostrRepository,
|
||||
chatRepository: ChatRepository
|
||||
) {
|
||||
@@ -124,35 +115,13 @@ fun HomeScreen(
|
||||
// a content one.
|
||||
Text(stringResource(Res.string.mantra))
|
||||
},
|
||||
navigationIcon = {
|
||||
IconButton(
|
||||
onClick = {
|
||||
onNavigateToRoute.invoke(
|
||||
ActiveProfileRoute(
|
||||
activeUserPublicKey = activeUserPublicKey,
|
||||
nostrEventId = homeScreenUIState.profileWithFollowing.nostrEvent.id
|
||||
)
|
||||
)
|
||||
}
|
||||
) {
|
||||
ProfileAvatar(
|
||||
profile = homeScreenUIState.profileWithFollowing.profile,
|
||||
publicKey = homeScreenUIState.profileWithFollowing.profile.publicKey
|
||||
)
|
||||
}
|
||||
},
|
||||
actions = {
|
||||
IconButton(
|
||||
onClick = {
|
||||
onNavigateToSearch.invoke()
|
||||
}
|
||||
) {
|
||||
Icon(
|
||||
Icons.Default.Search,
|
||||
contentDescription = "Search"
|
||||
)
|
||||
}
|
||||
}
|
||||
// No navigation icon and no actions. The avatar in the leading
|
||||
// slot and the search icon in the trailing one were the app's only
|
||||
// two peer surfaces, and they are now items in the navigation
|
||||
// component -- a bar on a phone, a rail on a wide window. Leaving
|
||||
// them here as well would be two routes to one destination, which
|
||||
// is the thing M3's "swap only functionally equivalent components"
|
||||
// caution is about.
|
||||
)
|
||||
|
||||
},
|
||||
@@ -305,7 +274,6 @@ It has survived not only five centuries, but also the leap into electronic types
|
||||
),
|
||||
onNavigateToRoute = {},
|
||||
onNavigateToChatRoomCreation = {},
|
||||
onNavigateToSearch = {},
|
||||
onNavigateToDirectMessageDetail = {},
|
||||
nostrRepository = NostrRepository.NO_OP_NOSTR_REPOSITORY,
|
||||
chatRepository = ChatRepository.NO_OP_CHAT_REPOSITORY
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,151 @@
|
||||
package press.mantra.compose.ui.composable.navigation
|
||||
|
||||
import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi
|
||||
import androidx.compose.material3.Icon
|
||||
import androidx.compose.material3.Text
|
||||
import androidx.compose.material3.adaptive.navigationsuite.NavigationSuiteItem
|
||||
import androidx.compose.material3.adaptive.navigationsuite.NavigationSuiteScaffold
|
||||
import androidx.compose.material3.adaptive.navigationsuite.NavigationSuiteType
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.navigation.NavDestination
|
||||
import androidx.navigation.NavHostController
|
||||
import androidx.navigation.compose.currentBackStackEntryAsState
|
||||
import org.jetbrains.compose.resources.stringResource
|
||||
import press.mantra.compose.ui.composable.navigation.routes.HomeRoute
|
||||
import press.mantra.compose.ui.theme.Breakpoint
|
||||
|
||||
/**
|
||||
* The navigation bar, collapsed rail or expanded rail, around whatever the nav host draws.
|
||||
*
|
||||
* **Which component, at which breakpoint**, straight from the layout foundation's table:
|
||||
*
|
||||
* | breakpoint | component |
|
||||
* |---------------------|----------------------|
|
||||
* | compact | navigation bar |
|
||||
* | medium, expanded | collapsed rail |
|
||||
* | large, extra-large | expanded rail |
|
||||
*
|
||||
* `NavigationSuiteScaffoldDefaults.navigationSuiteType` is not used, and the difference is
|
||||
* the last row: it stops at `WideNavigationRailCollapsed` and never returns the expanded
|
||||
* rail, because it classifies with the three-value window size class rather than the five
|
||||
* breakpoints the May 2026 revision published. Deriving the type from [Breakpoint] instead
|
||||
* keeps one source of truth for window width in the app and reaches the row the library's
|
||||
* default cannot.
|
||||
*
|
||||
* **Hidden on everything else.** [NavigationSuiteType.None] draws no component at all, and
|
||||
* that is the case for every screen that is not one of the three: onboarding, a chat room,
|
||||
* a signing screen, the desktop passphrase gate. A navigation bar belongs on the
|
||||
* destinations it switches between; on a screen reached by pushing a route and left by
|
||||
* coming back it is a permanent invitation to lose your place.
|
||||
*
|
||||
* @param navController the host's controller, read for the current destination and used to
|
||||
* navigate. Passed rather than hoisted into callbacks because the top-level navigation
|
||||
* options below -- `saveState`, `restoreState`, `launchSingleTop` -- are the point of the
|
||||
* component and belong with it rather than at three call sites.
|
||||
* @param breakpoint the window's breakpoint, which decides the component.
|
||||
* @param activeUserPublicKey the signed-in key, or `null` before anybody has signed in --
|
||||
* in which case no destination has a route and the component stays hidden.
|
||||
* @param activeProfileNostrEventId the metadata event addressing the active profile, or
|
||||
* `null` until it has been read. Only [TopLevelDestination.Profile] needs it.
|
||||
*/
|
||||
@OptIn(ExperimentalMaterial3ExpressiveApi::class)
|
||||
@Composable
|
||||
fun MantraNavigationSuite(
|
||||
navController: NavHostController,
|
||||
breakpoint: Breakpoint,
|
||||
activeUserPublicKey: String?,
|
||||
activeProfileNostrEventId: String?,
|
||||
content: @Composable () -> Unit,
|
||||
) {
|
||||
val currentDestination: NavDestination? =
|
||||
navController.currentBackStackEntryAsState().value?.destination
|
||||
val selected = TopLevelDestination.of(currentDestination)
|
||||
|
||||
val suiteType = navigationSuiteTypeFor(
|
||||
breakpoint = breakpoint,
|
||||
// Not on a top-level destination, or nobody is signed in, so there is nothing for
|
||||
// the component to switch between.
|
||||
isTopLevel = selected != null && activeUserPublicKey != null,
|
||||
)
|
||||
|
||||
NavigationSuiteScaffold(
|
||||
navigationSuiteType = suiteType,
|
||||
navigationItems = {
|
||||
if (suiteType == NavigationSuiteType.None || activeUserPublicKey == null) return@NavigationSuiteScaffold
|
||||
|
||||
TopLevelDestination.entries.forEach { destination ->
|
||||
val route = destination.route(activeUserPublicKey, activeProfileNostrEventId)
|
||||
|
||||
NavigationSuiteItem(
|
||||
navigationSuiteType = suiteType,
|
||||
selected = destination == selected,
|
||||
// Disabled rather than absent while the profile's event id is still
|
||||
// being read. An item that appears and then disappears moves the two
|
||||
// beside it, and a bar whose items move under a thumb is worse than
|
||||
// one with a briefly unavailable item.
|
||||
enabled = route != null,
|
||||
onClick = {
|
||||
if (route != null && destination != selected) {
|
||||
navController.navigate(route) {
|
||||
// The standard top-level options, and each answers a
|
||||
// failure this app would otherwise have. Without
|
||||
// `popUpTo` the back stack grows by one every time
|
||||
// somebody taps between the three, so back becomes a
|
||||
// walk through their tapping history rather than a way
|
||||
// out. Without `launchSingleTop` a second tap on the
|
||||
// current item pushes a duplicate. `saveState` and
|
||||
// `restoreState` are what keep a scrolled feed scrolled
|
||||
// when you come back to it.
|
||||
//
|
||||
// Popped to `HomeRoute` and not to the graph's start
|
||||
// destination, which is the shape the android docs give
|
||||
// and which would be wrong here: this graph starts at
|
||||
// `LoadingRoute`, and onboarding clears the stack with
|
||||
// `popUpTo(0)` on its way to home -- so by the time these
|
||||
// items exist the start destination is not on the stack at
|
||||
// all, and popping to it would leave the loading screen
|
||||
// underneath as the thing back returns to. Home is the
|
||||
// root of the signed-in graph, and is what back should
|
||||
// reach from either of the other two.
|
||||
popUpTo<HomeRoute> { saveState = true }
|
||||
launchSingleTop = true
|
||||
restoreState = true
|
||||
}
|
||||
}
|
||||
},
|
||||
icon = {
|
||||
Icon(
|
||||
imageVector = destination.icon,
|
||||
// Null, and decided rather than defaulted: the label is
|
||||
// beside the icon in the bar and in the expanded rail, and a
|
||||
// reader would hear the name twice. The collapsed rail is the
|
||||
// exception, and it draws the label as a tooltip that the
|
||||
// accessibility tree still carries.
|
||||
contentDescription = null,
|
||||
)
|
||||
},
|
||||
label = { Text(stringResource(destination.label)) },
|
||||
)
|
||||
}
|
||||
},
|
||||
content = content,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The navigation component a window at [breakpoint] should carry, per the table above.
|
||||
*
|
||||
* Separate from the composable so the table can be asserted. A breakpoint-to-component
|
||||
* mapping is exactly the kind of thing that is wrong in one row and looks right in every
|
||||
* screenshot anybody takes: the two rail rows differ only in whether labels are shown, and
|
||||
* nobody opens a 1200dp window to check.
|
||||
*/
|
||||
@OptIn(ExperimentalMaterial3ExpressiveApi::class)
|
||||
internal fun navigationSuiteTypeFor(breakpoint: Breakpoint, isTopLevel: Boolean): NavigationSuiteType =
|
||||
when {
|
||||
!isTopLevel -> NavigationSuiteType.None
|
||||
breakpoint == Breakpoint.Compact -> NavigationSuiteType.ShortNavigationBarCompact
|
||||
breakpoint.isAtLeast(Breakpoint.Large) -> NavigationSuiteType.WideNavigationRailExpanded
|
||||
else -> NavigationSuiteType.WideNavigationRailCollapsed
|
||||
}
|
||||
@@ -0,0 +1,80 @@
|
||||
package press.mantra.compose.ui.composable.navigation
|
||||
|
||||
import androidx.compose.material.icons.Icons
|
||||
import androidx.compose.material.icons.filled.Forum
|
||||
import androidx.compose.material.icons.filled.Person
|
||||
import androidx.compose.material.icons.filled.Search
|
||||
import androidx.compose.ui.graphics.vector.ImageVector
|
||||
import androidx.navigation.NavDestination
|
||||
import androidx.navigation.NavDestination.Companion.hasRoute
|
||||
import mantra.composeapp.generated.resources.Res
|
||||
import mantra.composeapp.generated.resources.messages
|
||||
import mantra.composeapp.generated.resources.profile
|
||||
import mantra.composeapp.generated.resources.search
|
||||
import org.jetbrains.compose.resources.StringResource
|
||||
import press.mantra.compose.ui.composable.navigation.routes.ActiveProfileRoute
|
||||
import press.mantra.compose.ui.composable.navigation.routes.HomeRoute
|
||||
import press.mantra.compose.ui.composable.navigation.routes.Route
|
||||
import press.mantra.compose.ui.composable.navigation.routes.SearchRoute
|
||||
|
||||
/**
|
||||
* The three destinations the navigation component switches between.
|
||||
*
|
||||
* The app has one screen that is genuinely a top-level destination -- [Messages], which
|
||||
* is the home feed -- and two surfaces that were one tap from it through the home screen's
|
||||
* app bar: the profile avatar in the leading slot and the search icon in the trailing one.
|
||||
* Promoting those two is an information-architecture decision rather than a component
|
||||
* swap, and it was taken deliberately: M3's caution is to swap only functionally
|
||||
* equivalent components, and a navigation bar holding a single item would have been
|
||||
* strictly worse than the app bar it replaced.
|
||||
*
|
||||
* The consequence is that the home screen's app bar no longer carries either icon. Two
|
||||
* routes to the same destination is what the caution is about; the navigation component is
|
||||
* now the one route, at every breakpoint.
|
||||
*
|
||||
* @property icon the item's icon, and the only thing shown when the rail is collapsed.
|
||||
* @property label the item's text, shown in the bar and in the expanded rail.
|
||||
*/
|
||||
enum class TopLevelDestination(val icon: ImageVector, val label: StringResource) {
|
||||
Messages(Icons.Default.Forum, Res.string.messages),
|
||||
Search(Icons.Default.Search, Res.string.search),
|
||||
Profile(Icons.Default.Person, Res.string.profile);
|
||||
|
||||
companion object {
|
||||
/**
|
||||
* Which destination [destination] belongs to, or `null` for everything else.
|
||||
*
|
||||
* `null` is what hides the navigation component. A navigation bar belongs on the
|
||||
* screens it switches between and nowhere else -- it has no meaning during
|
||||
* onboarding, in a chat room, or on a signing screen, all of which are reached by
|
||||
* pushing a route and left by coming back.
|
||||
*/
|
||||
fun of(destination: NavDestination?): TopLevelDestination? = when {
|
||||
destination == null -> null
|
||||
destination.hasRoute(HomeRoute::class) -> Messages
|
||||
destination.hasRoute(SearchRoute::class) -> Search
|
||||
destination.hasRoute(ActiveProfileRoute::class) -> Profile
|
||||
else -> null
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The route this item navigates to, or `null` when it cannot yet be built.
|
||||
*
|
||||
* Only [Profile] ever returns `null`, and only before the active profile's metadata
|
||||
* event has been read from the database. `ActiveProfileRoute` is addressed by event id
|
||||
* rather than by public key, so until that read lands there is no route to construct --
|
||||
* and an item that navigates nowhere is worse than one that is visibly disabled.
|
||||
*/
|
||||
fun route(activeUserPublicKey: String, activeProfileNostrEventId: String?): Route? =
|
||||
when (this) {
|
||||
Messages -> HomeRoute(activeUserPublicKey = activeUserPublicKey)
|
||||
Search -> SearchRoute(activeUserPublicKey = activeUserPublicKey)
|
||||
Profile -> activeProfileNostrEventId?.let {
|
||||
ActiveProfileRoute(
|
||||
activeUserPublicKey = activeUserPublicKey,
|
||||
nostrEventId = it,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user