summaryrefslogtreecommitdiff
path: root/app/src
diff options
context:
space:
mode:
authorPeter Stone <thepeterstone@gmail.com>2026-10-27 07:02:48 +0000
committerPeter Stone <thepeterstone@gmail.com>2026-10-27 07:02:48 +0000
commitef20a5421c322ef8053f5c2766d851873bc472f3 (patch)
tree4f3fb41eeeb9e361d0b3d5c88395987da7d2eeee /app/src
parent85aa11207fb64a86eddd0591ab5a189d22433f7e (diff)
Follow the player, and put the ad button on the episode screen
Peter: "combining the player and episode screens is weird. Now I end up with a different episode playing than shown because it doesn't follow what's playing. And there should be an ad skip button in the player." Both are consequences of the merge rather than arguments against it. The screen is the player now, so when the queue advanced the screen stayed on the episode that had finished -- and its transport then belonged to an episode nobody was listening to, so pressing play there would restart it over the one that was. It follows only when it was already showing the playing episode. Opening an older episode while something else plays is a deliberate act, and a screen that snatched itself away mid-read would be a worse bug than this one. The route argument could not answer it alone: the queue advances with no navigation at all. The ad button sends the same session command the notification button sends rather than reimplementing it. That gesture is four meanings deep -- press, press again, come back inside the jump, come back past it -- and two copies of it would drift. It carries the same circle-and-slash so the two read as one control in two places. It is on the episode screen for the same reason it is on the notification: detection accounts for roughly 38% of the advertising the feeds imply, so most of what gets heard was never found. Verified on the emulator with two twenty-second episodes queued: the screen was showing the first, playback reached its end, and the screen moved to the second on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
Diffstat (limited to 'app/src')
-rw-r--r--app/src/main/java/org/terst/listen/playback/PlayerConnection.kt12
-rw-r--r--app/src/main/java/org/terst/listen/ui/EpisodeDetailActions.kt5
-rw-r--r--app/src/main/java/org/terst/listen/ui/EpisodeFollowing.kt23
-rw-r--r--app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt26
-rw-r--r--app/src/main/java/org/terst/listen/ui/EpisodeTransport.kt12
-rw-r--r--app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt3
-rw-r--r--app/src/test/java/org/terst/listen/ui/EpisodeFollowingTest.kt55
7 files changed, 128 insertions, 8 deletions
diff --git a/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt b/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt
index d88414f..14e7f4f 100644
--- a/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt
+++ b/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt
@@ -135,6 +135,18 @@ class PlayerConnection(private val context: Context) {
publish()
}
+ /**
+ * "This is an ad": records the claim and jumps past it.
+ *
+ * Sent as the same session command the notification button sends, rather than reimplemented
+ * here. The gesture is four meanings deep -- press, press again, come back inside the jump,
+ * come back past it -- and two copies of that would drift. The service owns it; see
+ * [AdReporter].
+ */
+ fun reportAd() {
+ controller?.sendCustomCommand(NotificationControls.reportAdCommand, android.os.Bundle.EMPTY)
+ }
+
/** Called on a UI tick, since position changes without any callback firing. */
fun publish() {
val player = controller
diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeDetailActions.kt b/app/src/main/java/org/terst/listen/ui/EpisodeDetailActions.kt
index b4e9d8a..18385d1 100644
--- a/app/src/main/java/org/terst/listen/ui/EpisodeDetailActions.kt
+++ b/app/src/main/java/org/terst/listen/ui/EpisodeDetailActions.kt
@@ -22,6 +22,11 @@ data class EpisodeDetailActions(
/** Skips back or forward by a step, from wherever this episode currently is. */
val skip: (deltaMs: Long) -> Unit,
val setSpeed: (Float) -> Unit,
+ /**
+ * "This is an ad": skips past it and records the claim, the same gesture the notification
+ * button carries. See [org.terst.listen.playback.AdReportGesture].
+ */
+ val reportAd: () -> Unit = {},
/** Stops playback after this many minutes, fading over the last twenty seconds. */
val armSleepTimer: (minutes: Long) -> Unit = {},
val cancelSleepTimer: () -> Unit = {},
diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeFollowing.kt b/app/src/main/java/org/terst/listen/ui/EpisodeFollowing.kt
new file mode 100644
index 0000000..19844f4
--- /dev/null
+++ b/app/src/main/java/org/terst/listen/ui/EpisodeFollowing.kt
@@ -0,0 +1,23 @@
+package org.terst.listen.ui
+
+/**
+ * Whether the episode screen moves with the player.
+ *
+ * The screen and the player are one thing now, which is the whole point of the merge -- and it
+ * created this: when the queue advanced, the screen stayed on the episode that had finished. Its
+ * transport then belonged to an episode nobody was listening to, so pressing play there restarted
+ * it over the one that was.
+ *
+ * It follows only when it was already showing the playing episode. Opening an older episode while
+ * something else plays is a deliberate act, and a screen that snatched itself away mid-read would
+ * be a worse bug than the one this fixes.
+ */
+object EpisodeFollowing {
+
+ /** The episode the screen should show. */
+ fun shown(current: String, wasPlaying: String?, nowPlaying: String?): String {
+ if (nowPlaying == null || nowPlaying == current) return current
+ // Was this screen the player's own? Only then does the player get to move it.
+ return if (wasPlaying == current) nowPlaying else current
+ }
+}
diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt b/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt
index 7ad3740..cb36d64 100644
--- a/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt
+++ b/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt
@@ -64,8 +64,21 @@ fun EpisodeScreen(
var episode by remember { mutableStateOf<EpisodeEntity?>(null) }
var ranges by remember { mutableStateOf<List<EpisodeMidrollEntity>>(emptyList()) }
var speed by remember { mutableFloatStateOf(PlaybackSpeed.DEFAULT) }
- LaunchedEffect(episodeId) {
- EpisodeOpening.open(episodeId, ScreenSource(vm, player))?.let {
+ val nowPlaying by player.nowPlaying.collectAsState()
+ val sleepRemaining by player.sleepRemainingMs.collectAsState()
+
+ // What the screen shows, which is the route's episode until the player moves off it. The
+ // route argument cannot be the answer on its own: the queue advances without any navigation,
+ // and the screen is the player now. See EpisodeFollowing.
+ var shownId by remember(episodeId) { mutableStateOf(episodeId) }
+ var wasPlaying by remember(episodeId) { mutableStateOf(nowPlaying.episodeId) }
+ LaunchedEffect(nowPlaying.episodeId) {
+ shownId = EpisodeFollowing.shown(shownId, wasPlaying, nowPlaying.episodeId)
+ wasPlaying = nowPlaying.episodeId
+ }
+
+ LaunchedEffect(shownId) {
+ EpisodeOpening.open(shownId, ScreenSource(vm, player))?.let {
episode = it.episode
ranges = it.ranges
speed = it.speed
@@ -73,9 +86,7 @@ fun EpisodeScreen(
}
val scope = rememberCoroutineScope()
val rules by vm.rules.collectAsState()
- val judging = rememberBreakJudging(vm, episodeId, ranges)
- val nowPlaying by player.nowPlaying.collectAsState()
- val sleepRemaining by player.sleepRemainingMs.collectAsState()
+ val judging = rememberBreakJudging(vm, shownId, ranges)
val showTitle = vm.subscriptions.collectAsState().value
.firstOrNull { it.id == episode?.subscriptionId }?.title
@@ -83,7 +94,7 @@ fun EpisodeScreen(
val hooks = EpisodeHooks(
onSpeed = { speed = it },
onEpisode = { episode = it },
- reload = { scope.launch { episode = vm.episode(episodeId) } },
+ reload = { scope.launch { episode = vm.episode(shownId) } },
)
Scaffold(
topBar = {
@@ -95,7 +106,7 @@ fun EpisodeScreen(
},
// The playing episode's own screen already has the transport, so the bar steps aside
// there -- two sets of play controls on one screen would be the drift this merge removed.
- bottomBar = { PlayerBar(player, nav.openEpisode, nav.openQueue, hideBarFor = episodeId) },
+ bottomBar = { PlayerBar(player, nav.openEpisode, nav.openQueue, hideBarFor = shownId) },
) { padding ->
if (current == null) {
Text("Episode not found", Modifier.padding(padding).padding(24.dp))
@@ -177,6 +188,7 @@ private fun episodeActions(
else -> player.skipForward()
}
},
+ reportAd = player::reportAd,
armSleepTimer = player::armSleepTimer,
cancelSleepTimer = player::cancelSleepTimer,
setSpeed = { chosen ->
diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeTransport.kt b/app/src/main/java/org/terst/listen/ui/EpisodeTransport.kt
index 3a56c9d..3c4cd05 100644
--- a/app/src/main/java/org/terst/listen/ui/EpisodeTransport.kt
+++ b/app/src/main/java/org/terst/listen/ui/EpisodeTransport.kt
@@ -10,6 +10,7 @@ import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.width
import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.filled.Check
+import androidx.compose.material.icons.filled.Block
import androidx.compose.material.icons.filled.FastForward
import androidx.compose.material.icons.filled.FastRewind
import androidx.compose.material.icons.filled.Pause
@@ -32,11 +33,15 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.unit.dp
/**
- * The one transport: back, the single primary control, forward, and the speed.
+ * The one transport: back, the single primary control, forward, the ad button, and the speed.
*
* The primary button's wording says what tapping it will do (Play, Resume, Pause) and it is the
* only play control on the screen. Speed sits behind a tap showing the current value, so a row
* of four chips no longer takes a full line.
+ *
+ * The ad button is the same gesture the notification carries, and it is here for the same reason
+ * it is there: detection accounts for roughly 38% of the advertising the feeds imply, so most of
+ * what gets heard was never found, and this is the only moment anyone notices.
*/
@Composable
fun EpisodeTransport(playback: EpisodePlayback, speed: Float, actions: EpisodeDetailActions) {
@@ -59,6 +64,11 @@ fun EpisodeTransport(playback: EpisodePlayback, speed: Float, actions: EpisodeDe
IconButton(onClick = { actions.skip(Transport.FORWARD_MS) }) {
Icon(Icons.Default.FastForward, contentDescription = "Forward 25 seconds")
}
+ // A circle-and-slash, the same mark the notification uses, so the two read as one
+ // control in two places rather than two features.
+ IconButton(onClick = actions.reportAd, enabled = playback.isPlaying) {
+ Icon(Icons.Default.Block, contentDescription = "This is an ad")
+ }
SpeedChip(speed, actions.setSpeed)
}
}
diff --git a/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt b/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt
index a32351b..b785763 100644
--- a/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt
+++ b/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt
@@ -165,6 +165,9 @@ class PlayerViewModel(app: Application) : AndroidViewModel(app) {
/** The scrubber, committed once the drag ends rather than on every move. */
fun seekTo(positionMs: Long) = connection.seekTo(positionMs)
+ /** "This is an ad": skips past it and says so, the same as the notification's button. */
+ fun reportAd() = connection.reportAd()
+
/**
* Sets the speed for the show behind [episode], persisted so it survives to the next episode
* from that show, and applies it at once when that episode is the one playing. This is what
diff --git a/app/src/test/java/org/terst/listen/ui/EpisodeFollowingTest.kt b/app/src/test/java/org/terst/listen/ui/EpisodeFollowingTest.kt
new file mode 100644
index 0000000..5faa2ff
--- /dev/null
+++ b/app/src/test/java/org/terst/listen/ui/EpisodeFollowingTest.kt
@@ -0,0 +1,55 @@
+package org.terst.listen.ui
+
+import org.junit.Assert.assertEquals
+import org.junit.Test
+
+/**
+ * Which episode the merged screen shows when the player moves on.
+ *
+ * Reported live: "I end up with a different episode playing than shown because it doesn't
+ * follow what's playing." The screen is the player now, so when the queue advances off the
+ * episode you are watching, the screen has to go with it -- otherwise its transport belongs to
+ * an episode nobody is listening to, and pressing play there restarts it over the one that is.
+ *
+ * It only follows when it was already showing the playing episode. Opening an older episode
+ * while something else plays is a deliberate act, and hijacking that screen would be worse than
+ * the bug.
+ */
+class EpisodeFollowingTest {
+
+ @Test
+ fun `follows when the queue advances off the episode being watched`() {
+ assertEquals("b", EpisodeFollowing.shown(current = "a", wasPlaying = "a", nowPlaying = "b"))
+ }
+
+ @Test
+ fun `keeps following as the queue keeps moving`() {
+ val first = EpisodeFollowing.shown(current = "a", wasPlaying = "a", nowPlaying = "b")
+ assertEquals("c", EpisodeFollowing.shown(current = first, wasPlaying = "b", nowPlaying = "c"))
+ }
+
+ @Test
+ fun `stays put when you opened something other than what is playing`() {
+ assertEquals("old", EpisodeFollowing.shown(current = "old", wasPlaying = "a", nowPlaying = "b"))
+ }
+
+ @Test
+ fun `stays put when playback stops rather than following nothing`() {
+ assertEquals("a", EpisodeFollowing.shown(current = "a", wasPlaying = "a", nowPlaying = null))
+ }
+
+ @Test
+ fun `stays put when nothing was playing to follow`() {
+ assertEquals("a", EpisodeFollowing.shown(current = "a", wasPlaying = null, nowPlaying = "b"))
+ }
+
+ @Test
+ fun `no move while the same episode keeps playing`() {
+ assertEquals("a", EpisodeFollowing.shown(current = "a", wasPlaying = "a", nowPlaying = "a"))
+ }
+
+ @Test
+ fun `starting playback of the episode being watched is not a move`() {
+ assertEquals("a", EpisodeFollowing.shown(current = "a", wasPlaying = null, nowPlaying = "a"))
+ }
+}