From be526711d70b4fc135f8b76cffb17ea7483cea31 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 5 Oct 2026 19:34:24 +0200 Subject: [PATCH] feat(shell): swipe between tabs The four tabs are now pages of one HorizontalPager on a single NavHost route, so a sideways swipe moves between Alarms, Timers, Stopwatch and World clock as well as tapping the bar. The pager is the source of truth for the selected tab; editors, the timer setup page and Settings stay NavHost destinations pushed over it. A tab tap from one of them pops back to the tabs, and a deep link always lands on its tab rather than on whatever nested screen was last open there. Swiping is on only under a navigation bar: beside a rail the tabs are stacked vertically, and a horizontal swipe has no counterpart there. Closes #2 --- .../clockula/ui/shell/ClockulaDestination.kt | 10 +- .../clockula/ui/shell/ClockulaShell.kt | 201 +++++++++++------- .../clockula/ui/shell/ShellNavigation.kt | 113 ++++++---- .../clockula/ui/alarms/AlarmRoutesTest.kt | 8 +- .../clockula/ui/shell/ShellNavigationTest.kt | 183 +++++++++------- docs/ARCHITECTURE.md | 59 +++-- 6 files changed, 358 insertions(+), 216 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/shell/ClockulaDestination.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/shell/ClockulaDestination.kt index 0b86314..4b92d09 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/shell/ClockulaDestination.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/shell/ClockulaDestination.kt @@ -3,8 +3,14 @@ package de.jeanlucmakiola.clockula.ui.shell import androidx.annotation.StringRes import de.jeanlucmakiola.clockula.R -/** The shell's four top-level destinations, in tab order. No Compose types. */ -enum class ClockulaDestination(val route: String, @StringRes val titleRes: Int) { +/** + * The shell's four top-level destinations, in tab order — which is also their + * page order in the shell's pager. No Compose types. + * + * [key] is the tab's page key: stable and `Bundle`-saveable, so a tab's + * `rememberSaveable` state is filed under it rather than under its position. + */ +enum class ClockulaDestination(val key: String, @StringRes val titleRes: Int) { ALARMS("alarms", R.string.tab_alarms), TIMERS("timers", R.string.tab_timers), STOPWATCH("stopwatch", R.string.tab_stopwatch), diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/shell/ClockulaShell.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/shell/ClockulaShell.kt index 3c9dddd..71a4595 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/shell/ClockulaShell.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/shell/ClockulaShell.kt @@ -8,15 +8,22 @@ import androidx.compose.foundation.layout.only import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.safeDrawing import androidx.compose.foundation.layout.windowInsetsPadding +import androidx.compose.foundation.pager.HorizontalPager +import androidx.compose.foundation.pager.PagerState +import androidx.compose.foundation.pager.rememberPagerState import androidx.compose.material3.Icon import androidx.compose.material3.Text +import androidx.compose.material3.adaptive.currentWindowAdaptiveInfo import androidx.compose.material3.adaptive.navigationsuite.NavigationSuiteItem import androidx.compose.material3.adaptive.navigationsuite.NavigationSuiteScaffold +import androidx.compose.material3.adaptive.navigationsuite.NavigationSuiteScaffoldDefaults import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment @@ -47,6 +54,10 @@ import de.jeanlucmakiola.clockula.ui.timers.TimersScreen import de.jeanlucmakiola.clockula.ui.worldclock.WorldClockScreen import de.jeanlucmakiola.floret.identity.fadeThrough import de.jeanlucmakiola.floret.identity.predictiveBack +import de.jeanlucmakiola.floret.identity.rememberReduceMotion +import kotlinx.coroutines.launch +import kotlin.math.abs +import kotlin.math.sign /** * The adaptive navigation shell: four tabs, the live pill, predictive back. @@ -55,10 +66,16 @@ import de.jeanlucmakiola.floret.identity.predictiveBack * size — a short navigation bar on a phone, a wide rail on a tablet — because * `NavigationSuiteScaffold`'s default already answers the tabletop posture and * the compact-*height* case that an `if (compact) bar else rail` would miss. + * The shell computes that default itself, with the scaffold's own function, so + * the pager can ask the same answer whether it may swipe. * - * The `NavHost` is the single source of truth for the selected tab: Navigation - * Compose already saves its back stack through a `SavedStateHandle`, so the - * selection survives rotation and process death with no mirror of our own. + * The four tabs are the pages of one [HorizontalPager] on a single `NavHost` + * route, [ShellNavigation.TABS_ROUTE], so they can be swiped between under a + * navigation bar ([ShellNavigation.swipesBetweenTabs] says why not under a + * rail). The pager is the single source of truth for the selected tab — its + * state is saveable, so the tab survives rotation and process death with no + * mirror of ours to drift — and the `NavHost` holds only what is pushed over + * the tabs: the editors, the timer setup page, Settings. */ @Composable fun ClockulaShell( @@ -71,11 +88,24 @@ fun ClockulaShell( val pill by viewModel.livePill.collectAsStateWithLifecycle() val entry by navController.currentBackStackEntryAsState() val route = entry?.destination?.route - val selected = ShellNavigation.selectedDestinationOf(route) ?: ShellNavigation.start - val back = ShellNavigation.onBack(route) + val scope = rememberCoroutineScope() + val reduceMotion = rememberReduceMotion() - // Peer destinations have no spatial relationship, so tabs fade through - // rather than sliding: a slide would claim a hierarchy that is not there. + // Hoisted above the NavHost, so it outlives the tabs route while a nested + // screen covers it, and a tab tap from an editor can still move it. + val pagerState = rememberPagerState(initialPage = ShellNavigation.pageOf(ShellNavigation.start)) { + ShellNavigation.topLevel.size + } + // A tapped tab is selected from the tap on, not from whichever page the + // slide happens to pass through on its way there. + var tabInFlight by remember { mutableStateOf(null) } + val selected = tabInFlight ?: ShellNavigation.destinationAt(pagerState.targetPage) + val back = ShellNavigation.onBack(route, selected) + + val navigationSuiteType = NavigationSuiteScaffoldDefaults.navigationSuiteType(currentWindowAdaptiveInfo()) + + // A nested screen is no peer of the tabs and has no spatial relationship + // to them, so it fades through rather than sliding. val fade = fadeThrough() // The `AlarmClock` contract's chooser, held across a rotation but not a @@ -83,35 +113,49 @@ fun ClockulaShell( // would be a dialog with no provenance (M9 D13). var dismissCandidates by rememberSaveable { mutableStateOf(LongArray(0)) } - // A notification's deep link — or the exported door's outcome — replayed as - // exactly the command a tab tap produces, which is what the user expects, - // and it leaves M4's asserted back policy alone: the start destination is - // still Alarms (M6 D24, M9 D23). + // Replays a TabSelection — the policy's decision — onto the controller + // and the pager. + fun select(selection: TabSelection) { + if (selection.popToTabs) navController.popBackStack(ShellNavigation.TABS_ROUTE, inclusive = false) + val page = ShellNavigation.pageOf(selection.destination) + if (!selection.animate || reduceMotion) { + // Safe while the pager is not composed: the request is read on its + // next measure, which is when the tabs route comes back. + pagerState.requestScrollToPage(page) + return + } + scope.launch { + tabInFlight = selection.destination + try { + pagerState.slideTo(page) + } finally { + tabInFlight = null + } + } + } + + // A notification's or widget's deep link — or the exported door's + // outcome — lands on its tab whatever was open there: a nested screen is + // popped rather than resumed. The start tab is still Alarms, so M4's + // asserted back policy is left alone (M6 D24, M9 D23). LaunchedEffect(openRequest) { when (val request = openRequest) { null -> return@LaunchedEffect - is ShellRequest.OpenTab -> - navController.replay(ShellNavigation.onTabSelected(route, request.destination)) + is ShellRequest.OpenTab -> select(ShellNavigation.onTabRequested(route, request.destination)) is ShellRequest.OpenAlarmEditor -> { // The same two steps the FAB takes, so the back stack is the one // M5 asserted: the tab, then the editor on top of it. - navController.replay( - ShellNavigation.onTabSelected(route, ClockulaDestination.ALARMS), - ) + select(ShellNavigation.onTabRequested(route, ClockulaDestination.ALARMS)) navController.navigate(AlarmRoutes.editor(request.alarmId)) } ShellRequest.ComposeTimer -> { // The same two steps the FAB takes: the tab, then the // full-screen setup page on top of it (M9 D7). - navController.replay( - ShellNavigation.onTabSelected(route, ClockulaDestination.TIMERS), - ) + select(ShellNavigation.onTabRequested(route, ClockulaDestination.TIMERS)) navController.navigate(TimerRoutes.NEW) } is ShellRequest.ChooseAlarmToDismiss -> { - navController.replay( - ShellNavigation.onTabSelected(route, ClockulaDestination.ALARMS), - ) + select(ShellNavigation.onTabRequested(route, ClockulaDestination.ALARMS)) dismissCandidates = request.alarmIds.toLongArray() } } @@ -124,9 +168,7 @@ fun ClockulaShell( val current = destination == selected NavigationSuiteItem( selected = current, - onClick = { - navController.replay(ShellNavigation.onTabSelected(route, destination)) - }, + onClick = { select(ShellNavigation.onTabSelected(route, destination)) }, // The label is right there beside it; announcing the glyph // too would only say everything twice. icon = { Icon(destination.icon(current), contentDescription = null) }, @@ -136,14 +178,13 @@ fun ClockulaShell( }, modifier = modifier.predictiveBack( onBack = { - if (back is ShellBackAction.GoTo) { - navController.replay(ShellNavigation.onTabSelected(route, back.destination)) - } + if (back is ShellBackAction.GoTo) select(ShellNavigation.onTabSelected(route, back.destination)) }, // On Alarms the handler stands aside, so the gesture previews // *leaving the app* — which is what it is about to do. enabled = back is ShellBackAction.GoTo, ), + navigationSuiteType = navigationSuiteType, ) { Box(Modifier.fillMaxSize()) { CompositionLocalProvider( @@ -151,24 +192,36 @@ fun ClockulaShell( ) { NavHost( navController = navController, - startDestination = ShellNavigation.start.route, + startDestination = ShellNavigation.TABS_ROUTE, enterTransition = { fade.targetContentEnter }, exitTransition = { fade.initialContentExit }, popEnterTransition = { fade.targetContentEnter }, popExitTransition = { fade.initialContentExit }, ) { - composable(ClockulaDestination.ALARMS.route) { - AlarmsScreen( - onEditAlarm = { navController.navigate(AlarmRoutes.editor(it)) }, - onCreateAlarm = { navController.navigate(AlarmRoutes.newEditor(it)) }, - onOpenSettings = { navController.navigate(SettingsRoutes.MAIN) }, - dismissCandidates = dismissCandidates, - onDismissCandidatesConsumed = { dismissCandidates = LongArray(0) }, - ) + composable(ShellNavigation.TABS_ROUTE) { + HorizontalPager( + state = pagerState, + modifier = Modifier.fillMaxSize(), + // beyondViewportPageCount stays 0: a tab is composed + // only while it is on screen, so the stopwatch's + // ~60 Hz readout costs nothing from next door. An + // off-screen tab keeps its saveable state (scroll + // position and the like) under its key, and its + // ViewModel in this route's store. + key = { ShellNavigation.destinationAt(it).key }, + userScrollEnabled = ShellNavigation.swipesBetweenTabs(navigationSuiteType), + ) { page -> + Tab( + destination = ShellNavigation.destinationAt(page), + navController = navController, + dismissCandidates = dismissCandidates, + onDismissCandidatesConsumed = { dismissCandidates = LongArray(0) }, + ) + } } - // A sibling route in the same flat host, not a nested one: - // it is no tab, so the Alarms tab stays selected and the - // shell stands aside on back (M5 D1). + // Nested screens in the same flat host: none is a tab, so + // the pager keeps the tab underneath selected and the shell + // stands aside on back (M5 D1). composable( route = AlarmRoutes.EDITOR_PATTERN, arguments = listOf( @@ -184,16 +237,6 @@ fun ClockulaShell( openTimePicker = entry.arguments?.getBoolean(AlarmRoutes.ARG_PICK_TIME) == true, ) } - composable(ClockulaDestination.TIMERS.route) { - TimersScreen( - onEditTimer = { navController.navigate(TimerRoutes.editor(it)) }, - onAddTimer = { navController.navigate(TimerRoutes.NEW) }, - onOpenSettings = { navController.navigate(SettingsRoutes.MAIN) }, - ) - } - // Sibling routes in the same flat host, exactly as the - // alarm editor is: neither is a tab, so the Timers tab - // stays selected and the shell stands aside on back (M5 D1). composable( route = TimerRoutes.EDITOR_PATTERN, arguments = listOf( @@ -205,19 +248,8 @@ fun ClockulaShell( composable(TimerRoutes.NEW) { TimerSetupScreen(onBack = { navController.popBackStack() }) } - composable(ClockulaDestination.STOPWATCH.route) { - StopwatchScreen( - onOpenSettings = { navController.navigate(SettingsRoutes.MAIN) }, - ) - } - composable(ClockulaDestination.WORLD_CLOCK.route) { - WorldClockScreen( - onOpenSettings = { navController.navigate(SettingsRoutes.MAIN) }, - ) - } - // Sibling routes, reachable from any tab via the gear — no - // single tab owns Settings, which is why ShellNavigation - // deliberately maps none of these to a parent destination. + // Reachable from any tab via the gear — no single tab owns + // Settings, and the tab it was opened from stays selected. composable(SettingsRoutes.MAIN) { SettingsScreen( onBack = { navController.popBackStack() }, @@ -255,14 +287,39 @@ fun ClockulaShell( } } -/** Replays a [TabNavCommand] — the policy's decision — onto the controller. */ -private fun NavHostController.replay(command: TabNavCommand) { - navigate(command.route) { - popUpTo(command.popUpTo) { - inclusive = command.popUpToInclusive - saveState = command.saveState - } - launchSingleTop = command.launchSingleTop - restoreState = command.restoreState +/** One tab's screen, as a page of the shell's pager. */ +@Composable +private fun Tab( + destination: ClockulaDestination, + navController: NavHostController, + dismissCandidates: LongArray, + onDismissCandidatesConsumed: () -> Unit, +) { + val openSettings = { navController.navigate(SettingsRoutes.MAIN) } + when (destination) { + ClockulaDestination.ALARMS -> AlarmsScreen( + onEditAlarm = { navController.navigate(AlarmRoutes.editor(it)) }, + onCreateAlarm = { navController.navigate(AlarmRoutes.newEditor(it)) }, + onOpenSettings = openSettings, + dismissCandidates = dismissCandidates, + onDismissCandidatesConsumed = onDismissCandidatesConsumed, + ) + ClockulaDestination.TIMERS -> TimersScreen( + onEditTimer = { navController.navigate(TimerRoutes.editor(it)) }, + onAddTimer = { navController.navigate(TimerRoutes.NEW) }, + onOpenSettings = openSettings, + ) + ClockulaDestination.STOPWATCH -> StopwatchScreen(onOpenSettings = openSettings) + ClockulaDestination.WORLD_CLOCK -> WorldClockScreen(onOpenSettings = openSettings) } } + +/** + * Slides to [page] from its neighbour: a tap two or three tabs away would + * otherwise compose — and flash past — every tab in between. + */ +private suspend fun PagerState.slideTo(page: Int) { + val distance = page - currentPage + if (abs(distance) > 1) scrollToPage(page - distance.sign) + animateScrollToPage(page) +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/shell/ShellNavigation.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/shell/ShellNavigation.kt index 22d834c..d37a96b 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/shell/ShellNavigation.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/shell/ShellNavigation.kt @@ -1,20 +1,25 @@ package de.jeanlucmakiola.clockula.ui.shell +import androidx.compose.material3.adaptive.navigationsuite.NavigationSuiteType import de.jeanlucmakiola.clockula.alarm.AlarmIntents import de.jeanlucmakiola.clockula.interop.InteropIntents import de.jeanlucmakiola.clockula.stopwatch.StopwatchIntents import de.jeanlucmakiola.clockula.timer.TimerIntents -import de.jeanlucmakiola.clockula.ui.alarms.AlarmRoutes -import de.jeanlucmakiola.clockula.ui.timers.TimerRoutes -/** What a tab tap does to the back stack. Pure data; the composable replays it. */ -data class TabNavCommand( - val route: String, - val popUpTo: String, - val popUpToInclusive: Boolean, - val saveState: Boolean, - val restoreState: Boolean, - val launchSingleTop: Boolean, +/** + * What selecting a tab does. Pure data; the shell replays it onto the + * `NavController` and the pager. + */ +data class TabSelection( + val destination: ClockulaDestination, + /** Pop whatever is open over the tabs — an editor, Settings — back to them first. */ + val popToTabs: Boolean, + /** + * Slide the pager there. False jumps instead: over a nested screen the + * `NavHost`'s own fade carries the change, and a slide would play unseen + * underneath it. + */ + val animate: Boolean, ) /** @@ -47,45 +52,65 @@ object ShellNavigation { */ const val ACTION_SHOW_WORLD_CLOCK: String = "de.jeanlucmakiola.clockula.action.SHOW_WORLD_CLOCK" + /** + * The one `NavHost` route the four tabs share: a pager holds them, and the + * pager — not the back stack — says which one is showing. Everything else in + * the host (the editors, the timer setup page, Settings) is a nested screen + * pushed over it. + */ + const val TABS_ROUTE: String = "tabs" + val start: ClockulaDestination = ClockulaDestination.ALARMS val topLevel: List = ClockulaDestination.entries.toList() - private val byRoute: Map = topLevel.associateBy { it.route } + /** The tab on pager page [page]; out of range clamps rather than throws. */ + fun destinationAt(page: Int): ClockulaDestination = topLevel[page.coerceIn(0, topLevel.lastIndex)] - /** Exact match only: a nested route is not a tab. */ - fun destinationOf(route: String?): ClockulaDestination? = byRoute[route] + /** The pager page [destination] lives on. */ + fun pageOf(destination: ClockulaDestination): Int = topLevel.indexOf(destination) /** - * Keep the parent tab highlighted while its editor — or setup page — is open. - * - * Settings (M10) deliberately has **no** entry here: it opens from a gear on - * every tab, not from one owning tab, so there is no single parent to - * highlight. It falls through to [destinationOf], which returns null for it, - * and the caller's `selected ?: start` then leaves Alarms highlighted - * underneath — the same fallback an unrecognised route already gets. + * Whether [route] is a screen pushed over the tabs. Null — before the host + * has its first destination — is not: there is nothing to pop yet. */ - fun selectedDestinationOf(route: String?): ClockulaDestination? = when (route) { - AlarmRoutes.EDITOR_PATTERN -> ClockulaDestination.ALARMS - TimerRoutes.EDITOR_PATTERN, TimerRoutes.NEW -> ClockulaDestination.TIMERS - else -> destinationOf(route) + fun isNested(route: String?): Boolean = route != null && route != TABS_ROUTE + + /** + * A tab tap. Over a nested screen it pops back to the tabs first, so + * re-selecting the tab underneath an editor closes the editor — the + * standard behaviour, and the contract M5's editor relies on. Every nested + * screen writes as the user goes (M5 D22), so popping one loses nothing. + */ + fun onTabSelected(currentRoute: String?, target: ClockulaDestination): TabSelection { + val nested = isNested(currentRoute) + return TabSelection(target, popToTabs = nested, animate = !nested) } /** - * The canonical multi-back-stack command. Re-selecting the tab the user is - * already on does *not* restore its saved stack, which pops that tab back - * to its own root — the standard behaviour, and the contract M5's editor - * will rely on. + * A tab an intent asks for — a notification, a widget, the exported door. + * The same pop as a tap, so it lands on the tab itself whatever was open, + * but a jump rather than a slide: the user did not come from the + * neighbouring tab, they came from outside the app. */ - fun onTabSelected(currentRoute: String?, target: ClockulaDestination): TabNavCommand = - TabNavCommand( - route = target.route, - popUpTo = start.route, - popUpToInclusive = false, - saveState = true, - restoreState = currentRoute != target.route, - launchSingleTop = true, - ) + fun onTabRequested(currentRoute: String?, target: ClockulaDestination): TabSelection = + onTabSelected(currentRoute, target).copy(animate = false) + + /** + * Whether a horizontal swipe pages between the tabs, given the navigation + * component the window gets. + * + * Only under a navigation **bar**. A bar is a horizontal row of the same + * four tabs, so a sideways swipe maps onto it one to one. A rail stacks them + * vertically down the side of a tablet or an unfolded foldable, where a + * sideways swipe has no counterpart in the navigation the user can see — + * and a wide window's content is where a horizontal gesture is most likely + * meant for the content itself. There, tabs change by tapping the rail. + */ + fun swipesBetweenTabs(type: NavigationSuiteType): Boolean = + type == NavigationSuiteType.ShortNavigationBarCompact || + type == NavigationSuiteType.ShortNavigationBarMedium || + type == NavigationSuiteType.NavigationBar /** * The tab an incoming intent action asks for. Null for null and for @@ -131,11 +156,13 @@ object ShellNavigation { /** * Alarms is the shell's root: back from any other tab returns to it, and - * back from Alarms leaves the app. An unrecognised route is a nested - * destination the `NavController` pops itself, so the shell stands aside. + * back from Alarms leaves the app. A nested screen is the `NavController`'s + * to pop, and a null route has nothing under it, so the shell stands aside. */ - fun onBack(currentRoute: String?): ShellBackAction = when (destinationOf(currentRoute)) { - null, start -> ShellBackAction.Exit - else -> ShellBackAction.GoTo(start) - } + fun onBack(currentRoute: String?, current: ClockulaDestination): ShellBackAction = + if (currentRoute != TABS_ROUTE || current == start) { + ShellBackAction.Exit + } else { + ShellBackAction.GoTo(start) + } } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRoutesTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRoutesTest.kt index ca519fa..3765562 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRoutesTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRoutesTest.kt @@ -1,6 +1,7 @@ package de.jeanlucmakiola.clockula.ui.alarms import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.clockula.ui.shell.ClockulaDestination import de.jeanlucmakiola.clockula.ui.shell.ShellBackAction import de.jeanlucmakiola.clockula.ui.shell.ShellNavigation import org.junit.jupiter.api.Test @@ -23,12 +24,13 @@ class AlarmRoutesTest { } @Test - fun `the editor is no tab, so the alarms tab stays selected`() { - assertThat(ShellNavigation.destinationOf(AlarmRoutes.editor(7L))).isNull() + fun `the editor is a screen nested over the tabs`() { + assertThat(ShellNavigation.isNested(AlarmRoutes.EDITOR_PATTERN)).isTrue() } @Test fun `the shell stands aside on back, leaving the editor to pop itself`() { - assertThat(ShellNavigation.onBack(AlarmRoutes.editor(7L))).isEqualTo(ShellBackAction.Exit) + assertThat(ShellNavigation.onBack(AlarmRoutes.EDITOR_PATTERN, ClockulaDestination.ALARMS)) + .isEqualTo(ShellBackAction.Exit) } } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/shell/ShellNavigationTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/shell/ShellNavigationTest.kt index 354fcc1..c00023c 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/shell/ShellNavigationTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/shell/ShellNavigationTest.kt @@ -1,5 +1,6 @@ package de.jeanlucmakiola.clockula.ui.shell +import androidx.compose.material3.adaptive.navigationsuite.NavigationSuiteType import com.google.common.truth.Truth.assertThat import de.jeanlucmakiola.clockula.stopwatch.StopwatchIntents import de.jeanlucmakiola.clockula.timer.TimerIntents @@ -10,8 +11,9 @@ import org.junit.jupiter.api.Test /** * The shell's navigation policy as pure data: which tab is the root, what a tab - * tap does to the back stack, and where Back goes. The four lines that replay a - * [TabNavCommand] onto a `NavController` are not worth testing; this is. + * tap does to the back stack and the pager, and where Back goes. The lines that + * replay a [TabSelection] onto a `NavController` and a `PagerState` are not + * worth testing; this is. */ class ShellNavigationTest { @@ -28,10 +30,10 @@ class ShellNavigationTest { } @Test - fun `every tab has its own non-blank route`() { - val routes = ShellNavigation.topLevel.map { it.route } + fun `every tab has its own non-blank page key`() { + val keys = ShellNavigation.topLevel.map { it.key } - assertThat(routes.filter { it.isNotBlank() }.toSet()).hasSize(4) + assertThat(keys.filter { it.isNotBlank() }.toSet()).hasSize(4) } @Test @@ -46,133 +48,156 @@ class ShellNavigationTest { assertThat(ShellNavigation.start).isEqualTo(ClockulaDestination.ALARMS) } + // --- #2: the tabs are pages of one pager --- + @Test - fun `each tab's own route resolves back to it`() { - val resolved = ClockulaDestination.entries.map { ShellNavigation.destinationOf(it.route) } + fun `each tab's own page resolves back to it`() { + val resolved = ClockulaDestination.entries.map { + ShellNavigation.destinationAt(ShellNavigation.pageOf(it)) + } assertThat(resolved).isEqualTo(ClockulaDestination.entries.toList()) } @Test - fun `a null route resolves to no destination`() { - assertThat(ShellNavigation.destinationOf(null)).isNull() + fun `pages follow tab order`() { + val pages = ShellNavigation.topLevel.map(ShellNavigation::pageOf) + + assertThat(pages).containsExactly(0, 1, 2, 3).inOrder() } @Test - fun `an unknown route resolves to no destination`() { - assertThat(ShellNavigation.destinationOf("nope")).isNull() + fun `an out-of-range page clamps instead of throwing`() { + assertThat(ShellNavigation.destinationAt(-1)).isEqualTo(ClockulaDestination.ALARMS) + assertThat(ShellNavigation.destinationAt(9)).isEqualTo(ClockulaDestination.WORLD_CLOCK) } @Test - fun `a nested route does not resolve by prefix`() { - assertThat(ShellNavigation.destinationOf("alarms/12")).isNull() + fun `the tabs route and an unknown host are not nested`() { + assertThat(ShellNavigation.isNested(ShellNavigation.TABS_ROUTE)).isFalse() + assertThat(ShellNavigation.isNested(null)).isFalse() } @Test - fun `selecting another tab saves and restores the tab's own stack`() { - val command = ShellNavigation.onTabSelected("alarms", ClockulaDestination.TIMERS) + fun `editors, the timer setup page and settings are nested over the tabs`() { + val routes = listOf( + AlarmRoutes.EDITOR_PATTERN, + TimerRoutes.EDITOR_PATTERN, + TimerRoutes.NEW, + SettingsRoutes.MAIN, + SettingsRoutes.SELF_CHECK, + SettingsRoutes.BACKUP, + ) - assertThat(command).isEqualTo( - TabNavCommand( - route = "timers", - popUpTo = "alarms", - popUpToInclusive = false, - saveState = true, - restoreState = true, - launchSingleTop = true, - ), + assertThat(routes.map(ShellNavigation::isNested).toSet()).containsExactly(true) + } + + @Test + fun `a tap on the tabs slides there without touching the back stack`() { + val selection = ShellNavigation.onTabSelected(ShellNavigation.TABS_ROUTE, ClockulaDestination.TIMERS) + + assertThat(selection).isEqualTo( + TabSelection(ClockulaDestination.TIMERS, popToTabs = false, animate = true), ) } @Test - fun `re-selecting the current tab pops its own stack instead of restoring it`() { - val command = ShellNavigation.onTabSelected("timers", ClockulaDestination.TIMERS) + fun `a tap over a nested screen pops back to the tabs and jumps`() { + val selection = ShellNavigation.onTabSelected(SettingsRoutes.MAIN, ClockulaDestination.STOPWATCH) - assertThat(command).isEqualTo( - TabNavCommand( - route = "timers", - popUpTo = "alarms", - popUpToInclusive = false, - saveState = true, - restoreState = false, - launchSingleTop = true, - ), + assertThat(selection).isEqualTo( + TabSelection(ClockulaDestination.STOPWATCH, popToTabs = true, animate = false), ) } @Test - fun `selecting a tab before the first destination is known restores its stack`() { - val command = ShellNavigation.onTabSelected(null, ClockulaDestination.TIMERS) + fun `re-selecting the tab under its editor pops the editor`() { + val selection = ShellNavigation.onTabSelected(TimerRoutes.EDITOR_PATTERN, ClockulaDestination.TIMERS) - assertThat(command.restoreState).isTrue() + assertThat(selection.popToTabs).isTrue() + assertThat(selection.destination).isEqualTo(ClockulaDestination.TIMERS) } @Test - fun `every tab tap pops up to the start destination`() { - val popUpTos = ClockulaDestination.entries.flatMap { current -> - ClockulaDestination.entries.map { target -> - ShellNavigation.onTabSelected(current.route, target).popUpTo - } - } + fun `a tab asked for by an intent never slides, wherever the app was`() { + val routes = listOf(null, ShellNavigation.TABS_ROUTE, AlarmRoutes.EDITOR_PATTERN, SettingsRoutes.BACKUP) - assertThat(popUpTos.toSet()).containsExactly(ShellNavigation.start.route) + val animated = routes.map { ShellNavigation.onTabRequested(it, ClockulaDestination.WORLD_CLOCK).animate } + + assertThat(animated.toSet()).containsExactly(false) } + @Test + fun `a tab asked for by an intent pops any nested screen, so it lands on the tab itself`() { + val selection = ShellNavigation.onTabRequested(SettingsRoutes.MAIN, ClockulaDestination.ALARMS) + + assertThat(selection).isEqualTo( + TabSelection(ClockulaDestination.ALARMS, popToTabs = true, animate = false), + ) + } + + @Test + fun `swiping pages the tabs under a navigation bar`() { + val bars = listOf( + NavigationSuiteType.ShortNavigationBarCompact, + NavigationSuiteType.ShortNavigationBarMedium, + NavigationSuiteType.NavigationBar, + ) + + assertThat(bars.map(ShellNavigation::swipesBetweenTabs).toSet()).containsExactly(true) + } + + @Test + fun `swiping is off beside a rail, a drawer, or no navigation at all`() { + val others = listOf( + NavigationSuiteType.WideNavigationRailCollapsed, + NavigationSuiteType.WideNavigationRailExpanded, + NavigationSuiteType.NavigationRail, + NavigationSuiteType.NavigationDrawer, + NavigationSuiteType.None, + ) + + assertThat(others.map(ShellNavigation::swipesBetweenTabs).toSet()).containsExactly(false) + } + + // --- back --- + @Test fun `back from alarms leaves the app`() { - assertThat(ShellNavigation.onBack("alarms")).isEqualTo(ShellBackAction.Exit) + assertThat(ShellNavigation.onBack(ShellNavigation.TABS_ROUTE, ClockulaDestination.ALARMS)) + .isEqualTo(ShellBackAction.Exit) } @Test fun `back from any other tab returns to alarms`() { - val actions = listOf("timers", "stopwatch", "world_clock").map(ShellNavigation::onBack) + val actions = listOf( + ClockulaDestination.TIMERS, + ClockulaDestination.STOPWATCH, + ClockulaDestination.WORLD_CLOCK, + ).map { ShellNavigation.onBack(ShellNavigation.TABS_ROUTE, it) } - assertThat(actions).containsExactly( - ShellBackAction.GoTo(ClockulaDestination.ALARMS), - ShellBackAction.GoTo(ClockulaDestination.ALARMS), - ShellBackAction.GoTo(ClockulaDestination.ALARMS), - ) + assertThat(actions.toSet()).containsExactly(ShellBackAction.GoTo(ClockulaDestination.ALARMS)) } @Test fun `back before the first destination is known leaves the app`() { - assertThat(ShellNavigation.onBack(null)).isEqualTo(ShellBackAction.Exit) + assertThat(ShellNavigation.onBack(null, ClockulaDestination.TIMERS)).isEqualTo(ShellBackAction.Exit) } @Test - fun `back from a nested destination is not the shell's to intercept`() { - assertThat(ShellNavigation.onBack("alarms/12")).isEqualTo(ShellBackAction.Exit) + fun `back from a nested screen is not the shell's to intercept, whatever tab is under it`() { + val actions = ClockulaDestination.entries.map { ShellNavigation.onBack(SettingsRoutes.MAIN, it) } + + assertThat(actions.toSet()).containsExactly(ShellBackAction.Exit) } // --- M6 §5.15: the timer editor's route, and the notification's deep link --- - /** §5.15 #1 */ - @Test - fun `the timer editor is no tab, so the timers tab stays selected`() { - assertThat(ShellNavigation.destinationOf(TimerRoutes.editor(7L))).isNull() - } - - // --- M10: settings has no single owning tab --- - - @Test - fun `settings has no owning tab, so the caller falls back to the start destination`() { - assertThat(ShellNavigation.selectedDestinationOf(SettingsRoutes.MAIN)).isNull() - assertThat(ShellNavigation.selectedDestinationOf(SettingsRoutes.SELF_CHECK)).isNull() - assertThat(ShellNavigation.selectedDestinationOf(SettingsRoutes.BACKUP)).isNull() - } - - @Test - fun `editors highlight their parent tab`() { - assertThat(ShellNavigation.selectedDestinationOf(AlarmRoutes.EDITOR_PATTERN)) - .isEqualTo(ClockulaDestination.ALARMS) - assertThat(ShellNavigation.selectedDestinationOf(TimerRoutes.EDITOR_PATTERN)) - .isEqualTo(ClockulaDestination.TIMERS) - } - /** §5.15 #2 */ @Test fun `the shell stands aside on back, leaving the timer editor to pop itself`() { - assertThat(ShellNavigation.onBack(TimerRoutes.editor(7L))).isEqualTo(ShellBackAction.Exit) + assertThat(ShellNavigation.onBack(TimerRoutes.EDITOR_PATTERN, ClockulaDestination.TIMERS)) + .isEqualTo(ShellBackAction.Exit) } /** §5.15 #3 */ diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 3ed5097..7d8e039 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1304,27 +1304,46 @@ default rather than writing `if (compact) NavigationBar else NavigationRail` is the point — the default also answers the tabletop posture and the compact-*height* case a hand-rolled branch quietly gets wrong. -The `NavHost` is the single source of truth for the selected tab. There is no -`var selectedTab by rememberSaveable`: the selection is derived from -`currentBackStackEntryAsState()`, and Navigation Compose already saves its back -stack through a `SavedStateHandle`, so the tab survives rotation and process -death with no mirror of ours to drift. +**The tabs are pages of one `HorizontalPager` (#2).** Until then each tab was +its own `NavHost` route and the back stack was the source of truth for the +selected tab; the four tabs could only be tapped. Now they sit on a single +route, `ShellNavigation.TABS_ROUTE`, and the **pager** is the single source of +truth for the selected tab: its state is saveable, hoisted above the `NavHost` +so a tap from an editor can still move it, and the tab survives rotation and +process death with no mirror of ours to drift. The `NavHost` keeps only what is +pushed over the tabs — the two editors, the timer setup page, Settings. Each tab +is composed only while it is on screen (`beyondViewportPageCount` stays 0), keeps +its saveable state under its page key while off screen, and its ViewModel in the +tabs route's store. -The policy that back stack follows is a pure function, `ShellNavigation`: +Swiping is on only under a navigation **bar**: the bar is a horizontal row of +the same four tabs, so a sideways swipe maps onto it one to one. Beside a rail — +tablets, unfolded foldables — the tabs are stacked vertically and a sideways +swipe has no counterpart the user can see, so there the pager is tap-only. The +shell computes `NavigationSuiteScaffoldDefaults.navigationSuiteType` itself and +hands the same value to the scaffold, so the two cannot disagree. -- a tab tap is `popUpTo(start) { saveState = true }` + `launchSingleTop` + - `restoreState`; re-selecting the tab you are already on is the same command - with `restoreState = false`, which pops that tab's own inner stack back to its - root — the standard behaviour, and the contract M5's editor will rely on; -- back from any of the other three tabs returns to **Alarms**; back from Alarms, - from a null route and from any route the shell does not recognise leaves the - app — a nested destination's back belongs to the `NavController`, not to us. +The policy the shell follows is a pure function, `ShellNavigation`: + +- a tab tap on the tabs slides the pager there — from the neighbouring page, so + a jump across two tabs does not compose and flash past the ones in between; +- a tab tap over a nested screen pops back to the tabs first and jumps, and + re-selecting the tab under an editor closes the editor — the contract M5's + editor relies on. A nested screen is no longer kept per tab and restored when + its tab is re-selected: every one writes as the user goes (M5 D22), so popping + one loses nothing; +- a tab an intent asks for (a notification, a widget, the exported door) is the + same pop and a jump — it always lands on the tab itself, never on whatever + nested screen was last open there; +- back from any of the other three tabs returns to **Alarms**; back from Alarms + and from a null route leaves the app, and a nested screen's back belongs to + the `NavController`, not to us. Predictive back is the kit's `Modifier.predictiveBack`, gated on exactly that policy: on Alarms the handler is *off*, so the gesture falls through to the -system and previews leaving the app, which is the truthful preview. Tab switches -use the kit's `fadeThrough()` — peer destinations have no spatial relationship, -and a slide would claim a hierarchy that is not there. +system and previews leaving the app, which is the truthful preview. Nested +screens use the kit's `fadeThrough()` — they have no spatial relationship to the +tabs, and a slide would claim one. ### The live pill @@ -1439,6 +1458,10 @@ tab stays highlighted while the editor is open; `onBack` of that route is `Exit`, so the shell's predictive-back handler stands aside and the editor pops itself; and a tab tap's `popUpTo(start.route)` pops the editor, which is the right behaviour and exactly the contract §12 wrote down for this milestone. +Since the tabs became pager pages (#2) the list lives on the shared tabs route +instead, and the same three properties hold through the pager: it keeps Alarms +selected under the editor, the shell stands aside on a nested route's back, and +a tab tap over the editor pops it. There is **no "new alarm" sentinel id**. The FAB creates the alarm — enabled, non-repeating, at the next whole hour, every override null — and then navigates @@ -1940,7 +1963,9 @@ deep link: the content intent carries `TimerIntents.ACTION_SHOW_TIMERS`, `ShellNavigation.onTabSelected` — the same command a tab tap produces. `startDestination` stays **Alarms**: it is the `popUpTo` anchor and the root of M4's asserted back policy, so changing it for a notification tap would rewrite -that policy. +that policy. (Since #2 the shell replays `ShellNavigation.onTabRequested` +instead: a tap's pop, but a jump rather than a slide, onto the pager whose start +page is still Alarms — §12.) M9 finished that surface and widened it by one step. `tabForAction` gained exactly **one** action, `AlarmIntents.ACTION_SHOW_ALARMS`: the platform's own