ui: polish artist art, chooser navigation, and list search

Fall back to the first album credit cover (then the first track credit) for artist art, keep list searches when opening a result, resume search at the end of the query, and navigate immediately from the artist chooser.
2026-11-10 15:11:58 +01:00 · 2026-10-19 09:30:18 +00:00
parent de64e8bc69
commit 88e83c2a86
9 changed files with 229 additions and 37 deletions
@@ -163,6 +163,40 @@ internal const val ARTISTS_FILTER =
        "OR (length(:search) < 3 AND (instr(lower(name), lower(:search)) > 0 " +
        "OR instr(lower(sortName), lower(:search)) > 0))) "

+internal const val ARTIST_ALBUM_CREDIT_ART =
+    "SELECT albums.coverArt FROM (" +
+        "SELECT albums.id, albums.coverArt, albums.year, albums.sortName FROM albums INDEXED BY index_albums_sourceId_artistId " +
+        "WHERE albums.sourceId = artists.sourceId AND albums.artistId = artists.id " +
+        "UNION " +
+        "SELECT albums.id, albums.coverArt, albums.year, albums.sortName FROM album_artists " +
+        "JOIN albums ON albums.sourceId = album_artists.sourceId AND albums.id = album_artists.albumId " +
+        "WHERE album_artists.sourceId = artists.sourceId AND album_artists.artistId = artists.id" +
+        ") albums ORDER BY albums.year DESC, albums.sortName COLLATE NOCASE, albums.id LIMIT 1"
+
+internal const val ARTIST_TRACK_CREDIT_ART =
+    "SELECT albums.coverArt FROM (" +
+        "SELECT albums.id, albums.coverArt, albums.year, albums.sortName FROM songs " +
+        "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
+        "WHERE songs.sourceId = artists.sourceId AND songs.artistId = artists.id " +
+        "UNION " +
+        "SELECT albums.id, albums.coverArt, albums.year, albums.sortName FROM song_artists " +
+        "JOIN songs ON songs.sourceId = song_artists.sourceId AND songs.id = song_artists.songId " +
+        "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
+        "WHERE song_artists.sourceId = artists.sourceId AND song_artists.artistId = artists.id" +
+        ") albums ORDER BY albums.year DESC, albums.sortName COLLATE NOCASE, albums.id LIMIT 1"
+
+internal const val ARTIST_COVER_ART =
+    "COALESCE(artists.coverArt, CASE " +
+        "WHEN EXISTS (SELECT 1 FROM albums WHERE albums.sourceId = artists.sourceId AND albums.artistId = artists.id) " +
+        "OR EXISTS (SELECT 1 FROM album_artists " +
+        "WHERE album_artists.sourceId = artists.sourceId AND album_artists.artistId = artists.id) " +
+        "THEN ($ARTIST_ALBUM_CREDIT_ART) ELSE ($ARTIST_TRACK_CREDIT_ART) END)"
+
+internal const val ARTIST_COLUMNS =
+    "artists.sourceId, artists.id, artists.name, artists.albumCount, artists.starred, " +
+        "$ARTIST_COVER_ART AS coverArt, artists.playCount, artists.played, artists.songCount, " +
+        "artists.sortName, artists.credited"
+
 // With the Downloaded filter on, count the albums that actually have a download so the number under
 // an artist matches the albums its detail screen lists.
 internal const val ARTISTS_SELECT =
@@ -170,8 +204,8 @@ internal const val ARTISTS_SELECT =
        "CASE WHEN :downloadedFilter = 1 THEN (" +
        "SELECT COUNT(*) FROM ($DOWNLOADED_ARTIST_ALBUMS)" +
        ") ELSE artists.albumCount END AS albumCount, " +
-        "artists.starred, artists.coverArt, artists.playCount, artists.played, artists.songCount, artists.sortName, " +
-        "artists.credited "
+        "artists.starred, $ARTIST_COVER_ART AS coverArt, artists.playCount, artists.played, artists.songCount, " +
+        "artists.sortName, artists.credited "

 internal const val ARTIST_ORDER_BY_NAME = "sortName COLLATE NOCASE, id"
 internal const val ARTIST_ORDER_BY_NAME_REVERSED = "sortName COLLATE NOCASE DESC, id DESC"
