mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-10 15:11:58 +01:00
library: keep the artist screen fast and avoid placeholder labels
Rewrite the artist album lookup as indexed UNION branches instead of ORs that scanned the song table, and let artistName take the first indexed row. Keep the type label and album count blank until the album flow has emitted, so the page no longer flashes "Artist" and "0 albums" before showing the real values.
This commit is contained in:
@@ -945,9 +945,9 @@ interface LibraryDao {
|
||||
@Query(
|
||||
"SELECT COALESCE(" +
|
||||
"(SELECT name FROM song_artists WHERE sourceId = :sourceId AND artistId = :artistId " +
|
||||
"AND name != '' ORDER BY songId, position LIMIT 1), " +
|
||||
"AND name != '' LIMIT 1), " +
|
||||
"(SELECT artist FROM songs WHERE sourceId = :sourceId AND artistId = :artistId " +
|
||||
"AND artist IS NOT NULL AND artist != '' ORDER BY id LIMIT 1))",
|
||||
"AND artist IS NOT NULL AND artist != '' LIMIT 1))",
|
||||
)
|
||||
fun artistName(
|
||||
sourceId: Long,
|
||||
@@ -973,9 +973,22 @@ interface LibraryDao {
|
||||
): Flow<List<String>>
|
||||
|
||||
@Query(
|
||||
"SELECT * FROM albums WHERE sourceId = :sourceId AND ($ALBUM_MATCHES_ARTIST " +
|
||||
"OR id IN (SELECT albumId FROM songs WHERE sourceId = :sourceId AND $SONG_MATCHES_ARTIST)) " +
|
||||
"ORDER BY year DESC, name COLLATE NOCASE, id",
|
||||
"SELECT * FROM (" +
|
||||
"SELECT albums.* FROM albums WHERE albums.sourceId = :sourceId AND albums.artistId = :artistId " +
|
||||
"UNION " +
|
||||
"SELECT albums.* FROM album_artists " +
|
||||
"JOIN albums ON albums.sourceId = album_artists.sourceId AND albums.id = album_artists.albumId " +
|
||||
"WHERE album_artists.sourceId = :sourceId AND album_artists.artistId = :artistId " +
|
||||
"UNION " +
|
||||
"SELECT albums.* 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 = :sourceId AND song_artists.artistId = :artistId " +
|
||||
"UNION " +
|
||||
"SELECT albums.* FROM songs " +
|
||||
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
|
||||
"WHERE songs.sourceId = :sourceId AND songs.artistId = :artistId" +
|
||||
") ORDER BY year DESC, name COLLATE NOCASE, id",
|
||||
)
|
||||
fun albumsForArtist(
|
||||
sourceId: Long,
|
||||
@@ -1018,15 +1031,31 @@ interface LibraryDao {
|
||||
): PagingSource<Int, PlaylistSongItem>
|
||||
|
||||
@Query(
|
||||
"SELECT * FROM albums WHERE sourceId = :sourceId AND (" +
|
||||
"($ALBUM_MATCHES_ARTIST AND EXISTS (SELECT 1 FROM songs " +
|
||||
"SELECT * FROM (" +
|
||||
"SELECT albums.* FROM albums " +
|
||||
"WHERE albums.sourceId = :sourceId AND albums.artistId = :artistId " +
|
||||
"AND EXISTS (SELECT 1 FROM songs " +
|
||||
"JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id " +
|
||||
"WHERE songs.sourceId = albums.sourceId AND songs.albumId = albums.id AND sd.status = 'Completed')) " +
|
||||
"OR EXISTS (SELECT 1 FROM songs " +
|
||||
"WHERE songs.sourceId = albums.sourceId AND songs.albumId = albums.id AND sd.status = 'Completed') " +
|
||||
"UNION " +
|
||||
"SELECT albums.* FROM album_artists " +
|
||||
"JOIN albums ON albums.sourceId = album_artists.sourceId AND albums.id = album_artists.albumId " +
|
||||
"WHERE album_artists.sourceId = :sourceId AND album_artists.artistId = :artistId " +
|
||||
"AND EXISTS (SELECT 1 FROM songs " +
|
||||
"JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id " +
|
||||
"WHERE songs.sourceId = albums.sourceId AND songs.albumId = albums.id " +
|
||||
"AND sd.status = 'Completed' AND $SONG_MATCHES_ARTIST)) " +
|
||||
"ORDER BY year DESC, name COLLATE NOCASE, id",
|
||||
"WHERE songs.sourceId = albums.sourceId AND songs.albumId = albums.id AND sd.status = 'Completed') " +
|
||||
"UNION " +
|
||||
"SELECT albums.* 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 " +
|
||||
"JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id AND sd.status = 'Completed' " +
|
||||
"WHERE song_artists.sourceId = :sourceId AND song_artists.artistId = :artistId " +
|
||||
"UNION " +
|
||||
"SELECT albums.* FROM songs " +
|
||||
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
|
||||
"JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id AND sd.status = 'Completed' " +
|
||||
"WHERE songs.sourceId = :sourceId AND songs.artistId = :artistId" +
|
||||
") ORDER BY year DESC, name COLLATE NOCASE, id",
|
||||
)
|
||||
fun albumsForArtistDownloaded(
|
||||
sourceId: Long,
|
||||
|
||||
@@ -159,7 +159,7 @@ fun ArtistDetailRoute(
|
||||
) {
|
||||
val artist by viewModel.artist.collectAsStateWithLifecycle(initialValue = null)
|
||||
val artistName by viewModel.artistName.collectAsStateWithLifecycle(initialValue = null)
|
||||
val albums by viewModel.albums.collectAsStateWithLifecycle(initialValue = emptyList())
|
||||
val albums by viewModel.albums.collectAsStateWithLifecycle(initialValue = null)
|
||||
val art by viewModel.art.collectAsStateWithLifecycle()
|
||||
val artThumbnail by viewModel.artThumbnail.collectAsStateWithLifecycle()
|
||||
val downloadStatus by viewModel.downloadStatus.collectAsStateWithLifecycle()
|
||||
@@ -192,8 +192,9 @@ fun ArtistDetailRoute(
|
||||
ArtistDetailScreen(
|
||||
artist = artist,
|
||||
artistName = artistName.orEmpty(),
|
||||
isAlbumArtist = isAlbumArtist(albums, artistId),
|
||||
albums = albums,
|
||||
isAlbumArtist = albums?.let { isAlbumArtist(it, artistId) },
|
||||
albums = albums.orEmpty(),
|
||||
albumsLoaded = albums != null,
|
||||
art = art,
|
||||
artThumbnail = artThumbnail,
|
||||
artwork = rememberArtworkColors(shortcutArt ?: artThumbnail ?: art, THEME_TRANSITION_MS, fallbackName = artistName),
|
||||
@@ -224,8 +225,9 @@ fun ArtistDetailScreen(
|
||||
artist: Artist?,
|
||||
modifier: Modifier = Modifier,
|
||||
artistName: String = "",
|
||||
isAlbumArtist: Boolean = false,
|
||||
isAlbumArtist: Boolean? = null,
|
||||
albums: List<Album>,
|
||||
albumsLoaded: Boolean = true,
|
||||
art: CoverArtRef?,
|
||||
artThumbnail: CoverArtRef? = null,
|
||||
artwork: ArtworkColors? = null,
|
||||
@@ -252,7 +254,7 @@ fun ArtistDetailScreen(
|
||||
val barHeight = statusBarTop + TopAppBarDefaults.TopAppBarExpandedHeight
|
||||
val albumCount = albums.size.toLong()
|
||||
val displayName = artistName.ifBlank { artist?.name.orEmpty() }
|
||||
val artistType = stringResource(if (isAlbumArtist) R.string.artist_type_album_artist else R.string.artist_type_artist)
|
||||
val artistType = stringResource(if (isAlbumArtist == true) R.string.artist_type_album_artist else R.string.artist_type_artist)
|
||||
val imageTitleTop = ART_HEIGHT - TITLE_INSET - lineHeight
|
||||
val fadeStartPx = with(density) { (imageTitleTop - barHeight - FADE_LEAD).toPx() }
|
||||
val fadeEndPx = with(density) { (imageTitleTop + lineHeight / 2 - barHeight).toPx() }
|
||||
@@ -373,7 +375,7 @@ fun ArtistDetailScreen(
|
||||
modifier = Modifier.fillMaxWidth().padding(start = 16.dp, end = 8.dp, top = TITLE_INSET),
|
||||
) {
|
||||
Text(
|
||||
text = artistType,
|
||||
text = if (albumsLoaded) artistType else "",
|
||||
style = MaterialTheme.typography.labelLarge,
|
||||
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||
)
|
||||
@@ -384,7 +386,12 @@ fun ArtistDetailScreen(
|
||||
verticalAlignment = Alignment.CenterVertically,
|
||||
) {
|
||||
Text(
|
||||
text = pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount),
|
||||
text =
|
||||
if (albumsLoaded) {
|
||||
pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount)
|
||||
} else {
|
||||
""
|
||||
},
|
||||
style = MaterialTheme.typography.bodyLarge,
|
||||
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||
modifier = Modifier.weight(1f),
|
||||
|
||||
@@ -23,6 +23,8 @@ import kotlinx.coroutines.flow.combine
|
||||
import kotlinx.coroutines.flow.filterNotNull
|
||||
import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.flow.flatMapLatest
|
||||
import kotlinx.coroutines.flow.map
|
||||
import kotlinx.coroutines.flow.onStart
|
||||
import kotlinx.coroutines.flow.stateIn
|
||||
import kotlinx.coroutines.launch
|
||||
|
||||
@@ -48,9 +50,11 @@ class ArtistDetailViewModel(
|
||||
|
||||
val offline: StateFlow<Boolean> = sourceRepository.offline
|
||||
|
||||
val albums: Flow<List<Album>> =
|
||||
val albums: Flow<List<Album>?> =
|
||||
combine(sourceId, sourceRepository.offline) { id, offline -> id to offline }
|
||||
.flatMapLatest { (id, offline) -> libraryRepository.artistAlbums(id, artistId, offline) }
|
||||
.map<List<Album>, List<Album>?> { it }
|
||||
.onStart { emit(null) }
|
||||
|
||||
val downloadStatus: StateFlow<ListDownloadStatus> =
|
||||
sourceId
|
||||
|
||||
@@ -91,6 +91,8 @@ class QueryTimingMeasurementTest {
|
||||
timed("albums first page (recent)") { dao.albumsByRecent(1, 0, "").firstPage() }
|
||||
timed("artists first page (name)") { dao.artistsByName(1, 0, "").firstPage() }
|
||||
timed("artists first page (frequent)") { dao.artistsByFrequent(1, 0, "").firstPage() }
|
||||
timed("artist albums") { dao.albumsForArtist(1, "ar0").first() }
|
||||
timed("artist name") { dao.artistName(1, "ar0").first() }
|
||||
timed("playlists first page (name)") { dao.playlistsByName(1, "").firstPage() }
|
||||
timed("albums deep page") { dao.albumsByName(1, 0, "").deepPage(1000) }
|
||||
timed("albums search FTS") { dao.albumsByName(1, 0, "Album 1").firstPage() }
|
||||
|
||||
@@ -24,7 +24,8 @@ class ArtistDetailScreenTest {
|
||||
private fun render(
|
||||
artist: Artist?,
|
||||
artistName: String,
|
||||
isAlbumArtist: Boolean,
|
||||
isAlbumArtist: Boolean? = null,
|
||||
albumsLoaded: Boolean = true,
|
||||
) {
|
||||
composeRule.setContent {
|
||||
SubtracksTheme {
|
||||
@@ -33,6 +34,7 @@ class ArtistDetailScreenTest {
|
||||
artistName = artistName,
|
||||
isAlbumArtist = isAlbumArtist,
|
||||
albums = emptyList(),
|
||||
albumsLoaded = albumsLoaded,
|
||||
art = null,
|
||||
coverArt = { _, _ -> null },
|
||||
onBack = {},
|
||||
@@ -62,4 +64,17 @@ class ArtistDetailScreenTest {
|
||||
composeRule.onNodeWithText("Album artist").assertExists()
|
||||
composeRule.onNodeWithContentDescription("More options").assertExists()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun theTypeAndCountStayBlankUntilTheAlbumsLoad() {
|
||||
render(
|
||||
artist = Artist(sourceId = 1, id = "ar-1", name = "Radiohead", albumCount = 1, starred = null),
|
||||
artistName = "Radiohead",
|
||||
albumsLoaded = false,
|
||||
)
|
||||
|
||||
composeRule.onNodeWithText("Artist").assertDoesNotExist()
|
||||
composeRule.onNodeWithText("Album artist").assertDoesNotExist()
|
||||
composeRule.onNodeWithText("0 albums").assertDoesNotExist()
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user