diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/MainActivity.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/MainActivity.kt index 58603b2..8cf8775 100644 --- a/android/app/src/main/java/com/fabledsword/thoughtsync/MainActivity.kt +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/MainActivity.kt @@ -12,6 +12,7 @@ import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState +import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource @@ -81,8 +82,13 @@ private fun App(core: ThoughtSync) { // Sheet and screen visibility are view STATE, not view-model state: they are // about what is on the display, and nothing in the store cares. - var composing by remember { mutableStateOf(false) } - var showingSync by remember { mutableStateOf(false) } + // + // Saveable, though: `remember` alone meant rotating the phone closed whatever + // was open and took the half-written note in the capture sheet with it. The + // editor never had that problem because the note it is on lives in a view + // model; these two are the only screen state that did not. + var composing by rememberSaveable { mutableStateOf(false) } + var showingSync by rememberSaveable { mutableStateOf(false) } val settings = remember(context) { SyncSettings(context) } var automatic by remember { mutableStateOf(settings.automatic) } diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/BoardScreen.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/BoardScreen.kt index a934ce2..2a68a0e 100644 --- a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/BoardScreen.kt +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/BoardScreen.kt @@ -11,7 +11,6 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size -import androidx.compose.foundation.layout.statusBarsPadding import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.staggeredgrid.LazyVerticalStaggeredGrid import androidx.compose.foundation.lazy.staggeredgrid.StaggeredGridCells @@ -195,10 +194,14 @@ private fun SearchBar( onMenu: () -> Unit, ) { Surface( + // No `statusBarsPadding()` here. The Scaffold this sits in already applies + // the system-bar insets to its content padding, so adding them again put + // the whole status bar's height of empty space above the search field — + // roughly a centimetre of nothing at the top of the first screen anyone + // sees. Insets get consumed once, by whichever component owns the edge. modifier = Modifier .fillMaxWidth() - .statusBarsPadding() .padding(horizontal = GUTTER, vertical = 8.dp), shape = RoundedCornerShape(SEARCH_RADIUS), color = MaterialTheme.colorScheme.surfaceVariant, diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/ComposeSheet.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/ComposeSheet.kt index bfd4c4c..721b275 100644 --- a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/ComposeSheet.kt +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/ComposeSheet.kt @@ -19,6 +19,7 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.focus.FocusRequester @@ -37,6 +38,20 @@ import com.fabledsword.thoughtsync.R * It asks note-or-list up front rather than making that a mode you discover later, * because on a phone the two are genuinely different typing tasks and switching * halfway is worse than choosing at the start. + * + * ## Leaving keeps what you wrote + * + * Every way out of this sheet except Discard SAVES: the save button, tapping the + * board behind it, swiping down, back, and the app being backgrounded. A sheet + * that throws away a typed thought because you touched outside it is a sheet that + * teaches people not to trust the app with a thought — and capture is the one + * place this product cannot afford that. + * + * The same shape the editor settled on, for the same reason, with one difference: + * capture also has to be abandonable, because tapping + and changing your mind is + * a normal thing to do. That is what Discard is, and it is the only path that + * loses anything. An empty draft needs neither — it is simply dropped, since a + * blank note nobody asked for is worse than no note at all. */ @OptIn(ExperimentalMaterial3Api::class) @Composable @@ -46,17 +61,27 @@ fun ComposeSheet( onSave: (DraftKind, String, String) -> Unit, ) { val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) - var kind by remember { mutableStateOf(DraftKind.NOTE) } - var title by remember { mutableStateOf("") } - var content by remember { mutableStateOf("") } + // Saveable, not just remembered: a rotation mid-sentence is the same lost + // thought as a discarded one, and it was losing it before this. + var kind by rememberSaveable { mutableStateOf(DraftKind.NOTE) } + var title by rememberSaveable { mutableStateOf("") } + var content by rememberSaveable { mutableStateOf("") } val contentFocus = remember { FocusRequester() } + val written = title.isNotBlank() || content.isNotBlank() + val leave = { if (written) onSave(kind, title, content) else onDismiss() } + // Land in the body, not the title. Most captures are a thought, not a titled // document, and making someone tab past an optional field is the difference // between "under a second" and not. LaunchedEffect(Unit) { contentFocus.requestFocus() } - ModalBottomSheet(onDismissRequest = onDismiss, sheetState = sheetState) { + // Backgrounding PERSISTS but does not close an empty sheet. Someone who tapped + // + and then got distracted should find the composer where they left it; the + // only reason to act here is that there is something to lose. + FlushOnStop { if (written) onSave(kind, title, content) } + + ModalBottomSheet(onDismissRequest = leave, sheetState = sheetState) { Column( modifier = Modifier @@ -100,8 +125,8 @@ fun ComposeSheet( ) SheetActions( - canSave = !saving && (title.isNotBlank() || content.isNotBlank()), - onCancel = onDismiss, + canSave = !saving && written, + onDiscard = onDismiss, onSave = { onSave(kind, title, content) }, ) } @@ -111,14 +136,17 @@ fun ComposeSheet( @Composable private fun SheetActions( canSave: Boolean, - onCancel: () -> Unit, + onDiscard: () -> Unit, onSave: () -> Unit, ) { Row( modifier = Modifier.fillMaxWidth().padding(bottom = 16.dp), horizontalArrangement = Arrangement.End, ) { - TextButton(onClick = onCancel) { Text(stringResource(R.string.compose_cancel)) } + // "Discard", not "Cancel". Cancel means "undo what I am doing", which is + // precisely what leaving no longer does — the word would now describe the + // one button it is NOT attached to. + TextButton(onClick = onDiscard) { Text(stringResource(R.string.compose_discard)) } Button(onClick = onSave, enabled = canSave) { Text(stringResource(R.string.compose_save)) } diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/FlushOnStop.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/FlushOnStop.kt new file mode 100644 index 0000000..6e7c308 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/FlushOnStop.kt @@ -0,0 +1,36 @@ +package com.fabledsword.thoughtsync.ui + +import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.rememberUpdatedState +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleEventObserver +import androidx.lifecycle.compose.LocalLifecycleOwner + +/** + * Run [flush] when the app goes to the background. + * + * `ON_STOP` rather than `ON_PAUSE`: pause also fires when a dialog opens over the + * activity, which would save mid-sentence for no reason. The lambda goes through + * `rememberUpdatedState` so the observer — registered once — always calls the + * CURRENT one; captured directly it would hold the first composition's empty text + * forever and save that over a full note. + * + * Shared by the editor and the capture sheet. Both are places where text exists + * only in a composable until something writes it down, and the process can be + * killed while backgrounded without either of them being told again. + */ +@Composable +fun FlushOnStop(flush: () -> Unit) { + val current by rememberUpdatedState(flush) + val owner = LocalLifecycleOwner.current + DisposableEffect(owner) { + val observer = + LifecycleEventObserver { _, event -> + if (event == Lifecycle.Event.ON_STOP) current() + } + owner.lifecycle.addObserver(observer) + onDispose { owner.lifecycle.removeObserver(observer) } + } +} diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteEditorScreen.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteEditorScreen.kt index a2f953e..9281fda 100644 --- a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteEditorScreen.kt +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteEditorScreen.kt @@ -24,19 +24,14 @@ import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar import androidx.compose.material3.TopAppBarDefaults import androidx.compose.runtime.Composable -import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.dp -import androidx.lifecycle.Lifecycle -import androidx.lifecycle.LifecycleEventObserver -import androidx.lifecycle.compose.LocalLifecycleOwner import com.fabledsword.thoughtsync.R import com.fabledsword.thoughtsync.core.Label import com.fabledsword.thoughtsync.core.Note @@ -254,29 +249,6 @@ private fun EditorOverlays( } } -/** - * Run [flush] when the app goes to the background. - * - * `ON_STOP` rather than `ON_PAUSE`: pause also fires when a dialog opens over the - * activity, which would save mid-sentence for no reason. The lambda goes through - * `rememberUpdatedState` so the observer — registered once — always calls the - * CURRENT one; captured directly it would hold the first composition's empty text - * forever and save that over a full note. - */ -@Composable -private fun FlushOnStop(flush: () -> Unit) { - val current by rememberUpdatedState(flush) - val owner = LocalLifecycleOwner.current - DisposableEffect(owner) { - val observer = - LifecycleEventObserver { _, event -> - if (event == Lifecycle.Event.ON_STOP) current() - } - owner.lifecycle.addObserver(observer) - onDispose { owner.lifecycle.removeObserver(observer) } - } -} - /** * The title and body fields. * diff --git a/android/app/src/main/res/values/strings.xml b/android/app/src/main/res/values/strings.xml index e8a4c46..148fb79 100644 --- a/android/app/src/main/res/values/strings.xml +++ b/android/app/src/main/res/values/strings.xml @@ -15,7 +15,7 @@ Title Take a note… One item per line - Cancel + Discard Save