mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-11 07:29:14 +01:00
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.
This commit is contained in:
@@ -174,7 +174,7 @@ class LibraryRepository(
|
|||||||
search: String,
|
search: String,
|
||||||
downloaded: Boolean,
|
downloaded: Boolean,
|
||||||
): Flow<PagingData<Album>> =
|
): Flow<PagingData<Album>> =
|
||||||
pager(40) {
|
pager(40, enablePlaceholders = true) {
|
||||||
val dao = db.libraryDao()
|
val dao = db.libraryDao()
|
||||||
if (!downloaded) {
|
if (!downloaded) {
|
||||||
dao.albumsPaging(albumsDefaultQuery(sourceId, starred.ordinal, search, sort, descending))
|
dao.albumsPaging(albumsDefaultQuery(sourceId, starred.ordinal, search, sort, descending))
|
||||||
@@ -507,10 +507,11 @@ class LibraryRepository(
|
|||||||
|
|
||||||
private fun <T : Any> pager(
|
private fun <T : Any> pager(
|
||||||
pageSize: Int,
|
pageSize: Int,
|
||||||
|
enablePlaceholders: Boolean = false,
|
||||||
source: () -> PagingSource<Int, T>,
|
source: () -> PagingSource<Int, T>,
|
||||||
): Flow<PagingData<T>> =
|
): Flow<PagingData<T>> =
|
||||||
Pager(
|
Pager(
|
||||||
config = PagingConfig(pageSize = pageSize, enablePlaceholders = false),
|
config = PagingConfig(pageSize = pageSize, enablePlaceholders = enablePlaceholders),
|
||||||
pagingSourceFactory = source,
|
pagingSourceFactory = source,
|
||||||
).flow
|
).flow
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
package com.subtracks.ui.library
|
package com.subtracks.ui.library
|
||||||
|
|
||||||
|
import androidx.compose.foundation.background
|
||||||
import androidx.compose.foundation.combinedClickable
|
import androidx.compose.foundation.combinedClickable
|
||||||
import androidx.compose.foundation.layout.Arrangement
|
import androidx.compose.foundation.layout.Arrangement
|
||||||
import androidx.compose.foundation.layout.Box
|
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.LazyVerticalGrid
|
||||||
import androidx.compose.foundation.lazy.grid.rememberLazyGridState
|
import androidx.compose.foundation.lazy.grid.rememberLazyGridState
|
||||||
import androidx.compose.foundation.shape.RoundedCornerShape
|
import androidx.compose.foundation.shape.RoundedCornerShape
|
||||||
|
import androidx.compose.material3.MaterialTheme
|
||||||
import androidx.compose.runtime.Composable
|
import androidx.compose.runtime.Composable
|
||||||
import androidx.compose.ui.Alignment
|
import androidx.compose.ui.Alignment
|
||||||
import androidx.compose.ui.Modifier
|
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_COVER_TAG = "album-cover"
|
||||||
|
|
||||||
|
const val ALBUM_PLACEHOLDER_TAG = "album-placeholder"
|
||||||
|
|
||||||
@Composable
|
@Composable
|
||||||
fun AlbumsContent(
|
fun AlbumsContent(
|
||||||
items: LazyPagingItems<Album>,
|
items: LazyPagingItems<Album>,
|
||||||
@@ -117,6 +121,16 @@ fun AlbumsContent(
|
|||||||
scrim = true,
|
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) }) {
|
item(span = { GridItemSpan(maxLineSpan) }) {
|
||||||
|
|||||||
@@ -2,6 +2,8 @@ package com.subtracks.data.repo
|
|||||||
|
|
||||||
import android.content.Context
|
import android.content.Context
|
||||||
import android.content.res.Resources
|
import android.content.res.Resources
|
||||||
|
import androidx.paging.PagingDataEvent
|
||||||
|
import androidx.paging.PagingDataPresenter
|
||||||
import androidx.room3.Room
|
import androidx.room3.Room
|
||||||
import androidx.sqlite.driver.bundled.BundledSQLiteDriver
|
import androidx.sqlite.driver.bundled.BundledSQLiteDriver
|
||||||
import androidx.test.core.app.ApplicationProvider
|
import androidx.test.core.app.ApplicationProvider
|
||||||
@@ -9,16 +11,23 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
|
|||||||
import com.subtracks.cancelAndJoinBlocking
|
import com.subtracks.cancelAndJoinBlocking
|
||||||
import com.subtracks.data.db.SubtracksDatabase
|
import com.subtracks.data.db.SubtracksDatabase
|
||||||
import com.subtracks.data.download.ArtworkStore
|
import com.subtracks.data.download.ArtworkStore
|
||||||
|
import com.subtracks.data.model.Album
|
||||||
import com.subtracks.data.model.Song
|
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.prefs.fakeUserPreferences
|
||||||
import com.subtracks.data.source.StarType
|
import com.subtracks.data.source.StarType
|
||||||
|
import kotlinx.coroutines.CoroutineDispatcher
|
||||||
import kotlinx.coroutines.CoroutineScope
|
import kotlinx.coroutines.CoroutineScope
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.SupervisorJob
|
import kotlinx.coroutines.SupervisorJob
|
||||||
|
import kotlinx.coroutines.asCoroutineDispatcher
|
||||||
|
import kotlinx.coroutines.cancelAndJoin
|
||||||
import kotlinx.coroutines.delay
|
import kotlinx.coroutines.delay
|
||||||
import kotlinx.coroutines.flow.first
|
import kotlinx.coroutines.flow.first
|
||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
import kotlinx.coroutines.runBlocking
|
import kotlinx.coroutines.runBlocking
|
||||||
|
import kotlinx.coroutines.withContext
|
||||||
import kotlinx.coroutines.withTimeout
|
import kotlinx.coroutines.withTimeout
|
||||||
import okhttp3.OkHttpClient
|
import okhttp3.OkHttpClient
|
||||||
import okhttp3.mockwebserver.MockResponse
|
import okhttp3.mockwebserver.MockResponse
|
||||||
@@ -32,7 +41,10 @@ import org.junit.Before
|
|||||||
import org.junit.Test
|
import org.junit.Test
|
||||||
import org.junit.runner.RunWith
|
import org.junit.runner.RunWith
|
||||||
import java.io.File
|
import java.io.File
|
||||||
|
import java.util.concurrent.Executors
|
||||||
import java.util.concurrent.TimeUnit
|
import java.util.concurrent.TimeUnit
|
||||||
|
import java.util.concurrent.atomic.AtomicInteger
|
||||||
|
import kotlin.coroutines.CoroutineContext
|
||||||
|
|
||||||
@RunWith(AndroidJUnit4::class)
|
@RunWith(AndroidJUnit4::class)
|
||||||
class LibraryRepositoryTest {
|
class LibraryRepositoryTest {
|
||||||
@@ -163,6 +175,75 @@ class LibraryRepositoryTest {
|
|||||||
assertEquals(2L, repository.albumSongOrdinal(sourceId, "al1", "s3"))
|
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<Album>(context) {
|
||||||
|
val refreshes = AtomicInteger()
|
||||||
|
|
||||||
|
override suspend fun presentPagingDataEvent(event: PagingDataEvent<Album>) {
|
||||||
|
if (event is PagingDataEvent.Refresh) refreshes.incrementAndGet()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private suspend fun withSource(block: suspend (MockWebServer) -> Unit) {
|
private suspend fun withSource(block: suspend (MockWebServer) -> Unit) {
|
||||||
val server = MockWebServer()
|
val server = MockWebServer()
|
||||||
server.start()
|
server.start()
|
||||||
|
|||||||
@@ -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<ComponentActivity>()
|
||||||
|
|
||||||
|
@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,
|
||||||
|
)
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user