From bb71f4889d130fad9585d1752cd6c951f8e08d55 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:14:01 +0000 Subject: [PATCH] ui: finish search focus and artist art review follow-ups Avoid re-requesting search focus when the screen is restored, keep the cheaper album-credit-first art lookup, and test restoration plus association-table invalidation. --- .../com/subtracks/data/db/LibraryDao.kt | 35 +++++++++++-------- .../subtracks/ui/components/SearchField.kt | 2 +- .../com/subtracks/ui/library/LibraryScreen.kt | 6 +++- .../com/subtracks/ui/search/SearchScreen.kt | 5 ++- .../com/subtracks/data/db/LibraryDaoTest.kt | 26 ++++++++++++++ .../ui/components/ArtistChooserSheetTest.kt | 2 +- .../subtracks/ui/search/SearchScreenTest.kt | 20 ++++++++++- 7 files changed, 77 insertions(+), 19 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt index 6e448133..a0f5f26f 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -165,28 +165,35 @@ 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_COVER_ART = - "COALESCE(artists.coverArt, (" + - "SELECT credit_albums.coverArt FROM (" + - "SELECT 0 AS creditRank, albums.id, albums.coverArt, albums.year, albums.sortName FROM albums " + +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 0, albums.id, albums.coverArt, albums.year, albums.sortName FROM album_artists " + + "UNION ALL " + + "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 " + - "UNION " + - "SELECT 1, albums.id, albums.coverArt, albums.year, albums.sortName FROM songs " + + "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 1, albums.id, albums.coverArt, albums.year, albums.sortName FROM song_artists " + + "UNION ALL " + + "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" + - ") credit_albums " + - "ORDER BY credit_albums.creditRank, credit_albums.year DESC, credit_albums.sortName COLLATE NOCASE, " + - "credit_albums.id LIMIT 1))" + ") 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, " + 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 98516ace..153f2482 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/SearchField.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/SearchField.kt @@ -57,7 +57,7 @@ fun SearchField( } } var focusRequested by rememberSaveable { mutableStateOf(false) } - LaunchedEffect(focusRequested) { + LaunchedEffect(Unit) { if (!focusRequested) { focusRequester.requestFocus() focusRequested = true diff --git a/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt index 1193c8dd..c7292728 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt @@ -273,6 +273,7 @@ fun LibraryRoute( val downloadsState = listTab?.let { viewModel.downloads(it).collectAsStateWithLifecycle() } val downloads = downloadsState?.value ?: emptyMap() val scope = rememberCoroutineScope() + val focusManager = LocalFocusManager.current var pendingDelete by remember { mutableStateOf(null) } DismissOnRequest(contextMenuHost?.dismissals ?: emptyFlow()) { pendingDelete = null } val itemActions = @@ -312,7 +313,10 @@ fun LibraryRoute( onSync = viewModel::sync, onOpenSettings = onOpenSettings, onOpenSearch = onOpenSearch, - onItemLongClick = { contextMenuHost?.show(it, itemActions) }, + onItemLongClick = { + focusManager.clearFocus() + contextMenuHost?.show(it, itemActions) + }, listQuery = displayQuery, resetKeys = resetKeys, offline = offline, 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 e36d6601..75f4cef1 100644 --- a/app/src/main/kotlin/com/subtracks/ui/search/SearchScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/search/SearchScreen.kt @@ -124,7 +124,10 @@ fun SearchRoute( focusManager.clearFocus() viewModel.play(it) }, - onLongClick = { contextMenuHost?.show(it, actions) }, + onLongClick = { + focusManager.clearFocus() + contextMenuHost?.show(it, actions) + }, ) } diff --git a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt index b4fb063f..7b5cd1fb 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt @@ -677,6 +677,32 @@ class LibraryDaoTest { dao.upsertSongs(listOf(song(sourceId, "s-1", "Song", starred = null))) awaitUntil("artist paging observes song writes") { songInvalidated.get() } + + val albumCreditInvalidated = AtomicBoolean(false) + val albumCreditPaging = dao.artistsPaging(artistsDefaultQuery(sourceId, 0, "", ArtistSort.Name, false)) + albumCreditPaging.registerInvalidatedCallback { albumCreditInvalidated.set(true) } + albumCreditPaging.page() + + db.useWriterConnection { connection -> + connection.usePrepared( + "INSERT INTO album_artists (sourceId, albumId, position, artistId, name) " + + "VALUES ($sourceId, 'al-1', 0, 'ar-1', 'Artist')", + ) { it.step() } + } + awaitUntil("artist paging observes album artist writes") { albumCreditInvalidated.get() } + + val songCreditInvalidated = AtomicBoolean(false) + val songCreditPaging = dao.artistsPaging(artistsDefaultQuery(sourceId, 0, "", ArtistSort.Name, false)) + songCreditPaging.registerInvalidatedCallback { songCreditInvalidated.set(true) } + songCreditPaging.page() + + db.useWriterConnection { connection -> + connection.usePrepared( + "INSERT INTO song_artists (sourceId, songId, position, artistId, name) " + + "VALUES ($sourceId, 's-1', 0, 'ar-1', 'Artist')", + ) { it.step() } + } + awaitUntil("artist paging observes song artist writes") { songCreditInvalidated.get() } } @Test diff --git a/app/src/test/kotlin/com/subtracks/ui/components/ArtistChooserSheetTest.kt b/app/src/test/kotlin/com/subtracks/ui/components/ArtistChooserSheetTest.kt index 21aa69de..09c512d8 100644 --- a/app/src/test/kotlin/com/subtracks/ui/components/ArtistChooserSheetTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/components/ArtistChooserSheetTest.kt @@ -46,7 +46,7 @@ class ArtistChooserSheetTest { } @Test - fun opensTheChosenArtistBeforeTheSheetFinishesClosing() { + fun opensTheChosenArtistImmediately() { var opened: ArtistRef? = null composeRule.setContent { SubtracksTheme { 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 873a9f97..3a096643 100644 --- a/app/src/test/kotlin/com/subtracks/ui/search/SearchScreenTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/search/SearchScreenTest.kt @@ -6,7 +6,10 @@ 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.assertIsFocused +import androidx.compose.ui.test.assertIsNotFocused import androidx.compose.ui.test.hasSetTextAction +import androidx.compose.ui.test.junit4.StateRestorationTester import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick @@ -91,7 +94,7 @@ class SearchScreenTest { } @Test - fun typingResumesAtTheEndOfTheQueryAfterTheScreenIsRecreated() { + fun typingKeepsTheCursorAtTheEndAfterTheFieldIsRecreated() { var query by mutableStateOf("road") var show by mutableStateOf(true) composeRule.setContent { @@ -120,6 +123,21 @@ class SearchScreenTest { .hideSoftInputFromWindow(composeRule.activity.window.decorView.windowToken, 0) } + @Test + fun restoringTheScreenDoesNotStealFocus() { + val restorationTester = StateRestorationTester(composeRule) + restorationTester.setContent { + SubtracksTheme { + SearchScreen(query = "road", onQueryChange = {}, results = SearchResults()) + } + } + + composeRule.onNode(hasSetTextAction()).assertIsFocused() + restorationTester.emulateSavedInstanceStateRestore() + + composeRule.onNode(hasSetTextAction()).assertIsNotFocused() + } + @Test fun idsSharedAcrossTypesDoNotCollide() { composeRule.setContent {