0.3.28 - More search fixes

app/build.gradle.kts

@@ -38,7 +38,7 @@ val membyGatewayUrl: String = (project.findProperty("memby.gatewayUrl") as Strin
 val membyDiagnosticLogLevel: String = (project.findProperty("memby.diagnosticLogLevel") as String?)
     ?.trim()?.uppercase()?.takeIf { it in setOf("INFO", "DEBUG", "TRACE") } ?: "INFO"
 
-val defaultVersionName = "0.3.27"
+val defaultVersionName = "0.3.28"
 val membyVersionName: String =
     (project.findProperty("memby.versionName") as String?)
         ?.trim()

app/src/main/java/com/ponzischeme89/memby/ui/search/SearchScreen.kt

@@ -187,11 +187,17 @@ fun SearchScreen(
             searchTracked = false
         }
     }
+    // Must name only destinations that actually exist right now: the "Finding more
+    // titles…" footer shown while state.discoveryLoading is true is a label, never a
+    // focus target, so it cannot count here on its own. Counting it was the bug — the
+    // keyboard's Right key would claim a results target that no composable had actually
+    // attached, and a library search with nothing in it (waiting on a still-loading
+    // Sonarr/Radarr lookup) left the remote unable to move off the keyboard until that
+    // lookup settled, however long it took.
     val hasResultsTarget = when {
         state.errorMessage != null && state.results.isEmpty() -> true
         state.isDiscovery -> state.genreSuggestions.isNotEmpty()
-        else -> state.results.any { it.isMovie || it.isSeries } || state.discovery.isNotEmpty() ||
+        else -> searchResultsHaveFocusTarget(state)
-            state.discoveryLoading
     }
 
     LaunchedEffect(voiceAvailable) { runCatching { contentFocusRequester.requestFocus() } }
@@ -269,7 +275,19 @@ fun SearchScreen(
         SearchPane(
             state = state,
             navigationFocusRequester = navigationFocusRequester,
-            resultsEntry = if (state.genreSuggestions.isNotEmpty()) genresEntry else searchResultsEntry,
+            // The genre strip is a discovery-mode shortcut, not a waypoint into search
+            // results: routing Right through it only while nothing is typed (or browsed)
+            // means an active search always reaches its results in exactly one press,
+            // regardless of whether the genre suggestions have finished loading yet. Making
+            // this depend on genreSuggestions alone meant the very first search — before
+            // Home's rows had warmed the suggestion list — went straight to results, while
+            // every search after that (once the list populated) gained an extra hop through
+            // the strip that the viewer had no reason to expect.
+            resultsEntry = if (state.isDiscovery && state.genreSuggestions.isNotEmpty()) {
+                genresEntry
+            } else {
+                searchResultsEntry
+            },
             keyboardEntry = keyboardEntry,
             keyboardReturn = keyboardReturn,
             lastKeyIndex = lastKeyIndex,
@@ -303,9 +321,7 @@ fun SearchScreen(
                 entryFocusRequester = genresEntry,
                 keyboardReturnFocusRequester = keyboardReturn,
                 resultsFocusRequester = searchResultsEntry,
-                resultsHaveFocusTarget = state.results.any { it.isMovie || it.isSeries } ||
+                resultsHaveFocusTarget = searchResultsHaveFocusTarget(state),
-                    (state.errorMessage != null && state.results.isEmpty()) ||
-                    state.discovery.isNotEmpty(),
                 onFocused = {
                     focusInResults = true
                     onContentFocused()
@@ -868,6 +884,23 @@ private fun LoadingDot() {
     )
 }
 
+/**
+ * Whether the results pane, as [SearchResults] would render [state] right now, has any
+ * composable actually willing to take focus.
+ *
+ * This is the one rule both the keyboard's rightward escape and the genre strip's
+ * downward one must agree with — a destination named to either of them that this
+ * function does not also call reachable is a D-pad key that quietly does nothing. It
+ * deliberately excludes [SearchUiState.discoveryLoading] and [SearchUiState.isLoadingMore]
+ * and their footer notes: those are text, not focus targets, and a search whose library
+ * half came back empty while Sonarr/Radarr was still being asked must not claim a
+ * destination that will not exist until that lookup settles.
+ */
+internal fun searchResultsHaveFocusTarget(state: SearchUiState): Boolean =
+    state.results.any { it.isMovie || it.isSeries } ||
+        (state.errorMessage != null && state.results.isEmpty()) ||
+        state.discovery.isNotEmpty()
+
 private const val KEYBOARD_COLUMNS = 6
 private const val ACTION_ROW_INDEX = 36
 

app/src/test/java/com/ponzischeme89/memby/ui/search/SearchFocusTest.kt

@@ -0,0 +1,59 @@
+package com.ponzischeme89.memby.ui.search
+
+import com.ponzischeme89.memby.data.model.BaseItem
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/**
+ * Pins [searchResultsHaveFocusTarget]: whatever it reports must always match whether
+ * [SearchResults] actually attaches a focus requester to something. A search whose
+ * library half came back empty while Sonarr/Radarr was still being asked used to report
+ * a target it had not built yet — the "Finding more titles…" footer is a label, not a
+ * focus destination — which left the keyboard's Right key pointed at nothing and the
+ * remote stuck unable to leave it until that lookup settled.
+ */
+class SearchFocusTest {
+
+    private fun movie(id: String) = BaseItem(id = id, name = id, type = "Movie")
+
+    @Test
+    fun `discovery loading with nothing else does not claim a target`() {
+        val state = SearchUiState(
+            query = "zqx",
+            hasSearched = true,
+            discoveryLoading = true,
+        )
+        assertFalse(searchResultsHaveFocusTarget(state))
+    }
+
+    @Test
+    fun `library results claim a target`() {
+        val state = SearchUiState(query = "reacher", results = listOf(movie("1")))
+        assertTrue(searchResultsHaveFocusTarget(state))
+    }
+
+    @Test
+    fun `discovery results alone claim a target`() {
+        val state = SearchUiState(query = "reacher", discovery = listOf(movie("1")))
+        assertTrue(searchResultsHaveFocusTarget(state))
+    }
+
+    @Test
+    fun `an error with nothing found claims a target for its retry chip`() {
+        val state = SearchUiState(query = "reacher", errorMessage = "Could not reach the server.")
+        assertTrue(searchResultsHaveFocusTarget(state))
+    }
+
+    @Test
+    fun `an empty result with nothing loading claims no target`() {
+        val state = SearchUiState(query = "reacher", hasSearched = true)
+        assertFalse(searchResultsHaveFocusTarget(state))
+    }
+
+    @Test
+    fun `loading more pages does not by itself claim a target`() {
+        val state = SearchUiState(query = "reacher", isLoadingMore = true)
+        assertFalse(searchResultsHaveFocusTarget(state))
+    }
+}