null
/
Privacy-Period-Tracker
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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) <noreply@anthropic.com>
This commit is contained in:
null 2026-08-20 21:49:57 -05:00
parent ca7a187520
commit a12d8e5c49
12 changed files with 490 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/build.gradle.kts
View File

@ -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)
}

91
app/src/main/kotlin/dev/privacyllc/period/feature/common/SettingsSubpage.kt Normal file
View File

@ -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() }
}
}
}

11
app/src/main/kotlin/dev/privacyllc/period/feature/export/ExportHost.kt
View File

@ -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,

20
app/src/main/kotlin/dev/privacyllc/period/feature/export/ExportScreen.kt
View File

@ -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(

117
app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsScreen.kt
View File

@ -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) {

10
app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsScreen.kt
View File

@ -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
}
}
// 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

82
app/src/main/kotlin/dev/privacyllc/period/navigation/PeriodApp.kt
View File

@ -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,21 +95,47 @@ 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 = {
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) { saveState = true }
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) },
// maxLines and ellipsis, because at the largest
@ -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) {
// 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() }
composable(SETTINGS_LOCK) { LockSettingsScreen() }
composable(SETTINGS_EXPORT) { ExportScreen() }
composable(SETTINGS_NOTIFICATIONS) {
NotificationSettingsScreen(onNavigateBack = { navController.popBackStack() })
}
composable(SETTINGS_LOCK) {
LockSettingsScreen(onNavigateBack = { navController.popBackStack() })
}
composable(SETTINGS_EXPORT) {
ExportScreen(onNavigateBack = { navController.popBackStack() })
}
}
}
}
}
@ -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

3
app/src/main/res/values/strings.xml
View File

@ -19,6 +19,9 @@
<string name="tab_insights">Insights</string>
<string name="tab_settings">Settings</string>
<!-- The back arrow on every screen that is not a tab. One string, one meaning. -->
<string name="action_back">Back</string>
<!--
One copy, read by every surface that shows a fertility estimate.

57
app/src/test/kotlin/dev/privacyllc/period/feature/common/SettingsSubpageTest.kt Normal file
View File

@ -0,0 +1,57 @@
package dev.privacyllc.period.feature.common
import androidx.activity.ComponentActivity
import androidx.compose.material3.Text
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.junit4.createAndroidComposeRule
import androidx.compose.ui.test.onNodeWithContentDescription
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import dev.privacyllc.period.R
import dev.privacyllc.period.designsystem.PeriodTheme
import org.junit.Assert.assertEquals
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
/**
* The one piece of chrome every screen below a tab shares.
*
* Before it, a grep for `TopAppBar`, `navigationIcon` and `popBackStack` across
* the whole app returned nothing: no screen had a way back that a user could
* see, and App lock had a headline that looked like a bar without being one.
* This asserts the arrow exists, says "Back" to a screen reader, and does
* something when tapped — the three things a label cannot do.
*/
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [34])
class SettingsSubpageTest {
@get:Rule val rule = createAndroidComposeRule<ComponentActivity>()
private val back get() = rule.activity.getString(R.string.action_back)
@Test fun `it offers a back arrow that a screen reader can name`() {
rule.setContent { PeriodTheme { SettingsSubpage("App lock", onBack = {}) { Text("body") } } }
rule.onNodeWithContentDescription(back).assertIsDisplayed()
}
@Test fun `tapping the arrow goes back exactly once`() {
var backs = 0
rule.setContent { PeriodTheme { SettingsSubpage("App lock", onBack = { backs++ }) { Text("body") } } }
rule.onNodeWithContentDescription(back).performClick()
assertEquals(1, backs)
}
@Test fun `it names where you are, and still shows what you came for`() {
rule.setContent { PeriodTheme { SettingsSubpage("Reminders and privacy", onBack = {}) { Text("body") } } }
rule.onNodeWithText("Reminders and privacy").assertIsDisplayed()
rule.onNodeWithText("body").assertIsDisplayed()
}
}

90
app/src/test/kotlin/dev/privacyllc/period/feature/lock/LockSettingsBackTest.kt Normal file
View File

