From a12d8e5c4940ac0337445bd56c464c2b3ad9ceb3 Mon Sep 17 00:00:00 2001 From: null Date: Thu, 20 Aug 2026 21:49:57 -0500 Subject: [PATCH] fix: give every settings page a way out Reported: "on the settings page, if you go to app lock you can't unnavigate out of it." Two causes, both real. The bottom nav saved and restored per-tab back stacks. From App lock, tapping the Settings tab popped [settings, settings/lock], saved it, and restored it in the same breath -- landing back on App lock. The saved stack survived visiting other tabs, so the Settings tab stayed pinned to App lock for the rest of the process. The KDoc above the NavHost claimed the opposite. And no screen in the app had a back arrow. A grep for TopAppBar, navigationIcon, BackHandler and popBackStack across app/, core/ and domain/ returned nothing at all. App lock had a headline styled like a bar without being one, so the affordance a user reaches for was a label, and after setting a PIN the only button on screen -- "Done" -- cleared a message and navigated nowhere. Settings is now a nested graph. Its children are inside the tab's hierarchy, so the tab renders as selected on App lock rather than looking unselected and inviting the tap that trapped you; and re-tapping the tab you are already on pops to its root, which is the gesture people reach for. Leaving Settings pops without saving, so there is nothing to restore. Today, Calendar and Insights keep their place exactly as before. One SettingsSubpage component carries the bar for all three children. Three copies would drift -- one would get the ellipsis for long titles at font scale 2.0 and the others would wrap mid-word, which is a defect the tab labels already shipped once. App lock's steps are remembered state, not destinations, so its back is step-aware: inside a step the arrow and the system gesture both return to the overview, and at the overview the handler is disabled so the gesture falls through and pops the destination, exactly as the arrow does. Two controls a hand's width apart now do the same thing. ExportHost moves into the Scaffold's topBar. It was a sibling emitted BEFORE the Scaffold inside PeriodTheme's Surface -- a Box, where later siblings draw over earlier ones -- so an opaque Scaffold was painted on top of it. It has almost certainly never been visible to anyone. Compose UI tests run on the JVM under Robolectric; nothing in this project could assert a navigation behaviour before. Proved with prove-guard, one red each: unwiring the arrow, and removing the step-aware BackHandler. Verified on the emulator: from App lock, tapping Settings now lands on the settings tree, the Settings tab is highlighted while on a child, and the arrow returns. closes #61 Co-Authored-By: Claude Opus 5 (1M context) --- app/build.gradle.kts | 3 + .../period/feature/common/SettingsSubpage.kt | 91 ++++++++++++++ .../period/feature/export/ExportHost.kt | 11 +- .../period/feature/export/ExportScreen.kt | 20 +-- .../period/feature/lock/LockSettingsScreen.kt | 117 +++++++++++++++--- .../settings/NotificationSettingsScreen.kt | 12 +- .../privacyllc/period/navigation/PeriodApp.kt | 98 ++++++++++++--- app/src/main/res/values/strings.xml | 3 + .../feature/common/SettingsSubpageTest.kt | 57 +++++++++ .../feature/lock/LockSettingsBackTest.kt | 90 ++++++++++++++ docs/design/README.md | 33 +++++ gradle/libs.versions.toml | 6 + 12 files changed, 490 insertions(+), 51 deletions(-) create mode 100644 app/src/main/kotlin/dev/privacyllc/period/feature/common/SettingsSubpage.kt create mode 100644 app/src/test/kotlin/dev/privacyllc/period/feature/common/SettingsSubpageTest.kt create mode 100644 app/src/test/kotlin/dev/privacyllc/period/feature/lock/LockSettingsBackTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 75a3a82..40cda40 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -109,6 +109,9 @@ dependencies { testImplementation(libs.kotlinx.coroutines.test) testImplementation(libs.robolectric) testImplementation(libs.androidx.test.core) + testImplementation(libs.compose.ui.test.junit4) + // Supplies the ComponentActivity the compose test rule launches into. + debugImplementation(libs.compose.ui.test.manifest) androidTestImplementation(libs.androidx.test.junit) androidTestImplementation(libs.androidx.espresso.core) } diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/common/SettingsSubpage.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/common/SettingsSubpage.kt new file mode 100644 index 0000000..957e17b --- /dev/null +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/common/SettingsSubpage.kt @@ -0,0 +1,91 @@ +package dev.privacyllc.period.feature.common + +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.WindowInsets +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.automirrored.filled.ArrowBack +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton +import androidx.compose.material3.Surface +import androidx.compose.material3.Text +import androidx.compose.material3.TopAppBar +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.heading +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.text.style.TextOverflow +import dev.privacyllc.period.R + +/** + * The chrome every screen below a tab shares: a bar naming the row the user + * tapped, and an arrow back to it. + * + * ## Why this exists as a component rather than three copies + * + * Until this file, no screen in the app had a top bar, a back arrow, or any + * navigation-controller call at all — a grep for `TopAppBar`, `navigationIcon` + * and `popBackStack` across the whole tree returned nothing. Every settings + * child relied on the system gesture, and App lock *looked* like it had a bar + * (a full-bleed Surface with a headline `Text` at the top) without having one. + * A user who reaches for the affordance she can see finds it is a label. + * + * Three copies of a bar would drift: one would get the ellipsis for long titles + * at font scale 2.0 and the others would wrap mid-word, which is the defect the + * tab labels already shipped once. One component cannot drift. + * + * ## Insets + * + * The bar declines window insets deliberately. `PeriodApp`'s `Scaffold` already + * pads its content region for the status bar and hands that down as + * `innerPadding`, so a bar that added `TopAppBarDefaults.windowInsets` here + * would pad it a second time and float below a gap. That makes this composable + * dependent on its host, which is worth saying out loud: it belongs inside that + * Scaffold, and putting it on a full-screen route would tuck the arrow under + * the status bar. + */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun SettingsSubpage( + title: String, + onBack: () -> Unit, + modifier: Modifier = Modifier, + content: @Composable () -> Unit, +) { + Surface(modifier = modifier.fillMaxSize()) { + Column(Modifier.fillMaxSize()) { + TopAppBar( + title = { + Text( + title, + maxLines = 1, + // Same reasoning as the tab labels: at the largest + // accessibility font scale Compose wraps mid-word, and + // "App loc / k" is worse than "App lock". + overflow = TextOverflow.Ellipsis, + // A screen reader can then jump to it, which is how + // somebody using one finds out where they are. + modifier = Modifier.semantics { heading() }, + ) + }, + navigationIcon = { + IconButton(onClick = onBack) { + Icon( + Icons.AutoMirrored.Filled.ArrowBack, + // AutoMirrored so the arrow turns around in a + // right-to-left layout rather than pointing the + // wrong way. + contentDescription = stringResource(R.string.action_back), + ) + } + }, + windowInsets = WindowInsets(0), + ) + Box(Modifier.weight(1f).fillMaxWidth()) { content() } + } + } +} diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/export/ExportHost.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/export/ExportHost.kt index 86f4227..01d914e 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/feature/export/ExportHost.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/export/ExportHost.kt @@ -2,6 +2,7 @@ package dev.privacyllc.period.feature.export import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.statusBarsPadding import androidx.compose.foundation.layout.padding import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Surface @@ -58,7 +59,15 @@ fun ExportHost(viewModel: ExportViewModel = hiltViewModel()) { } ?: return Surface(color = MaterialTheme.colorScheme.surfaceVariant) { - Column(Modifier.fillMaxWidth().padding(horizontal = 24.dp, vertical = 12.dp)) { + // The banner is the Scaffold's top bar, so nothing above it pads for the + // status bar: the Surface paints edge to edge and the text clears the + // clock itself. + Column( + Modifier + .fillMaxWidth() + .statusBarsPadding() + .padding(horizontal = 24.dp, vertical = 12.dp), + ) { Text( message, style = MaterialTheme.typography.bodyMedium, diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/export/ExportScreen.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/export/ExportScreen.kt index 8996681..00803ec 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/feature/export/ExportScreen.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/export/ExportScreen.kt @@ -23,6 +23,7 @@ import androidx.compose.ui.unit.dp import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import dev.privacyllc.period.designsystem.PeriodTheme +import dev.privacyllc.period.feature.common.SettingsSubpage /** * The disclosure, and one button. @@ -39,29 +40,30 @@ import dev.privacyllc.period.designsystem.PeriodTheme * result above the nav host instead, where they actually land. */ @Composable -fun ExportScreen(viewModel: ExportViewModel = hiltViewModel()) { +fun ExportScreen(onNavigateBack: () -> Unit, viewModel: ExportViewModel = hiltViewModel()) { val outcome by viewModel.outcome.collectAsStateWithLifecycle() ExportScreenContent( working = outcome == ExportOutcome.WRITING, onExport = viewModel::begin, + onNavigateBack = onNavigateBack, ) } @Composable -internal fun ExportScreenContent(working: Boolean, onExport: () -> Unit) { - Surface(modifier = Modifier.fillMaxSize()) { +internal fun ExportScreenContent( + working: Boolean, + onExport: () -> Unit, + onNavigateBack: () -> Unit = {}, +) { + // The title moves into the bar, which carries the heading semantics now — + // two headings saying the same thing is one for a screen reader to skip. + SettingsSubpage(title = ExportCopy.TITLE, onBack = onNavigateBack) { Column( modifier = Modifier .fillMaxSize() .verticalScroll(rememberScrollState()) .padding(24.dp), ) { - Text( - ExportCopy.TITLE, - style = MaterialTheme.typography.headlineMedium, - modifier = Modifier.semantics { heading() }, - ) - Section(ExportCopy.WHAT_HEADING, ExportCopy.WHAT_BODY) Section(ExportCopy.WHERE_HEADING, ExportCopy.WHERE_BODY) Section( diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsScreen.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsScreen.kt index 27e74da..faa7d19 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsScreen.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsScreen.kt @@ -20,6 +20,7 @@ import androidx.compose.material3.Surface import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.material3.TextButton +import androidx.activity.compose.BackHandler import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -39,6 +40,7 @@ import androidx.compose.ui.unit.dp import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import dev.privacyllc.period.designsystem.PeriodTheme +import dev.privacyllc.period.feature.common.SettingsSubpage private enum class Mode { OVERVIEW, SET_FIRST, CONFIRM_TO_REMOVE, CONFIRM_TO_CHANGE, SET_REPLACEMENT } @@ -52,25 +54,106 @@ private enum class Mode { OVERVIEW, SET_FIRST, CONFIRM_TO_REMOVE, CONFIRM_TO_CHA * nothing yet to authenticate against. */ @Composable -fun LockSettingsScreen(viewModel: LockSettingsViewModel = hiltViewModel()) { +fun LockSettingsScreen( + onNavigateBack: () -> Unit, + viewModel: LockSettingsViewModel = hiltViewModel(), +) { val state by viewModel.state.collectAsStateWithLifecycle() + LockSettingsContent( + state = state, + onNavigateBack = onNavigateBack, + onSetPin = viewModel::setPin, + onAuthoriseChange = viewModel::authoriseChange, + onCancelChange = viewModel::cancelChange, + onRemovePin = viewModel::removePin, + onBiometric = viewModel::setBiometricEnabled, + onDismissMessage = viewModel::clearMessage, + ) +} + +/** + * The screen without Hilt, so its back behaviour can be asserted. + * + * The steps below are remembered state rather than destinations, which is + * exactly why they need a test: nothing in the navigation graph knows they + * exist, so nothing but this file makes the arrow and the system gesture agree + * about them. + */ +@Composable +internal fun LockSettingsContent( + state: LockSettings, + onNavigateBack: () -> Unit, + onSetPin: (CharArray) -> Unit = {}, + onAuthoriseChange: (CharArray, () -> Unit) -> Unit = { _, _ -> }, + onCancelChange: () -> Unit = {}, + onRemovePin: (CharArray, () -> Unit) -> Unit = { _, _ -> }, + onBiometric: (Boolean) -> Unit = {}, + onDismissMessage: () -> Unit = {}, +) { var mode by remember { mutableStateOf(Mode.OVERVIEW) } + // One definition of "back", used by the arrow and by the system gesture. + // + // They have to agree. This screen has steps the navigation graph knows + // nothing about — `mode` is remembered state, not a destination — so system + // back from "choose a PIN" would otherwise leave App lock entirely while + // the arrow beside it stepped back one. Two controls a hand's width apart, + // doing different things. + val toOverview: () -> Unit = { + // Abandoning a step abandons the permission it was granted, too. + onCancelChange() + onDismissMessage() + mode = Mode.OVERVIEW + } + val stepBack: () -> Unit = { if (mode == Mode.OVERVIEW) onNavigateBack() else toOverview() } + + // Enabled only inside a step: at the Overview the gesture falls through to + // the NavHost, which pops the destination — exactly what the arrow does + // there. Nothing is swallowed. + BackHandler(enabled = mode != Mode.OVERVIEW, onBack = toOverview) + + SettingsSubpage(title = "App lock", onBack = stepBack) { + LockSettingsSteps( + mode = mode, + state = state, + onSetPin = onSetPin, + onAuthoriseChange = onAuthoriseChange, + onCancelChange = onCancelChange, + onRemovePin = onRemovePin, + onBiometric = onBiometric, + onDismissMessage = onDismissMessage, + setMode = { mode = it }, + ) + } +} + +@Composable +private fun LockSettingsSteps( + mode: Mode, + state: LockSettings, + onSetPin: (CharArray) -> Unit, + onAuthoriseChange: (CharArray, () -> Unit) -> Unit, + onCancelChange: () -> Unit, + onRemovePin: (CharArray, () -> Unit) -> Unit, + onBiometric: (Boolean) -> Unit, + onDismissMessage: () -> Unit, + setMode: (Mode) -> Unit, +) { when (mode) { Mode.OVERVIEW -> Overview( state = state, - onSetPin = { mode = Mode.SET_FIRST }, - onChangePin = { mode = Mode.CONFIRM_TO_CHANGE }, - onRemovePin = { mode = Mode.CONFIRM_TO_REMOVE }, - onBiometric = viewModel::setBiometricEnabled, - onDismissMessage = viewModel::clearMessage, + onSetPin = { setMode(Mode.SET_FIRST) }, + onChangePin = { setMode(Mode.CONFIRM_TO_CHANGE) }, + onRemovePin = { setMode(Mode.CONFIRM_TO_REMOVE) }, + onBiometric = onBiometric, + onDismissMessage = onDismissMessage, ) Mode.SET_FIRST, Mode.SET_REPLACEMENT -> PinSetupScreen( busy = state.busy, failed = state.message == LockSettings.Message.COULD_NOT_SET, - onCancel = { mode = Mode.OVERVIEW; viewModel.cancelChange(); viewModel.clearMessage() }, - onConfirmed = { pin -> viewModel.setPin(pin); mode = Mode.OVERVIEW }, + onCancel = { onCancelChange(); onDismissMessage(); setMode(Mode.OVERVIEW) }, + onConfirmed = { pin -> onSetPin(pin); setMode(Mode.OVERVIEW) }, ) // Both confirmations stay on screen until the PIN is actually right. @@ -86,16 +169,16 @@ fun LockSettingsScreen(viewModel: LockSettingsViewModel = hiltViewModel()) { title = "Enter your PIN to turn the lock off", busy = state.busy, wrong = state.message == LockSettings.Message.WRONG_PIN, - onCancel = { mode = Mode.OVERVIEW; viewModel.clearMessage() }, - onSubmit = { pin -> viewModel.removePin(pin) { mode = Mode.OVERVIEW } }, + onCancel = { onDismissMessage(); setMode(Mode.OVERVIEW) }, + onSubmit = { pin -> onRemovePin(pin) { setMode(Mode.OVERVIEW) } }, ) Mode.CONFIRM_TO_CHANGE -> ConfirmPin( title = "Enter your current PIN", busy = state.busy, wrong = state.message == LockSettings.Message.WRONG_PIN, - onCancel = { mode = Mode.OVERVIEW; viewModel.cancelChange(); viewModel.clearMessage() }, - onSubmit = { pin -> viewModel.authoriseChange(pin) { mode = Mode.SET_REPLACEMENT } }, + onCancel = { onCancelChange(); onDismissMessage(); setMode(Mode.OVERVIEW) }, + onSubmit = { pin -> onAuthoriseChange(pin) { setMode(Mode.SET_REPLACEMENT) } }, ) } } @@ -116,13 +199,9 @@ private fun Overview( .verticalScroll(rememberScrollState()) .padding(vertical = 16.dp), ) { - Text( - "App lock", - style = MaterialTheme.typography.headlineSmall, - modifier = Modifier.padding(horizontal = 24.dp), - ) - - Spacer(Modifier.height(8.dp)) + // No heading here: the bar above carries it, and it carries the + // heading semantics too. Two of them is one for a screen reader to + // wade through and one more thing to keep in step. Text( if (state.hasPin) { diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsScreen.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsScreen.kt index b3bae52..c373353 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsScreen.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsScreen.kt @@ -36,6 +36,7 @@ import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import dev.privacyllc.period.core.datastore.NotificationPrivacy import dev.privacyllc.period.core.datastore.UserPreferences +import dev.privacyllc.period.feature.common.SettingsSubpage import dev.privacyllc.period.designsystem.PeriodTheme import java.time.LocalTime import java.time.format.DateTimeFormatter @@ -49,7 +50,10 @@ import java.time.format.DateTimeFormatter * the screen changed. */ @Composable -fun NotificationSettingsScreen(viewModel: NotificationSettingsViewModel = hiltViewModel()) { +fun NotificationSettingsScreen( + onNavigateBack: () -> Unit, + viewModel: NotificationSettingsViewModel = hiltViewModel(), +) { val prefs by viewModel.state.collectAsStateWithLifecycle() val needsPermission by viewModel.needsPermission.collectAsStateWithLifecycle() @@ -68,7 +72,11 @@ fun NotificationSettingsScreen(viewModel: NotificationSettingsViewModel = hiltVi } } - NotificationSettingsContent(prefs, viewModel) + // The title is the Settings row she tapped to get here, so the bar names + // where she is in the words she chose it by. + SettingsSubpage(title = "Reminders and privacy", onBack = onNavigateBack) { + NotificationSettingsContent(prefs, viewModel) + } } @Composable diff --git a/app/src/main/kotlin/dev/privacyllc/period/navigation/PeriodApp.kt b/app/src/main/kotlin/dev/privacyllc/period/navigation/PeriodApp.kt index 61345fe..3fe90e4 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/navigation/PeriodApp.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/navigation/PeriodApp.kt @@ -29,7 +29,9 @@ import androidx.compose.ui.tooling.preview.Preview import androidx.navigation.NavDestination.Companion.hierarchy import androidx.navigation.NavGraph.Companion.findStartDestination import androidx.navigation.compose.NavHost +import androidx.navigation.NavDestination import androidx.navigation.compose.composable +import androidx.navigation.compose.navigation import androidx.navigation.compose.currentBackStackEntryAsState import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle @@ -93,20 +95,46 @@ fun PeriodApp() { val backStackEntry by navController.currentBackStackEntryAsState() val currentDestination = backStackEntry?.destination - ExportHost() + val inSettings = currentDestination.isIn(PeriodDestination.SETTINGS) Scaffold( + // The export result belongs above everything, including a sub-page's own + // bar. It used to be a sibling emitted BEFORE the Scaffold — and + // PeriodTheme's Surface is a Box, where later siblings draw on top of + // earlier ones, so an opaque Scaffold was painted over it. It has + // almost certainly never been seen; the export's on-device checks were + // never run either. + topBar = { ExportHost() }, bottomBar = { NavigationBar { PeriodDestination.entries.forEach { destination -> - val selected = currentDestination?.hierarchy?.any { it.route == destination.route } == true + val selected = currentDestination.isIn(destination) NavigationBarItem( selected = selected, onClick = { - navController.navigate(destination.route) { - popUpTo(navController.graph.findStartDestination().id) { saveState = true } - launchSingleTop = true - restoreState = true + if (selected) { + // Re-tapping the tab you are already on returns + // it to its root. For three tabs that is a + // no-op; for Settings it is the way out of a + // child, and the thing a user reaches for when + // a screen has no other exit. + navController.popBackStack(destination.rootRoute, inclusive = false) + } else { + navController.navigate(destination.route) { + popUpTo(navController.graph.findStartDestination().id) { + // Today, Calendar and Insights keep their + // place across a tab switch. Settings must + // not: its children are one-shot tasks, and + // a saved stack is restored on the way back + // in — which is how tapping Settings from + // inside App lock landed on App lock again, + // for the rest of the process. Nothing is + // saved, so there is nothing to restore. + saveState = !inSettings + } + launchSingleTop = true + restoreState = true + } } }, icon = { Icon(destination.icon, contentDescription = null) }, @@ -139,21 +167,32 @@ fun PeriodApp() { composable(PeriodDestination.CALENDAR.route) { CalendarScreen() } composable(PeriodDestination.INSIGHTS.route) { InsightsScreen() } - // Settings is a root with children, so the tab lands on the §36 - // tree and reminders live one level down. The tab item's own - // navigate() pops back to this start destination, which is what - // makes tapping Settings from inside reminders return here rather - // than doing nothing. - composable(PeriodDestination.SETTINGS.route) { - SettingsScreen( - onOpenNotifications = { navController.navigate(SETTINGS_NOTIFICATIONS) }, - onOpenAppLock = { navController.navigate(SETTINGS_LOCK) }, - onOpenExport = { navController.navigate(SETTINGS_EXPORT) }, - ) + // Settings is a nested graph, not a single destination. + // + // Two things fall out of that, and both were broken without it. The + // tab's route now names the graph, so every child is inside the + // tab's hierarchy and the tab renders as selected while you are on + // App lock — before, no tab looked selected on any settings child, + // and the user tapped an unselected Settings tab to escape. And the + // graph gives the re-tap above a root to pop back to. + navigation(route = PeriodDestination.SETTINGS.route, startDestination = SETTINGS_ROOT) { + composable(SETTINGS_ROOT) { + SettingsScreen( + onOpenNotifications = { navController.navigate(SETTINGS_NOTIFICATIONS) }, + onOpenAppLock = { navController.navigate(SETTINGS_LOCK) }, + onOpenExport = { navController.navigate(SETTINGS_EXPORT) }, + ) + } + composable(SETTINGS_NOTIFICATIONS) { + NotificationSettingsScreen(onNavigateBack = { navController.popBackStack() }) + } + composable(SETTINGS_LOCK) { + LockSettingsScreen(onNavigateBack = { navController.popBackStack() }) + } + composable(SETTINGS_EXPORT) { + ExportScreen(onNavigateBack = { navController.popBackStack() }) + } } - composable(SETTINGS_NOTIFICATIONS) { NotificationSettingsScreen() } - composable(SETTINGS_LOCK) { LockSettingsScreen() } - composable(SETTINGS_EXPORT) { ExportScreen() } } } } @@ -203,6 +242,12 @@ private fun PlaceholderPreview() { * screen itself may be long gone, because returning from the save dialog can * re-lock the app and the user unlocks onto Today. */ +/** + * The §36 tree itself. Distinct from the Settings *graph*, whose route is the + * tab's — one names where the tab points, the other what it shows first. + */ +private const val SETTINGS_ROOT = "settings/root" + /** Reminder settings, a child of the Settings tab rather than a fifth tab. */ private const val SETTINGS_NOTIFICATIONS = "settings/notifications" @@ -211,3 +256,16 @@ private const val SETTINGS_LOCK = "settings/lock" /** Export, above Delete in §36's order: the offer of a copy comes before the erase. */ private const val SETTINGS_EXPORT = "settings/export" + +/** True while [tab] is anywhere in this destination's hierarchy, children included. */ +private fun NavDestination?.isIn(tab: PeriodDestination) = + this?.hierarchy?.any { it.route == tab.route } == true + +/** + * Where re-selecting a tab returns to. + * + * Only Settings has children, so only Settings distinguishes the two; for the + * others the graph route and the screen route are the same thing. + */ +private val PeriodDestination.rootRoute + get() = if (this == PeriodDestination.SETTINGS) SETTINGS_ROOT else route diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 5115e13..22746c5 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -19,6 +19,9 @@ Insights Settings + + Back +