From f6e087b6abd2924ccc6bc1018e30d33d43b81e0b Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sat, 3 Oct 2026 10:25:47 +0000 Subject: [PATCH] search: list playlists before artists, albums and songs Order the result sections smallest-first and apply the review fixes: pad the filter bar for the navigation bar, keep at least one filter enabled, hide the clear button on an empty field, drop the unused autoFocus and bar-modifier parameters, and test the filter mask and section order. --- .../subtracks/ui/components/SearchField.kt | 18 ++++--- .../com/subtracks/ui/search/SearchScreen.kt | 22 +++++--- .../subtracks/ui/search/SearchScreenTest.kt | 54 +++++++++++++++++++ 3 files changed, 78 insertions(+), 16 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/ui/components/SearchField.kt b/app/src/main/kotlin/com/subtracks/ui/components/SearchField.kt index bdffa969..64a68ad8 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/SearchField.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/SearchField.kt @@ -39,11 +39,11 @@ fun SearchField( onClose: () -> Unit, focusRequester: FocusRequester, modifier: Modifier = Modifier, - autoFocus: Boolean = true, + showClose: Boolean = true, ) { val hint = stringResource(R.string.search_input_placeholder) val closeLabel = stringResource(R.string.close_search) - LaunchedEffect(autoFocus) { if (autoFocus) focusRequester.requestFocus() } + LaunchedEffect(Unit) { focusRequester.requestFocus() } Surface( shape = RoundedCornerShape(8.dp), color = MaterialTheme.colorScheme.surfaceContainerHigh, @@ -85,12 +85,14 @@ fun SearchField( inner() }, ) - IconButton(onClick = onClose) { - Icon( - imageVector = Icons.Rounded.Close, - contentDescription = closeLabel, - modifier = Modifier.size(20.dp), - ) + if (showClose) { + IconButton(onClick = onClose) { + Icon( + imageVector = Icons.Rounded.Close, + contentDescription = closeLabel, + modifier = Modifier.size(20.dp), + ) + } } } } diff --git a/app/src/main/kotlin/com/subtracks/ui/search/SearchScreen.kt b/app/src/main/kotlin/com/subtracks/ui/search/SearchScreen.kt index 79f3e19e..2bb9befd 100644 --- a/app/src/main/kotlin/com/subtracks/ui/search/SearchScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/search/SearchScreen.kt @@ -8,6 +8,7 @@ import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.navigationBarsPadding import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListScope @@ -98,7 +99,10 @@ fun SearchRoute( onQueryChange = viewModel::setQuery, results = results, filters = maskToFilters(filterMask), - onToggleFilter = { filterMask = filterMask xor (1 shl it.ordinal) }, + onToggleFilter = { filter -> + val next = filterMask xor (1 shl filter.ordinal) + if (next != 0) filterMask = next + }, coverArt = viewModel::coverArt, playingSongId = playback.item?.id, onBack = onBack, @@ -110,9 +114,10 @@ fun SearchRoute( ) } -private fun allFilterMask(): Int = SearchFilter.entries.fold(0) { mask, filter -> mask or (1 shl filter.ordinal) } +internal fun allFilterMask(): Int = SearchFilter.entries.fold(0) { mask, filter -> mask or (1 shl filter.ordinal) } -private fun maskToFilters(mask: Int): Set = SearchFilter.entries.filterTo(mutableSetOf()) { mask and (1 shl it.ordinal) != 0 } +internal fun maskToFilters(mask: Int): Set = + SearchFilter.entries.filterTo(mutableSetOf()) { mask and (1 shl it.ordinal) != 0 } @OptIn(ExperimentalMaterial3Api::class) @Composable @@ -158,6 +163,7 @@ fun SearchScreen( onValueChange = onQueryChange, onClose = { onQueryChange("") }, focusRequester = focusRequester, + showClose = query.isNotEmpty(), modifier = Modifier.fillMaxWidth().padding(vertical = 8.dp), ) }, @@ -191,10 +197,10 @@ fun SearchScreen( modifier = Modifier.fillMaxSize().padding(padding), contentPadding = PaddingValues(bottom = 24.dp), ) { - if (showSongs) songResults(results.songs, songsLabel, coverArt, playingSongId, onSongClick, onLongClick) - if (showAlbums) albumResults(results.albums, albumsLabel, coverArt, onAlbumClick, onLongClick) - if (showArtists) artistResults(results.artists, artistsLabel, coverArt, onArtistClick, onLongClick) if (showPlaylists) playlistResults(results.playlists, playlistsLabel, coverArt, onPlaylistClick, onLongClick) + if (showArtists) artistResults(results.artists, artistsLabel, coverArt, onArtistClick, onLongClick) + if (showAlbums) albumResults(results.albums, albumsLabel, coverArt, onAlbumClick, onLongClick) + if (showSongs) songResults(results.songs, songsLabel, coverArt, playingSongId, onSongClick, onLongClick) } } } @@ -205,12 +211,12 @@ fun SearchScreen( private fun SearchFilterBar( filters: Set, onToggle: (SearchFilter) -> Unit, - modifier: Modifier = Modifier, ) { Row( modifier = - modifier + Modifier .fillMaxWidth() + .navigationBarsPadding() .horizontalScroll(rememberScrollState()) .padding(horizontal = 12.dp, vertical = 8.dp), horizontalArrangement = Arrangement.spacedBy(8.dp), diff --git a/app/src/test/kotlin/com/subtracks/ui/search/SearchScreenTest.kt b/app/src/test/kotlin/com/subtracks/ui/search/SearchScreenTest.kt index 870b6aae..e732ecf8 100644 --- a/app/src/test/kotlin/com/subtracks/ui/search/SearchScreenTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/search/SearchScreenTest.kt @@ -14,6 +14,7 @@ import com.subtracks.data.model.Playlist import com.subtracks.data.model.Song import com.subtracks.ui.theme.SubtracksTheme import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -147,6 +148,59 @@ class SearchScreenTest { assertEquals(SearchFilter.Playlists, toggled) } + @Test + fun smallerGroupsAreListedFirst() { + composeRule.setContent { + SubtracksTheme { + SearchScreen( + query = "road", + onQueryChange = {}, + results = + SearchResults( + albums = listOf(album("al-1", "Road Album")), + artists = listOf(artist("ar-1", "Road Artist")), + playlists = listOf(playlist("pl-1", "Road Trip")), + songs = listOf(AlbumSongItem(song("s1", "Road Song"), coverArt = null)), + ), + ) + } + } + + val playlistTop = + composeRule + .onNodeWithText("Road Trip") + .fetchSemanticsNode() + .boundsInRoot.top + val artistTop = + composeRule + .onNodeWithText("Road Artist") + .fetchSemanticsNode() + .boundsInRoot.top + val albumTop = + composeRule + .onNodeWithText("Road Album") + .fetchSemanticsNode() + .boundsInRoot.top + val songTop = + composeRule + .onNodeWithText("Road Song") + .fetchSemanticsNode() + .boundsInRoot.top + + assertTrue("playlists should come first", playlistTop < artistTop) + assertTrue("artists should come before albums", artistTop < albumTop) + assertTrue("albums should come before songs", albumTop < songTop) + } + + @Test + fun filterMaskRoundTrips() { + assertEquals(SearchFilter.entries.toSet(), maskToFilters(allFilterMask())) + + val withoutSongs = allFilterMask() and (1 shl SearchFilter.Songs.ordinal).inv() + + assertEquals(SearchFilter.entries.toSet() - SearchFilter.Songs, maskToFilters(withoutSongs)) + } + private fun album( id: String, name: String,