@@ -377,7 +411,7 @@ interface LibraryDao {
    ): Flow<List<Album>>

    @Query(
-        "SELECT * FROM artists WHERE sourceId = :sourceId AND played IS NOT NULL AND $ARTIST_CREDITED " +
+        "SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND played IS NOT NULL AND $ARTIST_CREDITED " +
            "ORDER BY played DESC, sortName COLLATE NOCASE, id LIMIT :limit",
    )
    fun recentlyPlayedArtists(
@@ -395,7 +429,7 @@ interface LibraryDao {
    ): Flow<List<Album>>

    @Query(
-        "SELECT * FROM artists WHERE sourceId = :sourceId AND playCount > 0 AND $ARTIST_CREDITED " +
+        "SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND playCount > 0 AND $ARTIST_CREDITED " +
            "ORDER BY playCount DESC, sortName COLLATE NOCASE, id LIMIT :limit",
    )
    fun mostPlayedArtists(
@@ -444,7 +478,7 @@ interface LibraryDao {
    ): Flow<List<Album>>

    @Query(
-        "SELECT * FROM artists WHERE sourceId = :sourceId AND $ARTIST_SEARCH_FILTER AND $ARTIST_CREDITED " +
+        "SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND $ARTIST_SEARCH_FILTER AND $ARTIST_CREDITED " +
            "ORDER BY sortName COLLATE NOCASE, id LIMIT :limit",
    )
    fun searchArtists(
@@ -856,13 +890,13 @@ interface LibraryDao {
    fun homeRecentlyAddedAlbums(sourceId: Long): PagingSource<Int, Album>

    @Query(
-        "SELECT * FROM artists WHERE sourceId = :sourceId AND played IS NOT NULL AND $ARTIST_CREDITED " +
+        "SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND played IS NOT NULL AND $ARTIST_CREDITED " +
            "ORDER BY played DESC, sortName COLLATE NOCASE, id",
    )
    fun homeRecentlyPlayedArtists(sourceId: Long): PagingSource<Int, Artist>

    @Query(
-        "SELECT * FROM artists WHERE sourceId = :sourceId AND playCount > 0 AND $ARTIST_CREDITED " +
+        "SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND playCount > 0 AND $ARTIST_CREDITED " +
            "ORDER BY playCount DESC, sortName COLLATE NOCASE, id",
    )
    fun homeMostPlayedArtists(sourceId: Long): PagingSource<Int, Artist>
@@ -921,7 +955,7 @@ interface LibraryDao {
    fun downloadedAlbums(sourceId: Long): PagingSource<Int, Album>

    @Query(
-        "SELECT artists.* FROM artists " +
+        "SELECT $ARTIST_COLUMNS FROM artists " +
            "JOIN (SELECT id, MAX(downloadedAt) AS lastDownloadedAt FROM ($DOWNLOADED_ARTIST_ROWS) " +
            "WHERE status = 'Completed' GROUP BY id) dl ON dl.id = artists.id " +
            "WHERE artists.sourceId = :sourceId AND $ARTIST_CREDITED " +
@@ -1020,13 +1054,13 @@ interface LibraryDao {
        albumId: String,
    ): Album?

-    @Query("SELECT * FROM artists WHERE sourceId = :sourceId AND id = :artistId")
+    @Query("SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND id = :artistId")
    suspend fun artistOnce(
        sourceId: Long,
        artistId: String,
    ): Artist?

-    @Query("SELECT * FROM artists WHERE sourceId = :sourceId AND id = :artistId")
+    @Query("SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND id = :artistId")
    fun artist(
        sourceId: Long,
        artistId: String,
@@ -90,7 +90,7 @@ internal fun artistsDefaultQuery(
    descending: Boolean,
 ): RoomRawQuery {
    val order = artistOrder(sort, descending)
-    return RoomRawQuery("SELECT * $ARTISTS_FILTER_BASE ORDER BY $order") { stmt ->
+    return RoomRawQuery("SELECT $ARTIST_COLUMNS $ARTISTS_FILTER_BASE ORDER BY $order") { stmt ->
        stmt.bindLong(1, sourceId)
        stmt.bindInt(2, starredFilter)
        stmt.bindInt(3, starredFilter)
@@ -68,10 +68,8 @@ fun ArtistChooserSheet(
                    colors = ListItemDefaults.colors(containerColor = Color.Transparent),
                    modifier =
                        Modifier.clickable {
-                            scope.launch {
-                                sheetState.hide()
-                                onArtistClick(artist)
-                            }
+                            onArtistClick(artist)
+                            scope.launch { sheetState.hide() }
                        },
                )
            }
@@ -20,6 +20,10 @@ import androidx.compose.material3.Surface
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.focus.FocusRequester
@@ -28,6 +32,8 @@ import androidx.compose.ui.graphics.SolidColor
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.semantics.contentDescription
 import androidx.compose.ui.semantics.semantics
+import androidx.compose.ui.text.TextRange
+import androidx.compose.ui.text.input.TextFieldValue
 import androidx.compose.ui.text.style.TextOverflow
 import androidx.compose.ui.unit.dp
 import com.subtracks.R
@@ -43,6 +49,12 @@ fun SearchField(
 ) {
    val hint = stringResource(R.string.search_input_placeholder)
    val closeLabel = stringResource(R.string.close_search)
+    var textFieldValue by remember { mutableStateOf(TextFieldValue(value, selection = TextRange(value.length))) }
+    LaunchedEffect(value) {
+        if (value != textFieldValue.text) {
+            textFieldValue = TextFieldValue(value, selection = TextRange(value.length))
+        }
+    }
    LaunchedEffect(Unit) { focusRequester.requestFocus() }
    Surface(
        shape = RoundedCornerShape(8.dp),
@@ -62,8 +74,11 @@ fun SearchField(
            )
            Spacer(Modifier.width(8.dp))
            BasicTextField(
-                value = value,
-                onValueChange = onValueChange,
+                value = textFieldValue,
+                onValueChange = {
+                    textFieldValue = it
+                    onValueChange(it.text)
+                },
                singleLine = true,
                textStyle = MaterialTheme.typography.bodyLarge.copy(color = MaterialTheme.colorScheme.onSurface),
                cursorBrush = SolidColor(MaterialTheme.colorScheme.primary),
@@ -73,7 +88,7 @@ fun SearchField(
                        .focusRequester(focusRequester)
                        .semantics { contentDescription = hint },
                decorationBox = { inner ->
-                    if (value.isEmpty()) {
+                    if (textFieldValue.text.isEmpty()) {
                        Text(
                            text = hint,
                            style = MaterialTheme.typography.bodyLarge,
@@ -445,13 +445,6 @@ fun LibraryScreen(
        }
    }

-    fun dismissSearch() {
-        if (searchActive) {
-            searchActive = false
-            onSearchChange("")
-        }
-    }
-
    val density = LocalDensity.current
    val statusBarTop = with(density) { WindowInsets.statusBars.getTop(this).toDp() }
    val filtersActive = listQuery.starred != StarredFilter.Any || listQuery.downloaded || search.isNotEmpty()
@@ -502,10 +495,7 @@ fun LibraryScreen(
                            albums.collectAsLazyPagingItems(),
                            coverArt,
                            listBottomInset,
-                            {
-                                dismissSearch()
-                                onAlbumClick(it)
-                            },
+                            onAlbumClick,
                            onLongClick = onItemLongClick,
                            filtered = filtersActive,
                            onClearFilters = onClearFilters,
@@ -522,10 +512,7 @@ fun LibraryScreen(
                            artists.collectAsLazyPagingItems(),
                            coverArt,
                            listBottomInset,
-                            {
-                                dismissSearch()
-                                onArtistClick(it)
-                            },
+                            onArtistClick,
                            onLongClick = onItemLongClick,
                            filtered = filtersActive,
                            onClearFilters = onClearFilters,
@@ -542,10 +529,7 @@ fun LibraryScreen(
                            playlists.collectAsLazyPagingItems(),
                            coverArt,
                            listBottomInset,
-                            {
-                                dismissSearch()
-                                onPlaylistClick(it)
-                            },
+                            onPlaylistClick,
                            onLongClick = onItemLongClick,
                            filtered = filtersActive,
                            onClearFilters = onClearFilters,
@@ -24,6 +24,7 @@ import kotlinx.coroutines.test.runTest
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
@@ -545,6 +546,74 @@ class LibraryDaoTest {
            )
        }

+    @Test
+    fun artistArtFallsBackToTheFirstAlbumCreditThenTheFirstTrackCredit() =
+        runTest {
+            val sourceId = source()
+            val dao = db.libraryDao()
+            dao.upsertArtists(
+                listOf(
+                    artist(sourceId, "ar-own", "Own", albumCount = 1).copy(coverArt = "own-art"),
+                    artist(sourceId, "ar-album", "Album", albumCount = 2),
+                    artist(sourceId, "ar-track", "Track", albumCount = 1),
+                    artist(sourceId, "ar-null-art", "Null Art", albumCount = 2),
+                    artist(sourceId, "ar-none", "None", albumCount = 0),
+                ),
+            )
+            dao.upsertAlbums(
+                listOf(
+                    album(sourceId, "own", "Own", year = 2020, starred = null).copy(coverArt = "own-album"),
+                    album(sourceId, "album-new", "New", year = 2020, starred = null)
+                        .copy(artistId = "ar-album", coverArt = "new-art"),
+                    album(sourceId, "album-old", "Old", year = 2000, starred = null)
+                        .copy(artistId = "ar-album", coverArt = "old-art"),
+                    album(sourceId, "track", "Track", year = 2021, starred = null)
+                        .copy(artistId = "ar-other", coverArt = "track-art"),
+                    album(sourceId, "null-new", "Null New", year = 2022, starred = null)
+                        .copy(artistId = "ar-null-art", coverArt = null),
+                    album(sourceId, "null-old", "Null Old", year = 1990, starred = null)
+                        .copy(artistId = "ar-null-art", coverArt = "later-art"),
+                ),
+            )
+            dao.upsertSongs(
+                listOf(
+                    song(sourceId, "s-own", "Own", starred = null).copy(albumId = "own", artistId = "ar-own"),
+                    song(sourceId, "s-album", "Album", starred = null).copy(albumId = "album-new", artistId = "ar-album"),
+                    song(sourceId, "s-track", "Track", starred = null).copy(albumId = "track", artistId = "ar-track"),
+                    song(sourceId, "s-null-new", "Null New", starred = null)
+                        .copy(albumId = "null-new", artistId = "ar-null-art"),
+                    song(sourceId, "s-null-old", "Null Old", starred = null)
+                        .copy(albumId = "null-old", artistId = "ar-null-art"),
+                ),
+            )
+
+            val artists = dao.artistsByName(sourceId, 0, "").page().associateBy { it.id }
+
+            assertEquals("own-art", artists.getValue("ar-own").coverArt)
+            assertEquals("new-art", artists.getValue("ar-album").coverArt)
+            assertEquals("track-art", artists.getValue("ar-track").coverArt)
+            assertNull(artists.getValue("ar-null-art").coverArt)
+            assertNull(artists.getValue("ar-none").coverArt)
+            assertEquals("new-art", dao.artistOnce(sourceId, "ar-album")!!.coverArt)
+        }
+
+    @Test
+    fun artistArtFallbackFindsCreditsThroughArtistIndexes() =
+        runTest {
+            val plan = plan("SELECT $ARTIST_COLUMNS $ARTISTS_FILTER ORDER BY $ARTIST_ORDER_BY_NAME LIMIT 20")
+
+            assertTrue(plan, plan.contains("USING INDEX index_albums_sourceId_artistId (sourceId=? AND artistId=?)"))
+            assertTrue(
+                plan,
+                plan.contains("USING INDEX index_album_artists_sourceId_artistId (sourceId=? AND artistId=?)"),
+            )
+            assertTrue(plan, plan.contains("USING INDEX index_songs_sourceId_artistId (sourceId=? AND artistId=?)"))
+            assertTrue(
+                plan,
+                plan.contains("USING INDEX index_song_artists_sourceId_artistId (sourceId=? AND artistId=?)"),
+            )
+        }
+
    @Test
    fun albumCreditIdsFindCreditsThroughArtistIndexes() =
        runTest {
@@ -232,6 +232,46 @@ class LibraryScreenTest {
        assertTrue("the tab that was shown should collect its flow", collected.contains("artists"))
    }

+    @Test
+    fun openingAResultKeepsTheListSearchQuery() {
+        val search = mutableStateOf("xyz")
+        val show = mutableStateOf(true)
+        var opened: Album? = null
+        composeRule.setContent {
+            if (show.value) {
+                SubtracksTheme {
+                    LibraryScreen(
+                        selectedTab = LibraryTab.Albums,
+                        onTabSelected = {},
+                        albums = remember { flowOf(PagingData.from(albums())) },
+                        artists = remember { flowOf(PagingData.empty<Artist>()) },
+                        playlists = remember { flowOf(PagingData.empty<Playlist>()) },
+                        coverArt = { _, _ -> null },
+                        onAlbumClick = { opened = it },
+                        onArtistClick = {},
+                        onPlaylistClick = {},
+                        onSync = {},
+                        onOpenSettings = {},
+                        search = search.value,
+                        onSearchChange = { search.value = it },
+                    )
+                }
+            }
+        }
+
+        composeRule.waitUntil(timeoutMillis = 5_000) {
+            composeRule.onAllNodesWithTag(ALBUM_COVER_TAG).fetchSemanticsNodes().isNotEmpty()
+        }
+        composeRule.onAllNodesWithTag(ALBUM_COVER_TAG)[0].performClick()
+        composeRule.waitForIdle()
+
+        assertEquals("al-1", opened?.id)
+        assertEquals("xyz", search.value)
+
+        composeRule.runOnIdle { show.value = false }
+        composeRule.waitForIdle()
+    }
+
    @Test
    fun theTabRowReflowsInOneDirectionDuringASwitch() {
        val pagerState = PagerState(currentPage = 1, pageCount = { LibraryTab.entries.size })
@@ -45,6 +45,29 @@ class ArtistChooserSheetTest {
        assertEquals(ArtistRef("ar2", "B"), opened)
    }

+    @Test
+    fun opensTheChosenArtistBeforeTheSheetFinishesClosing() {
+        var opened: ArtistRef? = null
+        composeRule.setContent {
+            SubtracksTheme {
+                ArtistChooserSheet(
+                    artists = listOf(ArtistRef("ar1", "A"), ArtistRef("ar2", "B")),
+                    onArtistClick = { opened = it },
+                    onDismiss = {},
+                )
+            }
+        }
+
+        composeRule.mainClock.autoAdvance = false
+        composeRule.onNodeWithText("B").performClick()
+
+        assertEquals(ArtistRef("ar2", "B"), opened)
+
+        composeRule.mainClock.advanceTimeBy(1_000)
+        composeRule.mainClock.autoAdvance = true
+        composeRule.waitForIdle()
+    }
+
    @Test
    fun longListsScrollToTheLastArtist() {
        var opened: ArtistRef? = null
@@ -1,10 +1,15 @@
 package com.subtracks.ui.search

 import androidx.activity.ComponentActivity
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.setValue
 import androidx.compose.ui.test.assertIsDisplayed
+import androidx.compose.ui.test.hasSetTextAction
 import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
 import androidx.compose.ui.test.onNodeWithText
 import androidx.compose.ui.test.performClick
+import androidx.compose.ui.test.performTextInput
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.github.takahirom.roborazzi.RobolectricDeviceQualifiers
 import com.subtracks.data.model.Album
@@ -84,6 +89,30 @@ class SearchScreenTest {
        composeRule.onNodeWithText("No results", substring = true).assertIsDisplayed()
    }

+    @Test
+    fun typingResumesAtTheEndOfTheQueryAfterTheScreenIsRecreated() {
+        var query by mutableStateOf("road")
+        var show by mutableStateOf(true)
+        composeRule.setContent {
+            if (show) {
+                SubtracksTheme {
+                    SearchScreen(query = query, onQueryChange = { query = it }, results = SearchResults())
+                }
+            }
+        }
+
+        composeRule.onNode(hasSetTextAction()).performTextInput("s")
+        assertEquals("roads", query)
+
+        composeRule.runOnIdle { show = false }
+        composeRule.runOnIdle { show = true }
+        composeRule.onNode(hasSetTextAction()).performTextInput("!")
+        assertEquals("roads!", query)
+
+        composeRule.runOnIdle { show = false }
+        composeRule.waitForIdle()
+    }
+
    @Test
    fun idsSharedAcrossTypesDoNotCollide() {
        composeRule.setContent {