diff --git a/app/build.gradle.kts b/app/build.gradle.kts index fdae99f..0ab3a53 100644 --- a/app/build.gradle.kts +++ b/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() diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchScreen.kt b/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchScreen.kt index 8cbd1d5..4db1cae 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchScreen.kt +++ b/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() || - state.discoveryLoading + else -> searchResultsHaveFocusTarget(state) } 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 } || - (state.errorMessage != null && state.results.isEmpty()) || - state.discovery.isNotEmpty(), + resultsHaveFocusTarget = searchResultsHaveFocusTarget(state), 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 diff --git a/app/src/test/java/com/ponzischeme89/memby/ui/search/SearchFocusTest.kt b/app/src/test/java/com/ponzischeme89/memby/ui/search/SearchFocusTest.kt new file mode 100644 index 0000000..4798787 --- /dev/null +++ b/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)) + } +}