From 9cfa4b62e2caab3fa0b90d327ea33752c9bc55cf Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Thu, 15 Oct 2026 13:19:46 +0000 Subject: [PATCH] library: keep the album grid aligned across a paging reload A scrobble writes albums.playCount/played, which invalidates the album list's PagingSource. Room then refreshes from anchor - initialLoadSize/2, and with placeholders off that page is the presented list: LazyPagingItems index zero becomes the page's first album, so the grid re-groups its rows and every cover moves by (oldStart - newStart) % 3 columns, while the last row's width becomes the loaded count % 3 instead of the library size % 3. Enable placeholders for the album pager so an album's index is its absolute position, and draw a placeholder cell for an unloaded slot. The other lists are single column, so a reload re-anchors them by key and they never shifted. This only keeps the grid still; the reloads themselves are still unnecessary and are tracked separately. --- .../subtracks/data/repo/LibraryRepository.kt | 5 +- .../com/subtracks/ui/library/AlbumsContent.kt | 14 ++++ .../data/repo/LibraryRepositoryTest.kt | 81 +++++++++++++++++++ .../ui/library/AlbumsPlaceholderTest.kt | 61 ++++++++++++++ 4 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 app/src/test/kotlin/com/subtracks/ui/library/AlbumsPlaceholderTest.kt diff --git a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt index d5a52237..d2495c78 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt @@ -174,7 +174,7 @@ class LibraryRepository( search: String, downloaded: Boolean, ): Flow> = - pager(40) { + pager(40, enablePlaceholders = true) { val dao = db.libraryDao() if (!downloaded) { dao.albumsPaging(albumsDefaultQuery(sourceId, starred.ordinal, search, sort, descending)) @@ -507,10 +507,11 @@ class LibraryRepository( private fun pager( pageSize: Int, + enablePlaceholders: Boolean = false, source: () -> PagingSource, ): Flow> = Pager( - config = PagingConfig(pageSize = pageSize, enablePlaceholders = false), + config = PagingConfig(pageSize = pageSize, enablePlaceholders = enablePlaceholders), pagingSourceFactory = source, ).flow } diff --git a/app/src/main/kotlin/com/subtracks/ui/library/AlbumsContent.kt b/app/src/main/kotlin/com/subtracks/ui/library/AlbumsContent.kt index 76b9d4c7..8df836ae 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/AlbumsContent.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/AlbumsContent.kt @@ -1,5 +1,6 @@ package com.subtracks.ui.library +import androidx.compose.foundation.background import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -15,6 +16,7 @@ import androidx.compose.foundation.lazy.grid.GridItemSpan import androidx.compose.foundation.lazy.grid.LazyVerticalGrid import androidx.compose.foundation.lazy.grid.rememberLazyGridState import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -43,6 +45,8 @@ import com.subtracks.ui.theme.PrefetchArtworkSeeds const val ALBUM_COVER_TAG = "album-cover" +const val ALBUM_PLACEHOLDER_TAG = "album-placeholder" + @Composable fun AlbumsContent( items: LazyPagingItems, @@ -117,6 +121,16 @@ fun AlbumsContent( scrim = true, ) } + } else { + Box( + modifier = + Modifier + .fillMaxWidth() + .aspectRatio(1f) + .clip(RoundedCornerShape(2.dp)) + .background(MaterialTheme.colorScheme.surfaceVariant) + .testTag(ALBUM_PLACEHOLDER_TAG), + ) } } item(span = { GridItemSpan(maxLineSpan) }) { diff --git a/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt index 57257fdf..18557e4e 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt @@ -2,6 +2,8 @@ package com.subtracks.data.repo import android.content.Context import android.content.res.Resources +import androidx.paging.PagingDataEvent +import androidx.paging.PagingDataPresenter import androidx.room3.Room import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider @@ -9,16 +11,23 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import com.subtracks.cancelAndJoinBlocking import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.download.ArtworkStore +import com.subtracks.data.model.Album import com.subtracks.data.model.Song +import com.subtracks.data.prefs.AlbumSort +import com.subtracks.data.prefs.StarredFilter import com.subtracks.data.prefs.fakeUserPreferences import com.subtracks.data.source.StarType +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.asCoroutineDispatcher +import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.delay import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeout import okhttp3.OkHttpClient import okhttp3.mockwebserver.MockResponse @@ -32,7 +41,10 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import java.io.File +import java.util.concurrent.Executors import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger +import kotlin.coroutines.CoroutineContext @RunWith(AndroidJUnit4::class) class LibraryRepositoryTest { @@ -163,6 +175,75 @@ class LibraryRepositoryTest { assertEquals(2L, repository.albumSongOrdinal(sourceId, "al1", "s3")) } + @Test + fun theAlbumGridPresentsTheWholeLibraryAndSurvivesAReload() = + runBlocking { + val sourceId = sourceRepository.addSource("nav", "http://localhost/", "u", "p", false) + db.libraryDao().upsertAlbums((0 until 250).map { seedAlbum(sourceId, it) }) + + val presenterDispatcher = Executors.newSingleThreadExecutor().asCoroutineDispatcher() + val presenter = RecordingPresenter(presenterDispatcher) + val job = + launch { + repository + .albums(sourceId, AlbumSort.Name, false, StarredFilter.Any, "", false) + .collect { presenter.collectFrom(it) } + } + try { + awaitRefreshes(presenter, 1) + assertEquals(250, presented(presenter, presenterDispatcher)) + + db.libraryDao().upsertAlbums(listOf(seedAlbum(sourceId, 0).copy(playCount = 1))) + awaitRefreshes(presenter, 2) + + assertEquals("a reload must not collapse the grid to one page", 250, presented(presenter, presenterDispatcher)) + } finally { + job.cancelAndJoin() + presenterDispatcher.close() + } + } + + private suspend fun awaitRefreshes( + presenter: RecordingPresenter, + target: Int, + ) { + withTimeout(30_000) { + while (presenter.refreshes.get() < target) delay(10) + } + } + + private suspend fun presented( + presenter: RecordingPresenter, + dispatcher: CoroutineDispatcher, + ): Int = withContext(dispatcher) { presenter.size } + + private fun seedAlbum( + sourceId: Long, + index: Int, + ) = Album( + sourceId = sourceId, + id = "al-" + index.toString().padStart(4, '0'), + artistId = "ar1", + name = "Album " + index.toString().padStart(4, '0'), + albumArtist = "Artist", + created = 0, + coverArt = null, + genre = null, + year = null, + starred = null, + songCount = 1, + ) + + private class RecordingPresenter( + context: CoroutineContext, + ) : PagingDataPresenter(context) { + val refreshes = AtomicInteger() + + override suspend fun presentPagingDataEvent(event: PagingDataEvent) { + if (event is PagingDataEvent.Refresh) refreshes.incrementAndGet() + } + } + private suspend fun withSource(block: suspend (MockWebServer) -> Unit) { val server = MockWebServer() server.start() diff --git a/app/src/test/kotlin/com/subtracks/ui/library/AlbumsPlaceholderTest.kt b/app/src/test/kotlin/com/subtracks/ui/library/AlbumsPlaceholderTest.kt new file mode 100644 index 00000000..c01bd39f --- /dev/null +++ b/app/src/test/kotlin/com/subtracks/ui/library/AlbumsPlaceholderTest.kt @@ -0,0 +1,61 @@ +package com.subtracks.ui.library + +import androidx.activity.ComponentActivity +import androidx.compose.runtime.remember +import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule +import androidx.compose.ui.test.onAllNodesWithTag +import androidx.compose.ui.unit.dp +import androidx.paging.PagingData +import androidx.paging.compose.collectAsLazyPagingItems +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.github.takahirom.roborazzi.RobolectricDeviceQualifiers +import com.subtracks.data.model.Album +import com.subtracks.ui.theme.SubtracksTheme +import kotlinx.coroutines.flow.flowOf +import org.junit.Assert.assertEquals +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode + +@RunWith(AndroidJUnit4::class) +@GraphicsMode(GraphicsMode.Mode.NATIVE) +@Config(qualifiers = RobolectricDeviceQualifiers.PixelXL) +class AlbumsPlaceholderTest { + @get:Rule + val composeRule = createAndroidComposeRule() + + @Test + fun anUnloadedCellIsDrawnAsAPlaceholder() { + composeRule.setContent { + SubtracksTheme { + val items = + remember { flowOf(PagingData.from(listOf(album(0)), placeholdersAfter = 2)) } + .collectAsLazyPagingItems() + AlbumsContent(items, { _, _ -> null }, 0.dp, {}) + } + } + composeRule.waitUntil(timeoutMillis = 5_000) { + composeRule.onAllNodesWithTag(ALBUM_PLACEHOLDER_TAG).fetchSemanticsNodes().isNotEmpty() + } + + assertEquals(2, composeRule.onAllNodesWithTag(ALBUM_PLACEHOLDER_TAG).fetchSemanticsNodes().size) + assertEquals(1, composeRule.onAllNodesWithTag(ALBUM_COVER_TAG).fetchSemanticsNodes().size) + } + + private fun album(index: Int) = + Album( + sourceId = 1, + id = "al-" + index.toString().padStart(4, '0'), + artistId = "ar", + name = "Album " + index.toString().padStart(4, '0'), + albumArtist = "Artist", + created = 0, + coverArt = null, + genre = null, + year = null, + starred = null, + songCount = 1, + ) +}