@ -0,0 +1,90 @@
package dev.privacyllc.period.feature.lock
import androidx.activity.ComponentActivity
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.junit4.createAndroidComposeRule
import androidx.compose.ui.test.onNodeWithContentDescription
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import dev.privacyllc.period.R
import dev.privacyllc.period.designsystem.PeriodTheme
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
/**
* Back, on a screen whose steps the navigation graph knows nothing about.
*
* App lock keeps its step in remembered state rather than in a destination, so
* the system gesture would leave the screen entirely from "choose a PIN" while
* the arrow beside it stepped back one — two controls a hand's width apart doing
* different things. Both are asserted here, at both depths.
*
* The last assertion is the subtle one: at the Overview the handler must be
* *disabled*, so the gesture falls through to the NavHost and pops the
* destination. A handler that stayed enabled and did nothing would be a
* different trap in the same place.
*/
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [34])
class LockSettingsBackTest {
@get:Rule val rule = createAndroidComposeRule<ComponentActivity>()
private val back get() = rule.activity.getString(R.string.action_back)
private var backs = 0
private fun setUpAtOverview() {
rule.setContent {
PeriodTheme {
LockSettingsContent(state = LockSettings(hasPin = false), onNavigateBack = { backs++ })
}
}
}
private fun systemBack() {
rule.runOnUiThread { rule.activity.onBackPressedDispatcher.onBackPressed() }
rule.waitForIdle()
}
@Test fun `the gesture is left alone at the first step, so it can leave the screen`() {
setUpAtOverview()
// Nothing of ours is registered, so back means what it means everywhere
// else: pop the destination.
assertFalse(rule.activity.onBackPressedDispatcher.hasEnabledCallbacks())
}
@Test fun `the arrow leaves the screen from the first step`() {
setUpAtOverview()
rule.onNodeWithContentDescription(back).performClick()
assertEquals(1, backs)
}
@Test fun `inside a step, both the gesture and the arrow step back rather than leaving`() {
setUpAtOverview()
rule.onNodeWithText("Set a PIN").performClick()
rule.onNodeWithText("Before you set a PIN").assertIsDisplayed()
assertTrue(rule.activity.onBackPressedDispatcher.hasEnabledCallbacks())
systemBack()
rule.onNodeWithText("Set a PIN").assertIsDisplayed()
assertEquals("the gesture left App lock instead of stepping back", 0, backs)
assertFalse(rule.activity.onBackPressedDispatcher.hasEnabledCallbacks())
// And the arrow, from the same step, does the same thing.
rule.onNodeWithText("Set a PIN").performClick()
rule.onNodeWithContentDescription(back).performClick()
rule.onNodeWithText("Set a PIN").assertIsDisplayed()
assertEquals(0, backs)
}
}

33
docs/design/README.md
View File

@ -153,6 +153,39 @@ have replaced it.
## The lock screen is the one semi-public surface
### Back is a promise, and the top bar is where it is kept
**Every screen that is not a tab has a top bar with a back arrow, and it is one
component — `SettingsSubpage` — not three copies.** Before it, a grep for
`TopAppBar`, `navigationIcon` and `popBackStack` across the whole app returned
nothing: no screen had a visible way back, and App lock had a headline styled
like a bar without being one, so the affordance a user reached for was a label.
**Back returns to the screen it came from, never to the app root.** A screen with
internal steps makes back step-aware, and the arrow and the system gesture do the
same thing at the same step: from a step, back to the previous step; from the
first step, out of the screen. Inside a step the gesture is intercepted; at the
first step it is deliberately *not*, so it falls through and pops the destination
exactly as the arrow does.
**Re-selecting a tab returns it to its root**, and the Settings tab always lands
on the §36 tree. That is not a nicety: settings children are one-shot tasks, so
leaving Settings pops its stack without saving it. Saving it is what produced the
bug this section exists for — the tab restored the child you were trying to
leave, and kept doing it for the rest of the process.
Three versions were tried and rejected. Making Settings a nested graph *alone*
fixes which tab looks selected and nothing else; the tab tap still saved and
restored the child. Dropping `restoreState` alone leaves the stack saved on the
way out, so nav-scoped ViewModels are retained per exit and the tab still reads
as unselected on children. And having the arrow dispatch the system back gesture
is one code path, but it makes the arrow mean "whatever happens to be registered"
rather than "go back".
The title lives in the bar and carries the heading semantics; the body does not
repeat it. Two headings saying the same thing is one for a screen reader to wade
through and one more thing to keep in step.
### A step that asks a question waits for the answer
Both confirmations in App lock — "turn the lock off" and "change PIN" — used to

6
gradle/libs.versions.toml
View File

@ -44,6 +44,12 @@ androidx-lifecycle-viewmodel-compose = { group = "androidx.lifecycle", name = "l
androidx-navigation-compose = { group = "androidx.navigation", name = "navigation-compose", version.ref = "navigationCompose" }
compose-bom = { group = "androidx.compose", name = "compose-bom", version.ref = "composeBom" }
# Compose tests, version-managed by the BOM above like every other Compose
# artifact. Added for the back-navigation guards: the trap they pin — a tab that
# restored you into the screen you were trying to leave — is a navigation
# behaviour, and nothing in this project could assert one before.
compose-ui-test-junit4 = { group = "androidx.compose.ui", name = "ui-test-junit4" }
compose-ui-test-manifest = { group = "androidx.compose.ui", name = "ui-test-manifest" }
compose-ui = { group = "androidx.compose.ui", name = "ui" }
compose-ui-graphics = { group = "androidx.compose.ui", name = "ui-graphics" }
compose-ui-tooling = { group = "androidx.compose.ui", name = "ui-tooling" }