Make repeated ids safe as lazy list keys across the Wolfskin screens
This commit is contained in:
parent
2b142af8d1
commit
6f85b9c611
8 changed files with 144 additions and 10 deletions
|
|
@ -97,9 +97,10 @@ fun ComingInSheet(shell: ShellState, modifier: Modifier = Modifier) {
|
|||
val visibleRows = remember(data, start, state.windowSize) {
|
||||
data.ordered.subList(start, (start + state.windowSize).coerceAtMost(data.ordered.size))
|
||||
}
|
||||
val rowKeys = remember(visibleRows) { visibleRows.map { it.id }.uniqueKeys() }
|
||||
LazyColumn(Modifier.clipToBounds().then(if (data.status == ComingInStatus.Unavailable) Modifier.weight(1f, fill = false) else Modifier), verticalArrangement = Arrangement.spacedBy(T.listGap)) {
|
||||
itemsIndexed(visibleRows,
|
||||
key = { _, item -> item.id }, contentType = { _, _ -> "request" }) { _, item ->
|
||||
key = { index, _ -> rowKeys[index] }, contentType = { _, _ -> "request" }) { _, item ->
|
||||
ComingInRow(item, shell.person, !state.recoveryFocused && item.id == state.focusedId, state.expanded && item.id == state.focusedId,
|
||||
state.ratingsVisible && data.signedIn && item.id == state.focusedId,
|
||||
onFocus = { state.focus(item.id) }, onClick = { state.focus(item.id); shell.activateRequest() })
|
||||
|
|
|
|||
|
|
@ -48,8 +48,9 @@ fun HomeShelf(row: HomeRow, memory: HomeViewMemory, focused: Boolean, column: In
|
|||
Box(Modifier.fillMaxWidth().height(L.rowHeadingBand + height + caption + L.rowGapAfter)) {
|
||||
if (!row.skeleton) Text(row.title, color = C.fg, style = T.rowHeading, maxLines = 1, modifier = Modifier.offset(x = L.safeSide))
|
||||
val cards = if (row.skeleton) List(if (row.cardKind == HomeCardKind.Poster) 8 else 5) { null } else row.cards + if (row.hasSeeAll) listOf(null) else emptyList()
|
||||
val cardKeys = remember(row.cards, row.skeleton, row.hasSeeAll, row.id) { cards.mapIndexed { index, title -> title?.id ?: "${row.id}:tail:$index" }.uniqueKeys() }
|
||||
LazyRow(state = track, modifier = Modifier.fillMaxWidth().offset(y = L.rowHeadingBand - L.rowGapAfter).height(height + caption + L.rowGapAfter * 2).skeletonPulse(row.skeleton).rowReveal(row.id, !row.skeleton), contentPadding = PaddingValues(horizontal = L.safeSide, vertical = L.rowGapAfter), horizontalArrangement = Arrangement.spacedBy(K.posterGap), userScrollEnabled = !row.skeleton) {
|
||||
itemsIndexed(cards, key = { index, title -> title?.id ?: "${row.id}:tail:$index" }, contentType = { _, title -> if (row.skeleton) "skeleton" else if (title == null) "seeAll" else row.cardKind.name.lowercase() }) { index, title ->
|
||||
itemsIndexed(cards, key = { index, _ -> cardKeys[index] }, contentType = { _, title -> if (row.skeleton) "skeleton" else if (title == null) "seeAll" else row.cardKind.name.lowercase() }) { index, title ->
|
||||
val selected = !row.skeleton && focused && column == index
|
||||
val cardModifier = Modifier.zIndex(if (selected) 2f else 0f).testTag("home-card:${row.id}:$index")
|
||||
.then(if (!row.skeleton) Modifier.focusOnPointerEnter { onHover(index) } else Modifier)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,21 @@
|
|||
package org.justinvisser.wolfskin.design
|
||||
|
||||
/**
|
||||
* Lazy lists throw "Key was already used" when two items share a key, and server lists can repeat an identity
|
||||
* (one person credited for several roles, a title listed twice). The first occurrence keeps its plain key so focus
|
||||
* and scroll restoration by key are unchanged for every list that has no repeat; each repeat gets a stable
|
||||
* `key#n` that cannot collide with another key in the list.
|
||||
*/
|
||||
internal fun List<String>.uniqueKeys(): List<String> {
|
||||
val used = HashSet<String>(size)
|
||||
val repeats = HashMap<String, Int>()
|
||||
return map { base ->
|
||||
var key = base
|
||||
while (!used.add(key)) {
|
||||
val count = (repeats[base] ?: 0) + 1
|
||||
repeats[base] = count
|
||||
key = "$base#$count"
|
||||
}
|
||||
key
|
||||
}
|
||||
}
|
||||
|
|
@ -98,6 +98,12 @@ fun LibraryScreen(
|
|||
animateLibraryScroll(view, headerCount, target, density)
|
||||
}
|
||||
}
|
||||
val rowKeys = remember(items, grid.columns, grid.catalogue) {
|
||||
(0 until (items.size + grid.columns - 1) / grid.columns).map { row ->
|
||||
val title = items[row * grid.columns]
|
||||
"grid-${if (grid.catalogue) title.catalogueKey else title.id}"
|
||||
}.uniqueKeys()
|
||||
}
|
||||
Box(modifier.semantics { stateDescription = "${grid.title}:${if (grid.toolbar) "tool-${grid.tool}" else if (grid.inRail) "rail-${grid.railLetter}" else focused?.id ?: "recovery"}" }.testTag("library-grid")) {
|
||||
LazyColumn(state = view.listState, modifier = Modifier.fillMaxSize().pointerInput(grid, headerCount) {
|
||||
awaitPointerEventScope {
|
||||
|
|
@ -128,10 +134,7 @@ fun LibraryScreen(
|
|||
LibraryStatus.Unavailable -> item(key = "unavailable", contentType = "state") { LibraryRecovery(grid, active) }
|
||||
LibraryStatus.Ready -> if (items.isEmpty()) item(key = "empty", contentType = "state") { LibraryRecovery(grid, active && !grid.toolbar) }
|
||||
else {
|
||||
items((items.size + grid.columns - 1) / grid.columns, key = { row ->
|
||||
val title = items[row * grid.columns]
|
||||
"grid-${if (grid.catalogue) title.catalogueKey else title.id}"
|
||||
}, contentType = { "grid" }) { row ->
|
||||
items((items.size + grid.columns - 1) / grid.columns, key = { rowKeys[it] }, contentType = { "grid" }) { row ->
|
||||
Box(Modifier.fillMaxWidth().height(rowHeight)) {
|
||||
Row(Modifier.padding(start = L.safeSide).rowReveal("grid-$row"), horizontalArrangement = Arrangement.spacedBy(K.posterGap)) {
|
||||
for (column in 0 until grid.columns) {
|
||||
|
|
|
|||
|
|
@ -128,6 +128,7 @@ private fun PersonPage(state: PersonState, shell: ShellState, modifier: Modifier
|
|||
}
|
||||
key?.let { state.key(it, event.type == KeyEventType.KeyDown, event.nativeKeyEvent.repeatCount, shell) } ?: false
|
||||
}.focusable()) {
|
||||
val rowKeys = androidx.compose.runtime.remember(state.credits, columns) { state.credits.chunked(columns).map { it.first().id }.uniqueKeys() }
|
||||
LazyColumn(state = list, modifier = Modifier.fillMaxSize()) {
|
||||
item(key = "person-header", contentType = "header") {
|
||||
Box(Modifier.height(L.row_phead)) {
|
||||
|
|
@ -144,7 +145,7 @@ private fun PersonPage(state: PersonState, shell: ShellState, modifier: Modifier
|
|||
item(key = "filmography", contentType = "header") {
|
||||
Text("Filmography", color = C.fg, style = TitleTextStyles.rowHeading, modifier = Modifier.height(L.row_gh).padding(start = L.safeSide, top = D.gridHeadingTop))
|
||||
}
|
||||
itemsIndexed(state.credits.chunked(columns), key = { _, row -> row.first().id }, contentType = { _, _ -> "grid" }) { rowIndex, row ->
|
||||
itemsIndexed(state.credits.chunked(columns), key = { index, _ -> rowKeys[index] }, contentType = { _, _ -> "grid" }) { rowIndex, row ->
|
||||
Row(Modifier.height(D.gridHeight).padding(start = L.safeSide), horizontalArrangement = Arrangement.spacedBy(WolfskinTokens.Card.posterGap)) {
|
||||
row.forEachIndexed { column, item ->
|
||||
val index = rowIndex * columns + column
|
||||
|
|
|
|||
|
|
@ -203,8 +203,9 @@ private fun TitleSeasons(state: TitleState, active: Boolean) {
|
|||
if (NetworkArtTransition.reduced) list.scrollToItem(visibleChip) else list.animateScrollToItem(visibleChip)
|
||||
}
|
||||
}
|
||||
val seasonKeys = remember(state.data.seasonNumbers) { state.data.seasonNumbers.map { "season-$it" }.uniqueKeys() }
|
||||
androidx.compose.foundation.lazy.LazyRow(Modifier.height(L.row_chips).padding(top = D.chipTop), state = list, contentPadding = androidx.compose.foundation.layout.PaddingValues(horizontal = L.safeSide), horizontalArrangement = Arrangement.spacedBy(D.chipGap)) {
|
||||
items(state.data.seasonNumbers.size, key = { "season-${state.data.seasonNumbers[it]}" }, contentType = { "chip" }) { index ->
|
||||
items(state.data.seasonNumbers.size, key = { seasonKeys[it] }, contentType = { "chip" }) { index ->
|
||||
val number = state.data.seasonNumbers[index]
|
||||
val focused = active && state.column == index
|
||||
val selected = state.season == number
|
||||
|
|
|
|||
|
|
@ -51,6 +51,8 @@ fun TitleCast(state: TitleState, active: Boolean) {
|
|||
LaunchedEffect(state.column, active, state.data.cast.size) {
|
||||
if (active) track.keepColumnInWindow(state.column, WolfskinTokens.Card.personSlots, pitch, state.data.cast.size)
|
||||
}
|
||||
// The same person is credited once per role, so the person id alone repeats.
|
||||
val castKeys = androidx.compose.runtime.remember(state.data.cast) { state.data.cast.map { it.id }.uniqueKeys() }
|
||||
Box(Modifier.height(D.castHeight).testTag("title-cast")) {
|
||||
Text("Cast", color = C.fg, style = WolfskinTokens.Type.rowHeading, modifier = Modifier.padding(start = L.safeSide).height(L.rowHeadingBand))
|
||||
LazyRow(state = track, modifier = Modifier.fillMaxWidth().offset(y = L.rowHeadingBand - L.rowGapAfter).height(WolfskinTokens.Card.personH + L.rowGapAfter * 2).skeletonPulse(state.data.castStatus == TitleReadStatus.Loading),
|
||||
|
|
@ -59,7 +61,7 @@ fun TitleCast(state: TitleState, active: Boolean) {
|
|||
items(WolfskinTokens.Card.personSlots, key = { "cast-skeleton-$it" }, contentType = { "skeleton" }) {
|
||||
Box(Modifier.size(L.castCircle).background(C.surface2, CircleShape))
|
||||
}
|
||||
} else itemsIndexed(state.data.cast, key = { _, person -> person.id }, contentType = { _, _ -> "person" }) { index, person ->
|
||||
} else itemsIndexed(state.data.cast, key = { index, _ -> castKeys[index] }, contentType = { _, _ -> "person" }) { index, person ->
|
||||
val focused = active && state.column == index
|
||||
Box(Modifier.size(WolfskinTokens.Card.personW, WolfskinTokens.Card.personH).zIndex(if (focused) 1f else 0f)
|
||||
.onGloballyPositioned { state.recordCenter("cast", index, it.boundsInRoot().center.x) }
|
||||
|
|
@ -88,6 +90,7 @@ fun TitleMediaShelf(state: TitleState, episodes: Boolean, active: Boolean) {
|
|||
LaunchedEffect(state.column, active, titles.size) {
|
||||
if (active) track.keepColumnInWindow(state.column, visible, pitch, titles.size)
|
||||
}
|
||||
val titleKeys = androidx.compose.runtime.remember(titles) { titles.map { it.id }.uniqueKeys() }
|
||||
Box(Modifier.height(L.rowHeadingBand + height + L.rowGapAfter + if (episodes) WolfskinTokens.Shell.zero else WolfskinTokens.Card.posterCaptionBand).testTag("title-shelf:$id")) {
|
||||
Text(if (episodes) "Season ${state.season}" else "More like this", color = C.fg, style = WolfskinTokens.Type.rowHeading, modifier = Modifier.padding(start = L.safeSide).height(L.rowHeadingBand))
|
||||
LazyRow(state = track, modifier = Modifier.fillMaxWidth().offset(y = L.rowHeadingBand - L.rowGapAfter)
|
||||
|
|
@ -97,7 +100,7 @@ fun TitleMediaShelf(state: TitleState, episodes: Boolean, active: Boolean) {
|
|||
items(visible, key = { "episode-skeleton-$it" }, contentType = { "skeleton" }) { index ->
|
||||
Box(Modifier.size(width, height).testTag("title-episode-slot-$index").background(C.surface2, RoundedCornerShape(WolfskinTokens.Radius.card)))
|
||||
}
|
||||
} else itemsIndexed(titles, key = { _, item -> item.id }, contentType = { _, _ -> if (episodes) "tile" else "poster" }) { index, item ->
|
||||
} else itemsIndexed(titles, key = { index, _ -> titleKeys[index] }, contentType = { _, _ -> if (episodes) "tile" else "poster" }) { index, item ->
|
||||
TitlePoster(item, active && state.column == index, Modifier.width(width).then(if (episodes) Modifier.testTag("title-episode-slot-$index") else Modifier)
|
||||
.onGloballyPositioned { state.recordCenter(id, index, it.boundsInRoot().center.x) },
|
||||
episodes, state.data.connection != TitleConnection.SignedOut, onFocus = { state.focus(id, index) }) { state.click(id, index) }
|
||||
|
|
|
|||
|
|
@ -0,0 +1,103 @@
|
|||
package org.justinvisser.wolfskin.design
|
||||
|
||||
import android.app.Application
|
||||
import androidx.compose.foundation.layout.requiredSize
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.test.junit4.createComposeRule
|
||||
import androidx.compose.ui.test.onAllNodesWithTag
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.unit.dp
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.annotation.Config
|
||||
|
||||
/**
|
||||
* Lazy lists throw "Key ... was already used" in measure when two items share an id. Server lists can repeat one:
|
||||
* a Jellyfin person is credited once per role, so the title page crashed on a cast with the same person twice.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
@Config(sdk = [34], application = Application::class, qualifiers = "w960dp-h540dp-xhdpi")
|
||||
class DuplicateLazyKeysTest {
|
||||
@get:Rule val rule = createComposeRule()
|
||||
private val viewer = ShellPerson("justin", "Justin", "J", false)
|
||||
private fun shell(route: String) = ShellState(listOf(viewer), viewer.key).apply { pickerVisible = false; pickerLaunch = false; enterDestination(route) }
|
||||
private fun repeated(id: String, name: String = id) = TitleItem(id, name, "movie", held = true)
|
||||
|
||||
@Test fun repeatedKeysKeepTheFirstAndSuffixTheRest() {
|
||||
assertEquals(listOf("a", "b", "a#1", "a#2", "a#1#1"), listOf("a", "b", "a", "a", "a#1").uniqueKeys())
|
||||
assertEquals(listOf("a", "b"), listOf("a", "b").uniqueKeys())
|
||||
}
|
||||
|
||||
@Test fun castCreditedForTwoRolesComposes() {
|
||||
val state = TitleState(TitlePageData(repeated("film"), cast = listOf(
|
||||
TitlePerson("person", "Actor", "Hero"), TitlePerson("other", "Other", "Friend"), TitlePerson("person", "Actor", "Villain"))), viewer) {}
|
||||
rule.setContent { WolfskinTheme { TitleCast(state, active = false) } }
|
||||
rule.waitForIdle()
|
||||
assertEquals(2, rule.onAllNodesWithTag("title-cast-person").fetchSemanticsNodes().size)
|
||||
rule.onNodeWithTag("title-cast-other").assertExists()
|
||||
}
|
||||
|
||||
@Test fun relatedTitleListedTwiceComposes() {
|
||||
val state = TitleState(TitlePageData(repeated("film"), related = listOf(repeated("same"), repeated("other"), repeated("same"))), viewer) {}
|
||||
rule.setContent { WolfskinTheme { TitleMediaShelf(state, episodes = false, active = false) } }
|
||||
rule.waitForIdle()
|
||||
rule.onNodeWithTag("title-shelf:related").assertExists()
|
||||
}
|
||||
|
||||
@Test fun episodeListedTwiceComposes() {
|
||||
val series = TitleItem("series", "A series", "tv", seasons = 1, held = true)
|
||||
val episode = TitleItem("episode", "Pilot", "episode", held = true, season = 1, episode = 1)
|
||||
val state = TitleState(TitlePageData(series, playTarget = episode, episodes = listOf(episode, episode), connection = TitleConnection.SignedIn), viewer) {}
|
||||
rule.setContent { WolfskinTheme { TitleMediaShelf(state, episodes = true, active = false) } }
|
||||
rule.waitForIdle()
|
||||
rule.onNodeWithTag("title-shelf:episodes").assertExists()
|
||||
}
|
||||
|
||||
@Test fun repeatedSeasonNumberComposes() {
|
||||
val series = TitleItem("series", "A series", "tv", seasons = 2, held = true)
|
||||
val state = TitleState(TitlePageData(series, seasonNumbers = listOf(1, 1, 2)), viewer) {}
|
||||
rule.setContent { WolfskinTheme { TitleSurface(state, shell("title/series"), Modifier.requiredSize(960.dp, 540.dp)) } }
|
||||
rule.waitForIdle()
|
||||
rule.onNodeWithTag("title-shelf:episodes").assertDoesNotExist()
|
||||
assertTrue(state.rows.contains("seasons"))
|
||||
}
|
||||
|
||||
@Test fun filmographyWithARepeatedCreditComposes() {
|
||||
val state = PersonState(TitlePerson("person", "Actor"), List(WolfskinTokens.Behaviour.gridColumns.toInt() + 1) { repeated("credit") }, {})
|
||||
rule.setContent { WolfskinTheme { PersonSurface(state, shell("title/person"), Modifier.requiredSize(960.dp, 540.dp)) } }
|
||||
rule.waitForIdle()
|
||||
rule.onNodeWithTag("person-page").assertExists()
|
||||
}
|
||||
|
||||
@Test fun homeShelfWithARepeatedCardComposes() {
|
||||
val card = HomeTitle("episode", "Pilot", "episode")
|
||||
val snapshot = HomeSnapshot(listOf(HomeRow("cw", HomeRowKind.Shelf, "Continue watching", listOf(card, HomeTitle("other", "Other", "movie"), card), HomeCardKind.Wide)), HomeAvailability.Ready)
|
||||
val shell = shell("home")
|
||||
rule.setContent { WolfskinTheme { ShellSurface(shell) { modifier -> HomeScreen(shell, snapshot, modifier) {} } } }
|
||||
rule.waitForIdle()
|
||||
rule.onNodeWithTag("home-card:cw:0").assertExists()
|
||||
rule.onNodeWithTag("home-card:cw:2").assertExists()
|
||||
}
|
||||
|
||||
@Test fun libraryGridWithARepeatedTitleComposes() {
|
||||
val columns = WolfskinTokens.Behaviour.gridColumns.toInt()
|
||||
val titles = List(columns * 2) { LibraryTitle("same", "Same title", "movie") }
|
||||
val shell = shell("movies")
|
||||
val grid = LibraryGridState("Movies", titles)
|
||||
rule.setContent { WolfskinTheme { ShellSurface(shell) { modifier -> LibraryScreen(shell, grid, modifier) } } }
|
||||
rule.waitForIdle()
|
||||
rule.onNodeWithTag("library-grid").assertExists()
|
||||
}
|
||||
|
||||
@Test fun requestListedTwiceComposes() {
|
||||
val request = ComingInRequest("r1", "movie", 1, "A film", "justin", "Justin", null, null, ComingInStage.Waiting)
|
||||
val shell = shell("settings").apply { requests.update(ComingInSnapshot(listOf(request, request.copy(id = "r2"), request), ComingInStatus.Ready, signedIn = true)); openRequests() }
|
||||
rule.setContent { WolfskinTheme { ShellSurface(shell) } }
|
||||
rule.waitForIdle()
|
||||
rule.onNodeWithTag("wolfskin-shell").assertExists()
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue