diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-10-28 07:21:20 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-10-28 07:21:20 +0000 |
| commit | b6112749708cced3c9f0d1dd7fb782214cc42e41 (patch) | |
| tree | d642861c862dfe025f14624de8ca376c5d062314 /app | |
| parent | 114c15b27b01c47de212e280923bc6d35b8dfa4f (diff) | |
Library rows offer the newest waiting episode, and the car sorts like the phone
Three things Peter reported about the podcast list.
**A slow-drip show sorted on episodes it was hiding.** The library's count and
its sort key were computed from every unplayed, unqueued episode, released or
not, so New Yorker Fiction ranked near the top on a brand-new episode drip had
not handed out yet -- a show with nothing visible sitting above shows with
something to hear. Both halves now apply the same released-set the show screen
shows. The count was wrong in the same way and for the same reason, so it moves
with it.
**The row now names the one episode you would actually play**, under the show's
name and in a quieter style, with the same Play, Queue and Download controls the
episode list offers. The show stays the focus: this is a list of shows, and an
episode offering different actions depending on which screen you found it on
would be two features wearing one name. `observeNewestWaiting` sits beside
`observeAll` because the two must agree on what "waiting" means -- a row that
offered an episode the count excludes would be offering something the show
claims not to have.
**Android Auto lists shows in the library's order now.** It was sorted, but
alphabetically, which is a different order from the phone's and so reads as no
order at all. `getBrowsable` is gone rather than reordered: the callback reads
`observeAll` directly, so there is one definition of what the library contains
and what order it is in, and the car cannot drift from the phone again.
Alphabetical was also wrong on its own merits -- the car hides played episodes,
so a show you are caught up on browses as an empty list, and burying the ones
with something to hear among them is exactly what a driver can do nothing about
at a traffic light.
Pinned by `MediaBrowseTest.theCarListsShowsNewestWaitingFirst`, which walks the
real session protocol and failed on alphabetical order before the change, five
new drip cases and five newest-waiting cases in `LibraryOrderTest` against a
real database, and `LibraryRowTest`, which drives the row as a composable
because a row offering the wrong episode's controls looks exactly like one
offering the right episode's.
`LibraryScreen.kt` split rather than relaxing detekt again, same call as
`Screens.kt`: the row and its parts are now `SubscriptionRow.kt`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
Diffstat (limited to 'app')
10 files changed, 604 insertions, 101 deletions
diff --git a/app/src/androidTest/java/org/terst/listen/data/LibraryOrderTest.kt b/app/src/androidTest/java/org/terst/listen/data/LibraryOrderTest.kt index 09a443e..3df28d0 100644 --- a/app/src/androidTest/java/org/terst/listen/data/LibraryOrderTest.kt +++ b/app/src/androidTest/java/org/terst/listen/data/LibraryOrderTest.kt @@ -261,4 +261,139 @@ class LibraryOrderTest { assertEquals(false, byId["quiet"]!!.hasWaiting) assertEquals(true, byId["busy"]!!.hasWaiting) } + + // --- drip shows ------------------------------------------------------------------------- + + private suspend fun dripShow(id: String) = subscriptions.insert( + SubscriptionEntity( + id = id, feedUrl = "https://example.com/$id", title = id, addedAtMs = 0, + dripEnabled = true, + ) + ) + + private suspend fun release(id: String, atMs: Long) = episodes.markDripReleased(id, atMs) + + /** + * Reported live: The New Yorker: Fiction showed 235 waiting and sorted near the top, while + * opening it showed nothing to listen to. Drip is holding the back catalogue back, so those + * 235 are episodes he cannot reach -- counting them says the show has something for him when + * it does not. + */ + @Test + fun aDripShowCountsOnlyWhatItHasActuallyReleased() = runBlocking { + dripShow("drip") + episode("a", "drip", day) + episode("b", "drip", 2 * day) + episode("c", "drip", 3 * day) + release("a", day) + + assertEquals(1, listing().single().waiting) + } + + @Test + fun aDripShowWithNothingReleasedHasNothingWaiting() = runBlocking { + dripShow("drip") + episode("a", "drip", day) + episode("b", "drip", 2 * day) + + val only = listing().single() + assertEquals(0, only.waiting) + assertEquals(false, only.hasWaiting) + } + + /** + * The sort key has to come from the same set as the count. A brand-new episode the feed just + * published, still held back by drip, must not lift the show to the top of a list of things + * to listen to. + */ + @Test + fun anUnreleasedEpisodeDoesNotLiftADripShowUpTheList() = runBlocking { + show("ordinary") + episode("recent", "ordinary", 5 * day) + dripShow("drip") + episode("old-released", "drip", day) + episode("brand-new-held-back", "drip", 9 * day) + release("old-released", day) + + assertEquals(listOf("ordinary", "drip"), listing().map { it.subscription.id }) + } + + @Test + fun aDripShowRanksOnItsNewestReleasedEpisode() = runBlocking { + show("ordinary") + episode("middling", "ordinary", 3 * day) + dripShow("drip") + episode("released-newer", "drip", 4 * day) + episode("held-back", "drip", 9 * day) + release("released-newer", 4 * day) + + assertEquals(listOf("drip", "ordinary"), listing().map { it.subscription.id }) + } + + @Test + fun anOrdinaryShowIsUnaffectedByTheDripRule() = runBlocking { + show("ordinary") + episode("a", "ordinary", day) + episode("b", "ordinary", 2 * day) + + assertEquals(2, listing().single().waiting) + } + + // --- the newest thing waiting, per show --------------------------------------------------- + + private suspend fun newest(bedtime: Boolean = false) = + subscriptions.observeNewestWaiting(bedtime).first().associate { it.subscriptionId to it.id } + + // The library row names the show, and under it the one episode you would actually play. + @Test + fun reportsTheNewestWaitingEpisodeForEachShow() = runBlocking { + show("a") + show("b") + episode("a-old", "a", day) + episode("a-new", "a", 3 * day) + episode("b-only", "b", 2 * day) + + assertEquals(mapOf("a" to "a-new", "b" to "b-only"), newest()) + } + + @Test + fun aShowWithNothingWaitingOffersNothing() = runBlocking { + show("caught-up") + episode("done", "caught-up", day) + episodes.markPlayed("done", 1) + + assertEquals(emptyMap<String, String>(), newest()) + } + + @Test + fun aQueuedEpisodeIsNotWhatTheRowOffers() = runBlocking { + show("s") + episode("queued", "s", 3 * day) + episode("waiting", "s", day) + queue.upsertAll(listOf(QueueEntity("queued", 0))) + + assertEquals(mapOf("s" to "waiting"), newest()) + } + + // Same rule as the count: an episode drip has not released is not something he can play. + @Test + fun aDripShowOffersItsNewestReleasedEpisode() = runBlocking { + dripShow("drip") + episode("released", "drip", day) + episode("held-back", "drip", 9 * day) + release("released", day) + + assertEquals(mapOf("drip" to "released"), newest()) + } + + @Test + fun bedtimeShowsAreTheirOwnList() = runBlocking { + show("awake") + show("asleep", bedtime = true) + episode("day", "awake", day) + episode("night", "asleep", day) + + assertEquals(mapOf("awake" to "day"), newest()) + assertEquals(mapOf("asleep" to "night"), newest(bedtime = true)) + } } diff --git a/app/src/androidTest/java/org/terst/listen/playback/MediaBrowseTest.kt b/app/src/androidTest/java/org/terst/listen/playback/MediaBrowseTest.kt index 640849a..5e3086e 100644 --- a/app/src/androidTest/java/org/terst/listen/playback/MediaBrowseTest.kt +++ b/app/src/androidTest/java/org/terst/listen/playback/MediaBrowseTest.kt @@ -13,7 +13,9 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith +import org.terst.listen.data.EpisodeEntity import org.terst.listen.data.Graph +import org.terst.listen.data.SubscriptionEntity /** * Browses the media library the way Android Auto does: over the session protocol from a @@ -129,4 +131,61 @@ class MediaBrowseTest { withContext(Dispatchers.Main) { browser.release() } } } + + /** + * The car lists shows in the same order the phone does: the one with the newest thing still + * waiting first. + * + * Reported from the car -- the list "isn't sorted". It was sorted, alphabetically, which is + * a different order from the phone's and so reads as none at all. Alphabetical is also the + * wrong order here on its own merits: the car hides played episodes, so a show you are + * caught up on browses as an empty list, and burying the ones with something to hear among + * them is exactly what a driver cannot do anything about at a traffic light. + * + * Seeded through the DAOs rather than by subscribing: two real feeds both publish recently, + * so their relative order would be whatever the publishers did that week. + */ + @Test + fun theCarListsShowsNewestWaitingFirst() = runBlocking { + val graph = Graph.get(context) + val day = 24 * 60 * 60 * 1000L + // Titles deliberately in the opposite order to the recency they will rank by. + graph.subscriptions.insert(seedShow("browse-zebra", "Zebra Hour")) + graph.subscriptions.insert(seedShow("browse-apple", "Apple Hour")) + graph.episodes.insertAll(listOf(seedEpisode("browse-z-ep", "browse-zebra", 3 * day))) + graph.episodes.insertAll(listOf(seedEpisode("browse-a-ep", "browse-apple", day))) + + val browser = connect() + try { + withContext(Dispatchers.Main) { + val shows = browser.getChildren(LibraryTree.SUBSCRIPTIONS, 0, 100, null) + .await().value!! + .mapNotNull { it.mediaMetadata.title?.toString() } + .filter { it.endsWith(" Hour") } + assertEquals(listOf("Zebra Hour", "Apple Hour"), shows) + } + } finally { + withContext(Dispatchers.Main) { browser.release() } + graph.subscriptions.delete("browse-zebra") + graph.subscriptions.delete("browse-apple") + } + } + + private fun seedShow(id: String, title: String) = SubscriptionEntity( + id = id, + feedUrl = "https://example.invalid/$id", + title = title, + addedAtMs = 0, + ) + + private fun seedEpisode(id: String, showId: String, publishedAtMs: Long) = EpisodeEntity( + id = id, + subscriptionId = showId, + guid = id, + title = id, + mediaUrl = "https://example.invalid/$id.mp3", + mediaType = "audio/mpeg", + publishedAtMs = publishedAtMs, + discoveredAtMs = 0, + ) } diff --git a/app/src/androidTest/java/org/terst/listen/ui/LibraryRowTest.kt b/app/src/androidTest/java/org/terst/listen/ui/LibraryRowTest.kt new file mode 100644 index 0000000..41dc682 --- /dev/null +++ b/app/src/androidTest/java/org/terst/listen/ui/LibraryRowTest.kt @@ -0,0 +1,137 @@ +package org.terst.listen.ui + +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithContentDescription +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertEquals +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.terst.listen.data.EpisodeEntity +import org.terst.listen.data.SubscriptionEntity +import org.terst.listen.data.SubscriptionListing + +/** + * A library row: the show's name, how much is waiting, and the one episode it offers. + * + * Driven as a composable because the thing that can go wrong is wiring, not appearance -- a row + * offering the wrong episode's controls looks exactly like a row offering the right one's, and + * the library draws sixty of them. + */ +@RunWith(AndroidJUnit4::class) +class LibraryRowTest { + + @get:Rule + val compose = createComposeRule() + + private fun show(id: String, title: String) = SubscriptionEntity( + id = id, + feedUrl = "https://example.invalid/$id", + title = title, + addedAtMs = 0, + ) + + private fun episode(id: String, showId: String, title: String) = EpisodeEntity( + id = id, + subscriptionId = showId, + guid = id, + title = title, + mediaUrl = "https://example.invalid/$id.mp3", + mediaType = "audio/mpeg", + publishedAtMs = 0, + discoveredAtMs = 0, + ) + + private fun noActions() = EpisodeActions( + play = {}, enqueue = {}, playNext = {}, download = {}, openDetails = {}, togglePlayed = {}, + ) + + @Test + fun theShowNameAndWhatIsWaitingAreBothOnTheRow() { + compose.setContent { + SubscriptionRow( + listing = SubscriptionListing(show("s", "Zebra Hour"), waiting = 3), + latest = episode("e", "s", "The one about stripes"), + actions = noActions(), + ) {} + } + + compose.onNodeWithText("Zebra Hour").assertIsDisplayed() + compose.onNodeWithText("3").assertIsDisplayed() + compose.onNodeWithText("The one about stripes").assertIsDisplayed() + compose.onNodeWithContentDescription("Download").assertIsDisplayed() + } + + /** + * A show with nothing waiting offers nothing, rather than offering an episode the count + * says is not there. + */ + @Test + fun aShowWithNothingWaitingOffersNoEpisode() { + compose.setContent { + SubscriptionRow( + listing = SubscriptionListing(show("s", "Caught Up"), waiting = 0), + latest = null, + actions = null, + ) {} + } + + compose.onNodeWithText("Caught Up").assertIsDisplayed() + compose.onNodeWithText("—").assertIsDisplayed() + // The controls belong to the episode, so with no episode there is nothing to control. + compose.onNodeWithContentDescription("Download").assertDoesNotExist() + } + + /** The point of the controls: they act on the episode named beside them, not on the show. */ + @Test + fun playActsOnTheEpisodeTheRowNames() { + var played = 0 + compose.setContent { + SubscriptionRow( + listing = SubscriptionListing(show("s", "Zebra Hour"), waiting = 1), + latest = episode("e", "s", "The one about stripes"), + actions = noActions().copy(play = { played++ }), + ) {} + } + + compose.onNodeWithContentDescription("Play The one about stripes").performClick() + + assertEquals(1, played) + } + + @Test + fun queueActsOnTheEpisodeTheRowNames() { + var queued = 0 + compose.setContent { + SubscriptionRow( + listing = SubscriptionListing(show("s", "Zebra Hour"), waiting = 1), + latest = episode("e", "s", "The one about stripes"), + actions = noActions().copy(enqueue = { queued++ }), + ) {} + } + + compose.onNodeWithContentDescription("Queue The one about stripes").performClick() + + assertEquals(1, queued) + } + + /** Tapping the row still opens the show: the episode controls sit inside it, not over it. */ + @Test + fun tappingTheRowOpensTheShow() { + var opened = 0 + compose.setContent { + SubscriptionRow( + listing = SubscriptionListing(show("s", "Zebra Hour"), waiting = 1), + latest = episode("e", "s", "The one about stripes"), + actions = noActions(), + ) { opened++ } + } + + compose.onNodeWithText("Zebra Hour").performClick() + + assertEquals(1, opened) + } +} diff --git a/app/src/main/java/org/terst/listen/data/Daos.kt b/app/src/main/java/org/terst/listen/data/Daos.kt index 292424a..74c8ad5 100644 --- a/app/src/main/java/org/terst/listen/data/Daos.kt +++ b/app/src/main/java/org/terst/listen/data/Daos.kt @@ -43,6 +43,11 @@ interface SubscriptionDao { * Queued episodes are excluded for the same reason played ones are: they have been dealt * with. Peter's words -- once they're on the queue they're noise in the list. * + * So are episodes a drip show has not released yet, and for the same reason again: they + * cannot be reached. Counting them said The New Yorker: Fiction had 235 waiting and sorted + * it near the top, while opening it showed nothing to listen to. The count and the sort key + * both come from the released set now, which is the set [getVisible] shows. + * * [bedtime] picks which of the two lists this is. They are disjoint, not a filter over one * list: a bedtime show is absent from the ordinary library entirely, which is the point. * @@ -53,11 +58,14 @@ interface SubscriptionDao { */ @Query( """SELECT s.*, COALESCE(e.waiting, 0) AS waiting FROM subscriptions s - LEFT JOIN (SELECT subscriptionId, COUNT(*) AS waiting, - MAX(COALESCE(publishedAtMs, 0)) AS latest - FROM episodes - WHERE playedAtMs IS NULL AND id NOT IN (SELECT episodeId FROM queue) - GROUP BY subscriptionId) e + LEFT JOIN (SELECT ep.subscriptionId AS subscriptionId, COUNT(*) AS waiting, + MAX(COALESCE(ep.publishedAtMs, 0)) AS latest + FROM episodes ep + JOIN subscriptions owner ON owner.id = ep.subscriptionId + WHERE ep.playedAtMs IS NULL + AND ep.id NOT IN (SELECT episodeId FROM queue) + AND (owner.dripEnabled = 0 OR ep.dripReleasedAtMs IS NOT NULL) + GROUP BY ep.subscriptionId) e ON e.subscriptionId = s.id WHERE s.bedtime = :bedtime ORDER BY COALESCE(e.latest, 0) DESC, s.title COLLATE NOCASE""" @@ -65,6 +73,30 @@ interface SubscriptionDao { fun observeAll(bedtime: Boolean): Flow<List<SubscriptionListing>> /** + * The newest episode still waiting on each show -- what the library row offers under the + * show's name. + * + * "Waiting" is the same set the library counts: unplayed, unqueued, and released if the + * show drips. A row that offered an episode the count does not include would be offering + * something the show claims not to have. + * + * One query for every show rather than one per show: the library observes this alongside + * the show list, and a per-show query would be sixty round trips on every change to any of + * them. SQLite guarantees the bare columns of a `GROUP BY` come from the row that produced + * the `MAX`, which is what picks the newest. + */ + @Query( + """SELECT ep.*, MAX(COALESCE(ep.publishedAtMs, 0)) AS newest FROM episodes ep + JOIN subscriptions owner ON owner.id = ep.subscriptionId + WHERE ep.playedAtMs IS NULL + AND ep.id NOT IN (SELECT episodeId FROM queue) + AND (owner.dripEnabled = 0 OR ep.dripReleasedAtMs IS NOT NULL) + AND owner.bedtime = :bedtime + GROUP BY ep.subscriptionId""" + ) + fun observeNewestWaiting(bedtime: Boolean): Flow<List<EpisodeEntity>> + + /** * Every show, bedtime ones included. * * Deliberately not the library query: a rule may well target a bedtime show, and a screen @@ -76,16 +108,6 @@ interface SubscriptionDao { @Query("SELECT * FROM subscriptions ORDER BY title COLLATE NOCASE") suspend fun getAll(): List<SubscriptionEntity> - /** - * The shows Android Auto lists, which is everything except bedtime ones. - * - * Driving is the opposite of falling asleep, and the car list is scrolled blind at a - * traffic light -- the same reason played episodes never appear there. Voice search still - * goes through [getAll]: asking for a show by name is unambiguous about wanting it. - */ - @Query("SELECT * FROM subscriptions WHERE bedtime = 0 ORDER BY title COLLATE NOCASE") - suspend fun getBrowsable(): List<SubscriptionEntity> - @Query("SELECT * FROM subscriptions WHERE id = :id") suspend fun getById(id: String): SubscriptionEntity? diff --git a/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt b/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt index f0c9ae3..723c406 100644 --- a/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt +++ b/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt @@ -17,6 +17,7 @@ import com.google.common.util.concurrent.Futures import com.google.common.util.concurrent.ListenableFuture import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.first import kotlinx.coroutines.guava.future import org.terst.listen.data.Graph @@ -118,7 +119,13 @@ class ListenSessionCallback( } parentId == LibraryTree.SUBSCRIPTIONS -> - graph.subscriptions.getBrowsable().map(LibraryTree::subscription) + // The library's own query, not a second one ordered differently: the car lists + // shows newest-waiting-first for the same reason the phone does, and a separate + // ordering here is what made the car list look unsorted. Bedtime shows are left + // out by it -- driving is the opposite of falling asleep -- while voice search + // still goes through getAll, since asking for a show by name is unambiguous. + graph.subscriptions.observeAll(bedtime = false).first() + .map { LibraryTree.subscription(it.subscription) } LibraryTree.subscriptionId(parentId) != null -> { val id = LibraryTree.subscriptionId(parentId)!! diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt b/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt index dd91e18..dac7902 100644 --- a/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt +++ b/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt @@ -89,14 +89,15 @@ internal fun EpisodeRow(episode: EpisodeEntity, actions: EpisodeActions, artwork } } +/** Internal so the library row can offer the same controls without a second set of icons. */ @Composable -private fun ActionIcon(icon: ImageVector, description: String, onClick: () -> Unit) { +internal fun ActionIcon(icon: ImageVector, description: String, onClick: () -> Unit) { IconButton(onClick = onClick) { Icon(icon, contentDescription = description) } } /** Download state is the icon itself, so the row doesn't need a separate progress line. */ @Composable -private fun DownloadIcon(state: DownloadState, onDownload: () -> Unit) { +internal fun DownloadIcon(state: DownloadState, onDownload: () -> Unit) { when (state) { DownloadState.DOWNLOADING -> IconButton(onClick = {}, enabled = false) { diff --git a/app/src/main/java/org/terst/listen/ui/LibraryScreen.kt b/app/src/main/java/org/terst/listen/ui/LibraryScreen.kt index 178a19e..a926ce6 100644 --- a/app/src/main/java/org/terst/listen/ui/LibraryScreen.kt +++ b/app/src/main/java/org/terst/listen/ui/LibraryScreen.kt @@ -10,6 +10,7 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.foundation.shape.CircleShape @@ -18,6 +19,8 @@ import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Add import androidx.compose.material.icons.filled.Bedtime import androidx.compose.material.icons.filled.LightMode +import androidx.compose.material.icons.filled.PlayArrow +import androidx.compose.material.icons.filled.PlaylistAdd import androidx.compose.material.icons.filled.Refresh import androidx.compose.material.icons.filled.Settings import androidx.compose.material3.Button @@ -52,12 +55,14 @@ import org.terst.listen.data.DownloadState import org.terst.listen.data.EpisodeEntity import org.terst.listen.data.SubscriptionEntity import org.terst.listen.data.SubscriptionListing +import org.terst.listen.playback.NowPlaying import java.util.concurrent.TimeUnit @OptIn(ExperimentalMaterial3Api::class) @Composable fun LibraryScreen(vm: ListenViewModel, player: PlayerViewModel, nav: LibraryNav) { val subscriptions by vm.library.collectAsState() + val newestWaiting by vm.newestWaiting.collectAsState() val queue by vm.queue.collectAsState() val refreshing by vm.refreshing.collectAsState() val banner by vm.banner.collectAsState() @@ -94,18 +99,7 @@ fun LibraryScreen(vm: ListenViewModel, player: PlayerViewModel, nav: LibraryNav) ) }, snackbarHost = { SnackbarHost(snackbar) { Snackbar(it) } }, - bottomBar = { - NowPlayingBar( - state = nowPlaying, - actions = NowPlayingActions( - togglePlayPause = player::togglePlayPause, - skipBack = player::skipBack, - skipForward = player::skipForward, - openEpisode = { nowPlaying.episodeId?.let(nav.openEpisode) }, - openQueue = nav.openQueue, - ), - ) - }, + bottomBar = { LibraryNowPlayingBar(nowPlaying, player, nav) }, ) { padding -> Column(Modifier.padding(padding).fillMaxSize()) { if (refreshing) { @@ -115,11 +109,34 @@ fun LibraryScreen(vm: ListenViewModel, player: PlayerViewModel, nav: LibraryNav) AddSubscription(onSubmit = { vm.subscribe(it); adding = false }) } - LibraryList(subscriptions, queue, bedtime, nav.openSubscription, nav.openQueue) + LibraryList( + LibraryContent(subscriptions, newestWaiting, queue, bedtime), + onOpenSubscription = nav.openSubscription, + onOpenQueue = nav.openQueue, + actionsFor = { episode -> libraryEpisodeActions(vm, player, episode, nav) }, + ) } } } +@Composable +private fun LibraryNowPlayingBar( + nowPlaying: NowPlaying, + player: PlayerViewModel, + nav: LibraryNav, +) { + NowPlayingBar( + state = nowPlaying, + actions = NowPlayingActions( + togglePlayPause = player::togglePlayPause, + skipBack = player::skipBack, + skipForward = player::skipForward, + openEpisode = { nowPlaying.episodeId?.let(nav.openEpisode) }, + openQueue = nav.openQueue, + ), + ) +} + /** The top bar's four callbacks, bundled so the signature stays readable. */ private data class LibraryTopBarActions( val toggleAdd: () -> Unit, @@ -157,14 +174,25 @@ private fun LibraryTopBar( ) } +/** What the library list draws, bundled so the signature stays readable. */ +internal data class LibraryContent( + val subscriptions: List<SubscriptionListing>, + /** The newest episode still waiting on each show, keyed by show id. */ + val newestWaiting: Map<String, EpisodeEntity>, + val queue: List<EpisodeEntity>, + val bedtime: Boolean, +) + @Composable -private fun LibraryList( - subscriptions: List<SubscriptionListing>, - queue: List<EpisodeEntity>, - bedtime: Boolean, +internal fun LibraryList( + content: LibraryContent, onOpenSubscription: (String) -> Unit, onOpenQueue: () -> Unit, + actionsFor: (EpisodeEntity) -> EpisodeActions, ) { + val subscriptions = content.subscriptions + val queue = content.queue + val bedtime = content.bedtime LazyColumn(Modifier.fillMaxSize()) { if (queue.isNotEmpty()) { item { SectionHeading("Up next \u00b7 ${queue.size}") } @@ -176,7 +204,12 @@ private fun LibraryList( item { SectionHeading("Shows") } } items(subscriptions, key = { it.subscription.id }) { listing -> - SubscriptionRow(listing) { onOpenSubscription(listing.subscription.id) } + val latest = content.newestWaiting[listing.subscription.id] + SubscriptionRow( + listing = listing, + latest = latest, + actions = latest?.let(actionsFor), + ) { onOpenSubscription(listing.subscription.id) } } } } @@ -228,9 +261,6 @@ private fun UpNextRow(episode: EpisodeEntity, onOpenQueue: () -> Unit) { } } -/** Dimmed rather than hidden: a show you are caught up on is still one you subscribe to. */ -private const val CAUGHT_UP_ALPHA = 0.38f - @Composable private fun EmptyBedtime() { Column(Modifier.fillMaxWidth().padding(32.dp), horizontalAlignment = Alignment.CenterHorizontally) { @@ -271,64 +301,3 @@ private fun AddSubscription(onSubmit: (String) -> Unit) { Button(onClick = { onSubmit(text) }, enabled = text.isNotBlank()) { Text("Add") } } } - -/** - * One show. Dimmed when it has nothing waiting, so a scan down the list separates the shows - * with something to hear from the ones you are caught up on. - * - * The dimming stops short of [SubscriptionEntity.lastError], which keeps its error colour: a - * feed that is failing to refresh looks exactly like a feed with nothing new, and that is the - * one case where "nothing waiting" is a lie worth shouting about. - */ -@Composable -private fun SubscriptionRow(listing: SubscriptionListing, onClick: () -> Unit) { - val subscription = listing.subscription - val alpha = if (listing.hasWaiting) 1f else CAUGHT_UP_ALPHA - - Row( - Modifier.fillMaxWidth().clickable(onClick = onClick).padding(horizontal = 16.dp, vertical = 10.dp), - verticalAlignment = Alignment.CenterVertically, - ) { - // A dot rather than artwork. Forty-three shows is a list to scan, not a shelf to browse, - // and the one thing worth seeing per row is whether there is anything in it. - Box( - Modifier - .size(WAITING_DOT) - .clip(CircleShape) - .background( - if (listing.hasWaiting) MaterialTheme.colorScheme.primary - else MaterialTheme.colorScheme.surfaceVariant - ) - ) - Column(Modifier.weight(1f).padding(start = 12.dp)) { - Text( - subscription.title, - style = MaterialTheme.typography.bodyLarge, - color = MaterialTheme.colorScheme.onSurface.copy(alpha = alpha), - maxLines = 1, - overflow = TextOverflow.Ellipsis, - ) - // A feed that has quietly stopped working is otherwise indistinguishable from one - // with nothing new, which is the whole reason the error is stored rather than logged. - // Kept when everything else about the row got shorter: it is the only line here that - // reports something being wrong. - subscription.lastError?.let { - Text( - "Last refresh failed: $it", - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.error, - maxLines = 2, - overflow = TextOverflow.Ellipsis, - ) - } - } - Text( - if (listing.hasWaiting) listing.waiting.toString() else "\u2014", - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = alpha), - ) - } -} - -/** How big the "something waiting" dot is. Small enough to read as punctuation, not a control. */ -private val WAITING_DOT = 8.dp diff --git a/app/src/main/java/org/terst/listen/ui/ListenViewModel.kt b/app/src/main/java/org/terst/listen/ui/ListenViewModel.kt index 3115c32..a9f95da 100644 --- a/app/src/main/java/org/terst/listen/ui/ListenViewModel.kt +++ b/app/src/main/java/org/terst/listen/ui/ListenViewModel.kt @@ -10,6 +10,7 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import org.terst.listen.data.EpisodeEntity @@ -49,6 +50,19 @@ class ListenViewModel(app: Application) : AndroidViewModel(app) { .stateIn(viewModelScope, SharingStarted.WhileSubscribed(STOP_TIMEOUT_MS), emptyList()) /** + * The newest episode still waiting on each show, keyed by show. + * + * What the library row offers under the show's name. Observed as one query across every + * show rather than one per row: sixty rows asking separately would be sixty round trips on + * every change to any of them. + */ + @OptIn(ExperimentalCoroutinesApi::class) + val newestWaiting: StateFlow<Map<String, EpisodeEntity>> = + _bedtime.flatMapLatest { graph.subscriptions.observeNewestWaiting(it) } + .map { episodes -> episodes.associateBy { it.subscriptionId } } + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(STOP_TIMEOUT_MS), emptyMap()) + + /** * Every show, for the screens that look one up by id. * * Separate from [library] on purpose: a show screen opened from bedtime mode must survive diff --git a/app/src/main/java/org/terst/listen/ui/SubscriptionRow.kt b/app/src/main/java/org/terst/listen/ui/SubscriptionRow.kt new file mode 100644 index 0000000..1ffa4a9 --- /dev/null +++ b/app/src/main/java/org/terst/listen/ui/SubscriptionRow.kt @@ -0,0 +1,156 @@ +package org.terst.listen.ui + +import androidx.compose.foundation.background +import androidx.compose.foundation.clickable +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.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.shape.CircleShape +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.PlayArrow +import androidx.compose.material.icons.filled.PlaylistAdd +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import org.terst.listen.data.EpisodeEntity +import org.terst.listen.data.SubscriptionEntity +import org.terst.listen.data.SubscriptionListing + +/** Dimmed rather than hidden: a show you are caught up on is still one you subscribe to. */ +private const val CAUGHT_UP_ALPHA = 0.38f + +/** + * One show: its name, how much is waiting, and the one episode you would actually play. + * + * The show name stays the focus -- this is a list of shows, not of episodes -- so the episode + * sits under it in a quieter style with its own controls. Those controls are the same ones the + * episode list offers, because an episode offering different actions depending on which screen + * you found it on would be two features wearing one name. + * + * Dimmed when nothing is waiting, so a scan down the list separates the shows with something to + * hear from the ones you are caught up on. The dimming stops short of + * [SubscriptionEntity.lastError], which keeps its error colour: a feed that is failing to + * refresh looks exactly like a feed with nothing new, and that is the one case where "nothing + * waiting" is a lie worth shouting about. + */ +@Composable +internal fun SubscriptionRow( + listing: SubscriptionListing, + latest: EpisodeEntity?, + actions: EpisodeActions?, + onClick: () -> Unit, +) { + val subscription = listing.subscription + val alpha = if (listing.hasWaiting) 1f else CAUGHT_UP_ALPHA + + Column(Modifier.fillMaxWidth().clickable(onClick = onClick).padding(start = 16.dp, end = 4.dp)) { + ShowHeadline(listing, alpha) + if (latest != null && actions != null) LatestEpisodeLine(latest, actions) + + // A feed that has quietly stopped working is otherwise indistinguishable from one with + // nothing new, which is the whole reason the error is stored rather than logged. + subscription.lastError?.let { + Text( + "Last refresh failed: $it", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.error, + maxLines = 2, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.padding(start = WAITING_DOT + 12.dp, bottom = 4.dp), + ) + } + Spacer(Modifier.size(6.dp)) + } +} + +/** The show's own line: a dot for whether anything is waiting, its name, and how much. */ +@Composable +private fun ShowHeadline(listing: SubscriptionListing, alpha: Float) { + Row(Modifier.fillMaxWidth().padding(top = 10.dp), verticalAlignment = Alignment.CenterVertically) { + // A dot rather than artwork. Forty-three shows is a list to scan, not a shelf to + // browse, and the one thing worth seeing per row is whether there is anything in it. + Box( + Modifier + .size(WAITING_DOT) + .clip(CircleShape) + .background( + if (listing.hasWaiting) MaterialTheme.colorScheme.primary + else MaterialTheme.colorScheme.surfaceVariant + ) + ) + Text( + listing.subscription.title, + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurface.copy(alpha = alpha), + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.weight(1f).padding(start = 12.dp), + ) + Text( + if (listing.hasWaiting) listing.waiting.toString() else "\u2014", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = alpha), + modifier = Modifier.padding(end = 12.dp), + ) + } +} + +/** + * The newest waiting episode, indented under the show's name and offered directly. + * + * Indented to the show title's text, not the dot: the dot belongs to the show, and lining the + * episode up with it would read as a second show rather than as this one's contents. + */ +@Composable +private fun LatestEpisodeLine(latest: EpisodeEntity, actions: EpisodeActions) { + Row( + Modifier.fillMaxWidth().padding(start = WAITING_DOT + 12.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + latest.title, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.weight(1f), + ) + ActionIcon(Icons.Default.PlayArrow, "Play ${latest.title}", actions.play) + ActionIcon(Icons.Default.PlaylistAdd, "Queue ${latest.title}", actions.enqueue) + DownloadIcon(latest.downloadState, actions.download) + } +} + +/** How big the "something waiting" dot is. Small enough to read as punctuation, not a control. */ +private val WAITING_DOT = 8.dp + +/** + * The controls the library row offers on a show's newest waiting episode. + * + * Deliberately the same actions the episode list builds, so an episode does not behave + * differently depending on which screen it was found on. + */ +internal fun libraryEpisodeActions( + vm: ListenViewModel, + player: PlayerViewModel, + episode: EpisodeEntity, + nav: LibraryNav, +) = EpisodeActions( + play = { player.play(episode) }, + enqueue = { vm.enqueue(episode.id) }, + playNext = { vm.playNext(episode.id) }, + download = { vm.download(episode) }, + openDetails = { nav.openEpisode(episode.id) }, + togglePlayed = { + if (episode.playedAtMs == null) vm.markPlayed(episode.id) else vm.markUnplayed(episode.id) + }, +) diff --git a/app/src/test/java/org/terst/listen/data/FakeDaos.kt b/app/src/test/java/org/terst/listen/data/FakeDaos.kt index a285721..54ef70c 100644 --- a/app/src/test/java/org/terst/listen/data/FakeDaos.kt +++ b/app/src/test/java/org/terst/listen/data/FakeDaos.kt @@ -19,8 +19,11 @@ class FakeSubscriptionDao : SubscriptionDao { flowOf(rows.filter { it.bedtime == bedtime }.map { SubscriptionListing(it, waiting = 1) }) override fun observeEvery(): Flow<List<SubscriptionEntity>> = flowOf(rows.toList()) + + // Which episode is newest and still waiting is SQL too, and no JVM caller branches on it: + // the library screen is the only consumer. LibraryOrderTest pins it on a real database. + override fun observeNewestWaiting(bedtime: Boolean): Flow<List<EpisodeEntity>> = flowOf(emptyList()) override suspend fun getAll() = rows.toList() - override suspend fun getBrowsable() = rows.filterNot { it.bedtime } override suspend fun getById(id: String) = rows.firstOrNull { it.id == id } override suspend fun getByFeedUrl(feedUrl: String) = rows.firstOrNull { it.feedUrl == feedUrl } |
