Merge pull request 'Remove the competing Discover title preview' (#72) from fm/tv-details-flash-2 into fm/tv-round2

This commit is contained in:
justinzeus 2026-11-08 01:01:37 +01:00
commit 328916c5ac
7 changed files with 151 additions and 54 deletions

View file

@ -8,6 +8,8 @@ import android.view.WindowManager
import androidx.activity.ComponentActivity import androidx.activity.ComponentActivity
import androidx.compose.foundation.layout.requiredSize import androidx.compose.foundation.layout.requiredSize
import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.SideEffect
import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
import androidx.compose.ui.geometry.Rect import androidx.compose.ui.geometry.Rect
import androidx.compose.ui.graphics.asImageBitmap import androidx.compose.ui.graphics.asImageBitmap
@ -48,7 +50,12 @@ class DiscoverRefreshMotionTest {
@Test fun loadedCatalogueRefreshKeepsReadyActionsOpaqueAndStationary() { @Test fun loadedCatalogueRefreshKeepsReadyActionsOpaqueAndStationary() {
FakeConductarr().use { fake -> FakeConductarr().use { fake ->
fake.localArt = true fake.localArt = true
ConductarrSession(MemoryCacheStore()).use { session -> val stored = MemoryCacheStore()
val cache = object : org.justinvisser.wolfskin.conductarr.CacheStore by stored {
@Volatile var lost = false
override fun read(scope: String, key: String) = if (lost) null else stored.read(scope, key)
}
ConductarrSession(cache).use { session ->
session.select("server:shared", SHARED_TOKEN, fake.url) session.select("server:shared", SHARED_TOKEN, fake.url)
runBlocking { session.authenticate() } runBlocking { session.authenticate() }
val facade = mockk<TitleReadFacade>() val facade = mockk<TitleReadFacade>()
@ -71,6 +78,7 @@ class DiscoverRefreshMotionTest {
val bounds = rule.onNodeWithTag("title-actions").fetchSemanticsNode().boundsInRoot val bounds = rule.onNodeWithTag("title-actions").fetchSemanticsNode().boundsInRoot
val pixels = IntArray(bounds.width.toInt() * bounds.height.toInt()) val pixels = IntArray(bounds.width.toInt() * bounds.height.toInt())
val baseline = brightness(File(directory, "loaded.png"), bounds, pixels) val baseline = brightness(File(directory, "loaded.png"), bounds, pixels)
cache.lost = true
coEvery { facade.findHeld(any(), any()) } coAnswers { refreshLookup.await() } coEvery { facade.findHeld(any(), any()) } coAnswers { refreshLookup.await() }
rule.runOnIdle { rule.runOnIdle {
model.load(route, person, preferences, fixtures = false, force = true) model.load(route, person, preferences, fixtures = false, force = true)
@ -83,6 +91,16 @@ class DiscoverRefreshMotionTest {
assertEquals("Loaded actions cannot replay their rise", bounds, rule.onNodeWithTag("title-actions").fetchSemanticsNode().boundsInRoot) assertEquals("Loaded actions cannot replay their rise", bounds, rule.onNodeWithTag("title-actions").fetchSemanticsNode().boundsInRoot)
assertEquals("Loaded actions cannot replay their fade", baseline, brightness(File(directory, "refresh-${index.toString().padStart(2, '0')}.png"), bounds, pixels)) assertEquals("Loaded actions cannot replay their fade", baseline, brightness(File(directory, "refresh-${index.toString().padStart(2, '0')}.png"), bounds, pixels))
} }
refreshLookup.complete(null)
rule.waitUntil(30_000) {
Shadows.shadowOf(android.os.Looper.getMainLooper()).idle()
model.title?.data?.watchReady == true
}
repeat(12) {
rule.mainClock.advanceTimeByFrame()
assertEquals(TitleReadStatus.Ready, model.title!!.data.status)
}
assertSameFrame(File(directory, "loaded.png"), File(directory, "refreshed.png"))
} finally { } finally {
model.viewModelScope.cancel() model.viewModelScope.cancel()
refreshLookup.complete(null) refreshLookup.complete(null)
@ -134,6 +152,9 @@ class DiscoverRefreshMotionTest {
model.title?.data?.watchTarget?.watched == refreshedWatched model.title?.data?.watchTarget?.watched == refreshedWatched
} }
assertEquals(refreshedWatched, model.title!!.watched) assertEquals(refreshedWatched, model.title!!.watched)
rule.mainClock.advanceTimeBy(640)
assertEquals(TitleReadStatus.Ready, model.title!!.data.status)
assertSameFrame(File(directory, "loaded.png"), File(directory, "refreshed.png"))
} finally { model.viewModelScope.cancel() } } finally { model.viewModelScope.cancel() }
} }
} }
@ -182,6 +203,8 @@ class DiscoverRefreshMotionTest {
} }
assertEquals(TitleReadStatus.Ready, model.title!!.data.status) assertEquals(TitleReadStatus.Ready, model.title!!.data.status)
assertTrue(model.title!!.options.any { it.id == "play" }) assertTrue(model.title!!.options.any { it.id == "play" })
rule.mainClock.advanceTimeBy(640)
assertSameFrame(File(directory, "loaded.png"), File(directory, "refreshed.png"))
} finally { model.viewModelScope.cancel(); refresh.complete(held) } } finally { model.viewModelScope.cancel(); refresh.complete(held) }
} }
} }
@ -194,6 +217,12 @@ class DiscoverRefreshMotionTest {
val art = Bitmap.createBitmap(8, 8, Bitmap.Config.ARGB_8888).apply { eraseColor(android.graphics.Color.DKGRAY) }.asImageBitmap() val art = Bitmap.createBitmap(8, 8, Bitmap.Config.ARGB_8888).apply { eraseColor(android.graphics.Color.DKGRAY) }.asImageBitmap()
rule.setContent { WolfskinTheme { rule.setContent { WolfskinTheme {
model.title?.let { state -> model.title?.let { state ->
val presented = remember(model) { booleanArrayOf(false) }
val status = state.data.status
SideEffect {
if (presented[0]) assertEquals("Every rendered refresh frame stays Ready", TitleReadStatus.Ready, status)
if (status == TitleReadStatus.Ready) presented[0] = true
}
val paths = listOfNotNull(state.data.item.poster, state.data.item.backdrop) + val paths = listOfNotNull(state.data.item.poster, state.data.item.backdrop) +
state.data.cast.mapNotNull { it.portrait } + state.data.cast.mapNotNull { it.portrait } +
(state.data.related + state.data.episodes).flatMap { listOfNotNull(it.poster, it.backdrop) } (state.data.related + state.data.episodes).flatMap { listOfNotNull(it.poster, it.backdrop) }
@ -204,6 +233,15 @@ class DiscoverRefreshMotionTest {
} } } }
} }
private fun assertSameFrame(before: File, after: File) {
rule.onRoot().captureRoboImage(after.absolutePath)
val first = BitmapFactory.decodeFile(before.absolutePath)
val second = BitmapFactory.decodeFile(after.absolutePath)
try {
assertTrue("Same-data refresh must return the identical rendered frame", first.sameAs(second))
} finally { first.recycle(); second.recycle() }
}
private fun brightness(file: File, bounds: Rect, pixels: IntArray): Long { private fun brightness(file: File, bounds: Rect, pixels: IntArray): Long {
rule.onRoot().captureRoboImage(file.absolutePath) rule.onRoot().captureRoboImage(file.absolutePath)
val bitmap = BitmapFactory.decodeFile(file.absolutePath) val bitmap = BitmapFactory.decodeFile(file.absolutePath)

View file

@ -36,6 +36,31 @@ import org.robolectric.annotation.GraphicsMode
class DiscoverTitleTimingTest { class DiscoverTitleTimingTest {
@get:Rule val rule = createAndroidComposeRule<ComponentActivity>() @get:Rule val rule = createAndroidComposeRule<ComponentActivity>()
@Test fun detailsPushKeepsTheOutgoingDiscoverHeroUntilNavigationReplacesIt() {
val shell = ShellState(listOf(ShellPerson("justin", "Justin", "J", true)), "justin").apply {
pickerVisible = false; pickerLaunch = false; enterDestination("discover")
}
var pushed = ""
lateinit var discover: DiscoverState
rule.setContent {
discover = rememberDiscoverState(shell, { pushed = it }, {}, fixtures = true)
WolfskinTheme { ScreenEntry(shell, "discover", true) {
ShellSurface(shell) { DiscoverScreen(discover, it) }
} }
}
rule.waitUntil(30_000) { discover.snapshot.status == DiscoverStatus.Ready }
rule.mainClock.advanceTimeBy(640)
rule.onNodeWithText("Details").assertIsDisplayed()
val directory = File(System.getProperty("wolfskin.captureDirectory"), "discover-entry").apply { mkdirs() }
rule.onRoot().captureRoboImage(File(directory, "hero-before-push.png").absolutePath)
rule.runOnIdle { discover.openTitle(requireNotNull(discover.hero)) }
rule.waitForIdle()
assertEquals(titleRoute("movie:1930"), pushed)
rule.onNodeWithText("Details").assertIsDisplayed()
rule.onNodeWithText("For someone else").assertDoesNotExist()
rule.onRoot().captureRoboImage(File(directory, "hero-after-push.png").absolutePath)
}
@Test fun discoverDetailsFillCastAndRelatedAfterAsyncReplyAndBackRestoresDiscover() { @Test fun discoverDetailsFillCastAndRelatedAfterAsyncReplyAndBackRestoresDiscover() {
FakeConductarr().use { fake -> FakeConductarr().use { fake ->
fake.localArt = true fake.localArt = true

View file

@ -6,6 +6,13 @@ import android.os.Looper
import androidx.activity.ComponentActivity import androidx.activity.ComponentActivity
import androidx.activity.BackEventCompat import androidx.activity.BackEventCompat
import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.ui.input.key.Key
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.performKeyInput
import androidx.compose.ui.test.pressKey
import androidx.lifecycle.viewmodel.navigation3.rememberViewModelStoreNavEntryDecorator
import androidx.navigation3.runtime.rememberSaveableStateHolderNavEntryDecorator
import androidx.compose.runtime.snapshots.Snapshot import androidx.compose.runtime.snapshots.Snapshot
import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.compose.ui.test.junit4.createAndroidComposeRule
import androidx.compose.ui.test.onRoot import androidx.compose.ui.test.onRoot
@ -45,9 +52,13 @@ class TitleEntryBackTest {
@get:Rule val rule = createAndroidComposeRule<ComponentActivity>() @get:Rule val rule = createAndroidComposeRule<ComponentActivity>()
private lateinit var viewModel: WolfskinTitleViewModel private lateinit var viewModel: WolfskinTitleViewModel
private lateinit var conductarr: ConductarrSession private lateinit var conductarr: ConductarrSession
private var homeModel: WolfskinHomeViewModel? = null
private var previousFixtures: Boolean? = null
@After fun dispose() { @After fun dispose() {
if (::viewModel.isInitialized) viewModel.viewModelScope.cancel() if (::viewModel.isInitialized) viewModel.viewModelScope.cancel()
homeModel?.viewModelScope?.cancel()
previousFixtures?.let(ConductarrPreference::updateFixtures)
if (::conductarr.isInitialized) conductarr.close() if (::conductarr.isInitialized) conductarr.close()
} }
@ -56,6 +67,66 @@ class TitleEntryBackTest {
@Test fun predictiveTitleBackPopsOnlyOneEntry() = verifyOutgoingBack(personPage = false, predictive = true) @Test fun predictiveTitleBackPopsOnlyOneEntry() = verifyOutgoingBack(personPage = false, predictive = true)
@Test fun predictivePersonBackPopsOnlyOneEntry() = verifyOutgoingBack(personPage = true, predictive = true) @Test fun predictivePersonBackPopsOnlyOneEntry() = verifyOutgoingBack(personPage = true, predictive = true)
@Test fun homeTitleBackKeepsTheExactPreOpenViewport() = verifyHomeViewport(reduced = false)
@Test fun reducedMotionHomeTitleBackKeepsTheExactPreOpenViewport() = verifyHomeViewport(reduced = true)
private fun verifyHomeViewport(reduced: Boolean) {
val navigation = NavigationManager()
val viewer = ShellPerson("justin", "Justin", "J", true)
val shell = ShellState(listOf(viewer), viewer.key).apply { pickerVisible = false; pickerLaunch = false }
val snapshot = HomeFixture(rule.activity).snapshot(viewer.key)
conductarr = ConductarrSession(MemoryCacheStore())
previousFixtures = ConductarrPreference.fixtures
ConductarrPreference.updateFixtures(true)
val server = com.github.damontecres.wholphin.data.model.JellyfinServer(UUID.fromString("00000000-0000-0000-0000-000000000001"), "Fixture", "https://example.invalid", null)
val user = com.github.damontecres.wholphin.data.model.JellyfinUser(id = UUID.fromString("00000000-0000-0000-0000-000000000002"), name = "Justin", serverId = server.id, accessToken = null)
val source = WolfskinHomeViewModel(rule.activity, mockk(relaxed = true), mockk(), navigation, WolfskinSeeAllLists(), conductarr)
homeModel = source
viewModel = WolfskinTitleViewModel(rule.activity, mockk(), TitleFixtureProvider(rule.activity), navigation, conductarr,
mockk(relaxed = true), mockk(relaxed = true))
val preferences = UserPreferences(AppPreferences.getDefaultInstance(), null)
rule.setContent { CompositionLocalProvider(LocalReducedMotion provides reduced) { WolfskinTheme {
val onBack = rememberWolfskinBackHandler(shell, navigation)
NavDisplay(backStack = navigation.backStack, onBack = onBack,
entryDecorators = listOf(rememberSaveableStateHolderNavEntryDecorator(), rememberViewModelStoreNavEntryDecorator()),
entryProvider = { destination -> NavEntry(destination) {
val active = navigation.backStack.lastOrNull() == destination
val route = (destination as? Destination.Wolfskin)?.route ?: "home"
LaunchedEffect(destination, active) { if (active) shell.enterDestination(route) }
ScreenEntry(shell, route, active) {
if (destination is Destination.Home) ShellSurface(shell) { modifier ->
HomeDestination(shell, server, user, modifier, source)
} else TitleDestination(route, shell, preferences, viewModel = viewModel, fixtures = true)
}
} })
} } }
rule.waitUntil(30_000) {
Shadows.shadowOf(Looper.getMainLooper()).idle()
source.snapshot.rows.any { it.id == "cw" } && !shell.navFocused
}
rule.onRoot().performKeyInput { pressKey(Key.DirectionDown) }
rule.mainClock.advanceTimeBy(1000)
rule.waitForIdle()
val home = shell.rememberedView("home")
val tag = "home-card:${snapshot.rows[home.row].id}:0"
val before = home.listState.let { it.firstVisibleItemIndex to it.firstVisibleItemScrollOffset }
val bounds = rule.onNodeWithTag(tag).fetchSemanticsNode().boundsInRoot
val captures = File(System.getProperty("wolfskin.captureDirectory"), "home-title-back-${if (reduced) "reduced" else "animated"}").apply { mkdirs() }
rule.onRoot().captureRoboImage(File(captures, "sdk${Build.VERSION.SDK_INT}-before.png").absolutePath)
rule.onRoot().performKeyInput { pressKey(Key.DirectionCenter) }
rule.waitUntil(30_000) {
Shadows.shadowOf(Looper.getMainLooper()).idle()
viewModel.title?.data?.status == TitleReadStatus.Ready
}
rule.mainClock.advanceTimeBy(1000)
dispatchBack(predictive = false)
rule.mainClock.advanceTimeBy(1000)
rule.waitForIdle()
rule.onRoot().captureRoboImage(File(captures, "sdk${Build.VERSION.SDK_INT}-after.png").absolutePath)
assertEquals(before, home.listState.let { it.firstVisibleItemIndex to it.firstVisibleItemScrollOffset })
assertEquals(bounds, rule.onNodeWithTag(tag).fetchSemanticsNode().boundsInRoot)
}
@Test fun nativeBackKeyWaitsForSystemCompletionBeforePoppingTheTitle() { @Test fun nativeBackKeyWaitsForSystemCompletionBeforePoppingTheTitle() {
val (navigation, shell) = mountTitle(personPage = false) val (navigation, shell) = mountTitle(personPage = false)
val before = navigation.backStack.toList() val before = navigation.backStack.toList()

View file

@ -927,9 +927,12 @@ retain the approved frames except for the requested feedback changes:
cache, 128 MiB disk cache and four image-read limit remain unchanged. cache, 128 MiB disk cache and four image-read limit remain unchanged.
`HomeViewportTest` covers short/long poster, tile and wide rows, both row `HomeViewportTest` covers short/long poster, tile and wide rows, both row
directions, first focus and native Back from the actual title surface. Native directions, first focus and exact scroll/card bounds after native Back from the
proof records real Bear season-three metadata through the title read facade; actual title surface. `TitleEntryBackTest` also checks the production Home/title
the proof-only harness is not part of the shipped tests or APK. loaders through `NavDisplay` and its entry decorators on SDK 34 and 36, with
animation and reduced motion. Native proof records real Bear season-three
metadata through the title read facade; the proof-only harness is not part of
the shipped tests or APK.
Episode lists request only ProviderIds and Overview. Next up reads only identity, Episode lists request only ProviderIds and Overview. Next up reads only identity,
then hydrates the selected playback target through the existing single-item read, then hydrates the selected playback target through the existing single-item read,
@ -959,6 +962,10 @@ Selections persist through the same playback repository as the upstream player.
Marking a whole series asks first; an explicit episode remains the exact play and Marking a whole series asks first; an explicit episode remains the exact play and
watched target even while another season is selected. Nested person/title visits watched target even while another season is selected. Nested person/title visits
retain their entry's focus and selection state. retain their entry's focus and selection state.
Discover Details opens this same title destination without replacing the outgoing
Discover page with a separate preview first. The shared destination owns its
initial loading skeleton; an already-ready title keeps its header and actions
through catalogue, held-title and fixture refreshes.
Choosing another episode from an episode page replaces only the top navigation Choosing another episode from an episode page replaces only the top navigation
entry, preserving its series/Home parent. Choosing the first episode from a entry, preserving its series/Home parent. Choosing the first episode from a
series, related titles, cast and performer credits still push normal drill-ins. series, related titles, cast and performer credits still push normal drill-ins.

View file

@ -74,7 +74,6 @@ fun DiscoverScreen(state: DiscoverState, modifier: Modifier = Modifier) {
} }
} }
} }
if (state.title != null) { DiscoverTitleDestination(state, modifier); return }
state.collection?.let { state.collection?.let {
LibraryScreen(shell, it, modifier, ratingsSignedIn = state.isSignedIn, offlineArt = LocalDiscoverArt.current, registerFocus = false) LibraryScreen(shell, it, modifier, ratingsSignedIn = state.isSignedIn, offlineArt = LocalDiscoverArt.current, registerFocus = false)
return return

View file

@ -1,49 +0,0 @@
// SPDX-License-Identifier: GPL-2.0-only
package org.justinvisser.wolfskin.design
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.offset
import androidx.compose.foundation.layout.width
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.testTag
import androidx.tv.material3.Text
import org.justinvisser.wolfskin.design.WolfskinTokens.Color as C
import org.justinvisser.wolfskin.design.WolfskinTokens.Discover as D
import org.justinvisser.wolfskin.design.WolfskinTokens.Layout as L
/** Explicit destination stub exposes only the Discover-owned request and pick actions. */
@Composable
fun DiscoverTitleDestination(state: DiscoverState, modifier: Modifier = Modifier) {
val title = state.title ?: return
val focused = state.contentFocused
Box(modifier.fillMaxSize().testTag("discover-title-destination-stub")) {
DiscoverBackdrop(title, detail = true)
Column(Modifier.offset(L.safeSide, L.detailCopyTop).width(L.detailCopyWidth)) {
Text(title.title, color = C.fg, style = WolfskinTokens.Type.displayDetail, maxLines = 2)
Spacer(Modifier.height(D.heroTitleGap))
Text("${title.typeWord}${title.year?.let { " · $it" }.orEmpty()} · ${title.genres.take(2).joinToString(", ")}", color = C.muted, style = WolfskinTokens.Type.body)
Spacer(Modifier.height(D.heroMetaGap))
if (state.isSignedIn) DiscoverRatings(title.ratings)
Spacer(Modifier.height(D.heroMetaGap))
Text(title.overview, color = C.muted, style = WolfskinTokens.Type.body, maxLines = 3)
Spacer(Modifier.height(D.heroOverviewGap))
Row(horizontalArrangement = Arrangement.spacedBy(L.buttonGap)) {
if (!state.canRequest) DiscoverButton("Try again", focused) { state.refresh() } else {
val receipt = state.requests[title.key]?.firstOrNull { it.requester == state.shell.person.key }
DiscoverButton(if (receipt == null) "Request" else "Requested · ${receipt.stage}", focused && state.titleButton == 0, glyph = if (receipt == null) "plus" else "clock") {
state.titleButton = 0; state.key(ShellKey.Ok)
}
DiscoverButton(if (state.shell.person.key in title.heldBy) "Add for someone" else "For someone else", focused && state.titleButton == 1, glyph = "profile", primary = false) { state.openRequest(true) }
if (state.isPick(title)) DiscoverButton(if (state.isHidden(title)) "Show again" else "Not interested", focused && state.titleButton == 2, glyph = if (state.isHidden(title)) "undo" else "hide", primary = false) { state.titleButton = 2; state.key(ShellKey.Ok) }
}
}
}
}
}

View file

@ -15,6 +15,7 @@ import androidx.compose.ui.test.pressKey
import com.github.takahirom.roborazzi.captureRoboImage import com.github.takahirom.roborazzi.captureRoboImage
import java.io.File import java.io.File
import org.junit.Assert.assertTrue import org.junit.Assert.assertTrue
import org.junit.Assert.assertEquals
import org.junit.Rule import org.junit.Rule
import org.junit.Test import org.junit.Test
import org.junit.runner.RunWith import org.junit.runner.RunWith
@ -59,10 +60,15 @@ class HomeViewportTest {
repeat(row.count - 1) { press(Key.DirectionRight) } repeat(row.count - 1) { press(Key.DirectionRight) }
assertVisible(shell, rows) assertVisible(shell, rows)
capture("viewport-${row.id}") capture("viewport-${row.id}")
val viewport = shell.view.listState.let { it.firstVisibleItemIndex to it.firstVisibleItemScrollOffset }
val bounds = rule.onNodeWithTag("home-card:${row.id}:${row.count - 1}").fetchSemanticsNode().boundsInRoot
press(Key.DirectionCenter) press(Key.DirectionCenter)
rule.onNodeWithTag("title-page").assertIsDisplayed() rule.onNodeWithTag("title-page").assertIsDisplayed()
press(Key.Escape) press(Key.Escape)
assertVisible(shell, rows) assertVisible(shell, rows)
assertEquals("Back restores the exact scroll, not only the focused card", viewport,
shell.view.listState.let { it.firstVisibleItemIndex to it.firstVisibleItemScrollOffset })
assertEquals(bounds, rule.onNodeWithTag("home-card:${row.id}:${row.count - 1}").fetchSemanticsNode().boundsInRoot)
} }
repeat(rows.lastIndex) { press(Key.DirectionUp); assertVisible(shell, rows) } repeat(rows.lastIndex) { press(Key.DirectionUp); assertVisible(shell, rows) }
} }