fix(editor): bound the settings read on note open, and test the two rules that had none (#204)

app/src/main/java/com/markleaf/notes/feature/editor/EditorScreen.kt

@@ -110,6 +110,7 @@ import kotlinx.coroutines.flow.collectLatest
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.withContext
+import kotlinx.coroutines.withTimeoutOrNull
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
 import kotlin.math.ceil
@@ -138,6 +139,9 @@ fun EditorScreen(
     var shouldRequestEditorFocus by remember(noteId) { mutableStateOf(noteId == null) }
     val editorFocusRequester = remember(noteId) { FocusRequester() }
     var isPreviewMode by remember(noteId) { mutableStateOf(false) }
+    // True when the settings read timed out and the note opened on defaults.
+    // The position recorder consults it — see [recordsPosition] (#204).
+    var openedOnFallbackSettings by remember(noteId) { mutableStateOf(false) }
     var isFocusMode by remember(noteId) { mutableStateOf(false) }
     var isFormattingExpanded by remember(noteId) { mutableStateOf(false) }
     var showDeleteConfirm by remember(noteId) { mutableStateOf(false) }
@@ -316,7 +320,17 @@ fun EditorScreen(
             // snapshot, which starts on the default before DataStore emits) so
             // an existing note honours "open notes in preview" on its very first
             // frame instead of flashing edit (#200). New notes stay in edit.
-            val persistedSettings = settingsRepository.settings.first()
+            //
+            // Bounded, because this suspends before the note is shown at all: a
+            // DataStore read that never returns would hang note-open with no
+            // way out, and the whole point of reading it here is a detail of
+            // which mode the note opens in (#204). Falling back to the defaults
+            // opens in edit at the top, which is the safe answer — it shows the
+            // note and puts the caret somewhere harmless.
+            val readSettings =
+                withTimeoutOrNull(SETTINGS_READ_TIMEOUT_MS) { settingsRepository.settings.first() }
+            openedOnFallbackSettings = readSettings == null
+            val persistedSettings = readSettings ?: AppSettings()
             val openInPreview = persistedSettings.openNotesInPreview
             val loadedNote = repo.getNote(noteId)
             val content = loadedNote?.contentMarkdown.orEmpty()
@@ -346,12 +360,7 @@ fun EditorScreen(
                 OpenNotesAt.LAST_POSITION ->
                     lastPosition?.let { PreviewScrollRequest(it.previewIndex, animate = false) }
             }
-            // Only land in preview when there is something to preview. A
-            // brand-new note is created first and then opened with a real id
-            // (the FAB path in MarkleafNavHost), so `noteId != null` alone would
-            // drop an empty note into preview with no editor/IME — honour "new
-            // notes still open for editing" by gating on content (#200).
-            isPreviewMode = openInPreview && content.isNotEmpty()
+            isPreviewMode = opensInPreview(openInPreview, content)
             shouldRequestEditorFocus = content.isEmpty()
             isLoaded = true
             // Remember this note as the launch target for the opt-in
@@ -444,9 +453,9 @@ fun EditorScreen(
     // write at the end of it rather than one per keystroke, and written while
     // the screen is still composed so it survives the process being killed
     // rather than depending on a tidy exit.
-    LaunchedEffect(noteId, isLoaded, appSettings.openNotesAt) {
+    LaunchedEffect(noteId, isLoaded, appSettings.openNotesAt, openedOnFallbackSettings) {
         if (noteId == null || !isLoaded) return@LaunchedEffect
-        if (appSettings.openNotesAt != OpenNotesAt.LAST_POSITION) return@LaunchedEffect
+        if (!recordsPosition(appSettings.openNotesAt, openedOnFallbackSettings)) return@LaunchedEffect
         snapshotFlow {
             Triple(
                 isPreviewMode,
@@ -1132,6 +1141,52 @@ private const val MAX_TAG_SUGGESTIONS = 8
  */
 private const val POSITION_WRITE_DEBOUNCE_MS = 1_000L
 
+/**
+ * How long the note-open path waits for DataStore before opening on defaults.
+ *
+ * Generous rather than tight: the read normally completes in milliseconds, and
+ * a device slow enough to need a second is a device where opening on the wrong
+ * mode is worse than waiting. This is a stuck-forever guard, not a latency
+ * budget (#204).
+ */
+private const val SETTINGS_READ_TIMEOUT_MS = 2_000L
+
+/**
+ * Whether a note opens in preview: only when the setting is on *and* the note
+ * has something to preview.
+ *
+ * The content check is not a nicety. A brand-new note is created first and then
+ * opened with a real id (the FAB path in `MarkleafNavHost`), so the id alone
+ * cannot tell a new note from an existing one — without this an empty note
+ * would land in preview with no editor and no keyboard, and "new notes still
+ * open for editing" would be broken (#200).
+ *
+ * Extracted so that rule has a test rather than living only in a load effect
+ * that needs a device to exercise (#204).
+ */
+internal fun opensInPreview(openNotesInPreview: Boolean, content: String): Boolean =
+    openNotesInPreview && content.isNotEmpty()
+
+/**
+ * Whether this screen may record where the note was left.
+ *
+ * The setting has to be on, and — the part that is easy to miss — the note must
+ * not have opened on fallback settings. When the settings read times out the
+ * note opens at the top on defaults; DataStore may then emit the real settings
+ * a moment later, flipping `openNotesAt` to `LAST_POSITION` and starting the
+ * recorder from that fallback state. Its first debounced write would store
+ * caret 0 and overwrite the position the user actually left, without them
+ * having touched anything (#204).
+ *
+ * Skipping the write is the conservative answer: we could not read what the
+ * user asked for, so we do not overwrite what we already knew. Reopening the
+ * note takes the normal path.
+ */
+internal fun recordsPosition(
+    openNotesAt: OpenNotesAt,
+    openedOnFallbackSettings: Boolean
+): Boolean = openNotesAt == OpenNotesAt.LAST_POSITION && !openedOnFallbackSettings
+
 /**
  * A preview scroll waiting for the preview to be built. [animate] separates the
  * two callers: restoring where a note was left should already be there when the

app/src/test/java/com/markleaf/notes/feature/editor/OpenModeTest.kt

@@ -0,0 +1,72 @@
+package com.markleaf.notes.feature.editor
+
+import com.markleaf.notes.data.settings.OpenNotesAt
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/**
+ * Which mode a note opens in when "open notes in preview" is on (#200).
+ *
+ * The rule lived inline in the editor's load effect, where the only thing
+ * exercising it was someone opening a note on a device (#204). It is one
+ * boolean and one emptiness check, and getting it wrong is not subtle — a new
+ * note that opens in preview has no editor and no keyboard, so the FAB appears
+ * to do nothing.
+ */
+class OpenModeTest {
+
+    @Test
+    fun `setting off always opens in edit`() {
+        assertFalse(opensInPreview(openNotesInPreview = false, content = "# A note"))
+        assertFalse(opensInPreview(openNotesInPreview = false, content = ""))
+    }
+
+    @Test
+    fun `an existing note opens in preview when the setting is on`() {
+        assertTrue(opensInPreview(openNotesInPreview = true, content = "# A note"))
+    }
+
+    /**
+     * The case the guard exists for. The FAB creates the note first and opens
+     * it with a real id, so the id cannot distinguish a new note from an
+     * existing one — only its content can.
+     */
+    @Test
+    fun `a just-created note opens in edit even with the setting on`() {
+        assertFalse(opensInPreview(openNotesInPreview = true, content = ""))
+    }
+
+    /**
+     * Whitespace is content: the user typed it, and a note holding a newline
+     * has something the preview can render. Emptiness is the "brand new"
+     * signal, not blankness.
+     */
+    @Test
+    fun `whitespace counts as content`() {
+        assertTrue(opensInPreview(openNotesInPreview = true, content = "\n"))
+        assertTrue(opensInPreview(openNotesInPreview = true, content = " "))
+    }
+
+    // ---- position recording ----
+
+    @Test
+    fun `position is recorded only when the setting selects it`() {
+        assertTrue(recordsPosition(OpenNotesAt.LAST_POSITION, openedOnFallbackSettings = false))
+        assertFalse(recordsPosition(OpenNotesAt.TOP, openedOnFallbackSettings = false))
+        assertFalse(recordsPosition(OpenNotesAt.BOTTOM, openedOnFallbackSettings = false))
+    }
+
+    /**
+     * The trap the timeout introduced. A note that opened on fallback settings
+     * sits at caret 0; if DataStore then emits the real settings and flips
+     * `openNotesAt` to LAST_POSITION, the recorder would start from that
+     * fallback state and its first debounced write would store 0 over the
+     * position the user actually left — without them touching anything.
+     */
+    @Test
+    fun `a note opened on fallback settings never records its position`() {
+        assertFalse(recordsPosition(OpenNotesAt.LAST_POSITION, openedOnFallbackSettings = true))
+        assertFalse(recordsPosition(OpenNotesAt.TOP, openedOnFallbackSettings = true))
+    }
+}

app/src/testDebug/java/com/markleaf/notes/feature/editor/EditorTopAppBarLockTest.kt

@@ -26,15 +26,20 @@ class EditorTopAppBarLockTest {
     @get:Rule
     val composeRule = createComposeRule()
 
-    @Test
-    fun tapFlipsPreviewAndLongPressFlipsLockIndependently() {
-        var previewToggles = 0
-        var lockToggles = 0
+    private var previewToggles = 0
+    private var lockToggles = 0
+
+    /**
+     * Renders the bar and returns the view-toggle node. In edit mode the toggle
+     * offers "Preview"; in preview mode it offers "Edit" — the label names the
+     * destination, not the current state.
+     */
+    private fun toggle(isPreviewMode: Boolean = false, isViewModeLocked: Boolean = false) = run {
         composeRule.setContent {
             MarkleafTheme(dynamicColor = false) {
                 EditorTopAppBar(
                     title = "New Note",
-                    isPreviewMode = false,
+                    isPreviewMode = isPreviewMode,
                     isFocusMode = false,
                     showMore = true,
                     moreExpanded = false,
@@ -46,14 +51,17 @@ class EditorTopAppBarLockTest {
                     onOpenMore = {},
                     onDismissMore = {},
                     moreMenuContent = {},
-                    isViewModeLocked = false,
+                    isViewModeLocked = isViewModeLocked,
                     onToggleLock = { lockToggles++ }
                 )
             }
         }
+        composeRule.onNodeWithContentDescription(if (isPreviewMode) "Edit" else "Preview")
+    }
 
-        // While in edit mode the toggle offers "Preview".
-        val toggle = composeRule.onNodeWithContentDescription("Preview")
+    @Test
+    fun tapFlipsPreviewAndLongPressFlipsLockIndependently() {
+        val toggle = toggle()
 
         toggle.performClick()
         assertEquals(1, previewToggles)
@@ -63,4 +71,69 @@ class EditorTopAppBarLockTest {
         assertEquals(1, lockToggles)
         assertEquals(1, previewToggles)
     }
+
+    /**
+     * The other order, which is the one at risk. The lock gesture runs in the
+     * Initial pass and consumes the release so the underlying IconButton does
+     * not also see a tap — if that consumption leaked past its own gesture, the
+     * *next* tap would be swallowed and the button would look dead.
+     */
+    @Test
+    fun aTapAfterALongPressStillFlipsPreview() {
+        val toggle = toggle()
+
+        toggle.performTouchInput { longClick() }
+        assertEquals(1, lockToggles)
+        assertEquals(0, previewToggles)
+
+        toggle.performClick()
+        assertEquals(1, previewToggles)
+        assertEquals(1, lockToggles)
+    }
+
+    /** Locking pins the mode; it does not disable the control that flips it. */
+    @Test
+    fun tapAndLongPressStillWorkWhileLocked() {
+        val toggle = toggle(isViewModeLocked = true)
+
+        toggle.performClick()
+        assertEquals(1, previewToggles)
+
+        // Long-pressing a locked toggle unlocks it — same gesture, same handler.
+        toggle.performTouchInput { longClick() }
+        assertEquals(1, lockToggles)
+        assertEquals(1, previewToggles)
+    }
+
+    /**
+     * Repeated long-presses each register. The lock is a toggle, so a second
+     * press has to reach the handler rather than being eaten as a repeat.
+     */
+    @Test
+    fun repeatedLongPressesEachFire() {
+        val toggle = toggle()
+
+        toggle.performTouchInput { longClick() }
+        toggle.performTouchInput { longClick() }
+        toggle.performTouchInput { longClick() }
+
+        assertEquals(3, lockToggles)
+        assertEquals(0, previewToggles)
+    }
+
+    /**
+     * The control keeps its label in preview mode, which is what a screen
+     * reader announces before either gesture. Losing it would leave the lock
+     * reachable only by sighted trial and error.
+     */
+    @Test
+    fun theToggleIsReachableByItsLabelInPreviewMode() {
+        val toggle = toggle(isPreviewMode = true)
+
+        toggle.performTouchInput { longClick() }
+        assertEquals(1, lockToggles)
+
+        toggle.performClick()
+        assertEquals(1, previewToggles)
+    }
 }