From 2cc252a27a2b1ec801522e942d255ca9a9b95392 Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Mon, 2 Nov 2026 07:45:07 +0000 Subject: Move what the app says about time out of the views that said it first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit formatClock lived in NowPlayingBar.kt and had grown five callers across screens that share no other code -- the transport, the break timeline, the chapter list, the sleep timer and the transcript gutter. progressLabel, the one line under every episode title, lived in EpisodeRow.kt and had three. Both are pure, neither is a view, and each file had defined its own minutes-per-hour, so changing how the now-playing bar reads a position meant editing the transcript gutter's formatter. They are the last two holdouts from a rule this app already follows everywhere else: anything it says out loud lives in a named file of its own and is tested, not inside the composable that renders it. SkipSummary, RuleSummary, FoundBySummary, WaitingForServerText, DescriptionPreview and SpeedChoices are all already arranged that way -- WaitingForServerText's own comment says so. Compose screens are excluded from coverage, so wording left inside one is wording nothing checks. TimeText holds both formatters and one set of constants: formatClock for a position on the transport, formatDuration for a length. They look similar and are not interchangeable -- 1:02:03 answers "where am I", 1h 2m answers "have I got time for this". EpisodeLabels holds progressLabel and the release-date wording it uses. Both had no test at all. The sixteen here were written against the old locations and confirmed green *before* the move, so re-running them after it is what proves the move changed no behaviour; written afterwards they would only prove the new code agrees with itself. They pin the hour boundary from both sides, because everything in this library is under an hour except the thing the app exists for, and the future-dated episode, which floors to zero days and would otherwise read "today" for as long as it existed. 951 unit tests pass, detekt clean, coverage unchanged. The one line under an episode title was read back off a device after the move: "20 Oct 2026 · 1h 20m". Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B --- .../main/java/org/terst/listen/ui/EpisodeLabels.kt | 53 ++++++++++ .../main/java/org/terst/listen/ui/EpisodeRow.kt | 46 -------- .../main/java/org/terst/listen/ui/NowPlayingBar.kt | 16 --- app/src/main/java/org/terst/listen/ui/TimeText.kt | 41 ++++++++ .../test/java/org/terst/listen/ui/ClockTextTest.kt | 47 +++++++++ .../terst/listen/ui/EpisodeProgressLabelTest.kt | 116 +++++++++++++++++++++ 6 files changed, 257 insertions(+), 62 deletions(-) create mode 100644 app/src/main/java/org/terst/listen/ui/EpisodeLabels.kt create mode 100644 app/src/main/java/org/terst/listen/ui/TimeText.kt create mode 100644 app/src/test/java/org/terst/listen/ui/ClockTextTest.kt create mode 100644 app/src/test/java/org/terst/listen/ui/EpisodeProgressLabelTest.kt (limited to 'app') diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeLabels.kt b/app/src/main/java/org/terst/listen/ui/EpisodeLabels.kt new file mode 100644 index 0000000..921fec2 --- /dev/null +++ b/app/src/main/java/org/terst/listen/ui/EpisodeLabels.kt @@ -0,0 +1,53 @@ +package org.terst.listen.ui + +import org.terst.listen.data.DownloadState +import org.terst.listen.data.EpisodeEntity +import org.terst.listen.data.playbackDurationMs +import org.terst.listen.download.WaitingForServerText +import java.util.concurrent.TimeUnit + +/** + * The one line under an episode's title, wherever an episode is listed. + * + * Read in three places that look nothing alike -- a show's list, the episode screen, and the + * queue -- which is why it is here rather than in `EpisodeRow.kt`, where it was. It is the + * app's most-read sentence, so its order is fixed: when it is from, how long it is, how far in, + * then anything stopping it playing. + */ + +/** Duration, how far in, and download state in one line. */ +internal fun EpisodeEntity.progressLabel(): String { + val parts = mutableListOf() + publishedAtMs?.let { parts += formatReleaseDate(it) } + // The measured length where it is known: the feed's is minutes short on any show using + // dynamic ad insertion, which is how Peter spotted the list and the player disagreeing. + playbackDurationMs?.let { parts += formatDuration(it) } + if (positionMs > 0) parts += "${formatDuration(positionMs)} in" + if (playedAtMs != null) parts += "played" + if (downloadState == DownloadState.FAILED) parts += "download failed" + if (downloadState == DownloadState.WAITING_FOR_SERVER) parts += WaitingForServerText.ROW_LABEL + return parts.joinToString(" · ") +} + +/** + * Relative for the last week, then absolute. + * + * "3d ago" answers "is this the new one?" at a glance, which is the question in an episode list; + * past a week the actual date is more use than a growing day count. A feed dating an episode in + * the future gets the date too -- a negative age floors to zero days, which would otherwise read + * "today" for as long as the episode existed. + */ +private fun formatReleaseDate(publishedAtMs: Long): String { + val ageMs = System.currentTimeMillis() - publishedAtMs + val days = TimeUnit.MILLISECONDS.toDays(ageMs) + return when { + ageMs < 0 -> dateFormat.format(java.util.Date(publishedAtMs)) + days == 0L -> "today" + days == 1L -> "yesterday" + days < DAYS_BEFORE_ABSOLUTE -> "${days}d ago" + else -> dateFormat.format(java.util.Date(publishedAtMs)) + } +} + +private const val DAYS_BEFORE_ABSOLUTE = 7L +private val dateFormat = java.text.SimpleDateFormat("d MMM yyyy", java.util.Locale.getDefault()) 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 dac7902..45ed36b 100644 --- a/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt +++ b/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt @@ -32,9 +32,6 @@ import androidx.compose.ui.unit.dp import coil3.compose.AsyncImage import org.terst.listen.data.DownloadState import org.terst.listen.data.EpisodeEntity -import org.terst.listen.download.WaitingForServerText -import org.terst.listen.data.playbackDurationMs -import java.util.concurrent.TimeUnit /** * One episode and its actions. @@ -132,46 +129,3 @@ internal fun DownloadIcon(state: DownloadState, onDownload: () -> Unit) { else -> ActionIcon(Icons.Default.Download, "Download", onDownload) } } - -/** Duration, how far in, and download state in one line. */ -internal fun EpisodeEntity.progressLabel(): String { - val parts = mutableListOf() - publishedAtMs?.let { parts += formatReleaseDate(it) } - // The measured length where it is known: the feed's is minutes short on any show using - // dynamic ad insertion, which is how Peter spotted the list and the player disagreeing. - playbackDurationMs?.let { parts += formatDuration(it) } - if (positionMs > 0) parts += "${formatDuration(positionMs)} in" - if (playedAtMs != null) parts += "played" - if (downloadState == DownloadState.FAILED) parts += "download failed" - if (downloadState == DownloadState.WAITING_FOR_SERVER) parts += WaitingForServerText.ROW_LABEL - return parts.joinToString(" · ") -} - -/** - * Relative for the last week, then absolute. - * - * "3d ago" answers "is this the new one?" at a glance, which is the question in an episode list; - * past a week the actual date is more use than a growing day count. - */ -private fun formatReleaseDate(publishedAtMs: Long): String { - val ageMs = System.currentTimeMillis() - publishedAtMs - val days = TimeUnit.MILLISECONDS.toDays(ageMs) - return when { - ageMs < 0 -> dateFormat.format(java.util.Date(publishedAtMs)) - days == 0L -> "today" - days == 1L -> "yesterday" - days < DAYS_BEFORE_ABSOLUTE -> "${days}d ago" - else -> dateFormat.format(java.util.Date(publishedAtMs)) - } -} - -private const val DAYS_BEFORE_ABSOLUTE = 7L -private val dateFormat = java.text.SimpleDateFormat("d MMM yyyy", java.util.Locale.getDefault()) - -private const val MINUTES_PER_HOUR = 60 - -private fun formatDuration(ms: Long): String { - val hours = TimeUnit.MILLISECONDS.toHours(ms) - val minutes = TimeUnit.MILLISECONDS.toMinutes(ms) % MINUTES_PER_HOUR - return if (hours > 0) "${hours}h ${minutes}m" else "${minutes}m" -} diff --git a/app/src/main/java/org/terst/listen/ui/NowPlayingBar.kt b/app/src/main/java/org/terst/listen/ui/NowPlayingBar.kt index 482fa86..c9960ef 100644 --- a/app/src/main/java/org/terst/listen/ui/NowPlayingBar.kt +++ b/app/src/main/java/org/terst/listen/ui/NowPlayingBar.kt @@ -88,22 +88,6 @@ fun NowPlayingBar(state: NowPlaying, actions: NowPlayingActions) { } } -private const val SECONDS_PER_MINUTE = 60 -private const val MINUTES_PER_HOUR = 60 -private const val MS_PER_SECOND = 1000 - -internal fun formatClock(ms: Long): String { - val totalSeconds = ms / MS_PER_SECOND - val seconds = totalSeconds % SECONDS_PER_MINUTE - val minutes = (totalSeconds / SECONDS_PER_MINUTE) % MINUTES_PER_HOUR - val hours = totalSeconds / (SECONDS_PER_MINUTE * MINUTES_PER_HOUR) - return if (hours > 0) { - "%d:%02d:%02d".format(hours, minutes, seconds) - } else { - "%d:%02d".format(minutes, seconds) - } -} - /** Title and show, tappable to open the player. */ @Composable private fun NowPlayingText(state: NowPlaying, modifier: Modifier, onOpen: () -> Unit) { diff --git a/app/src/main/java/org/terst/listen/ui/TimeText.kt b/app/src/main/java/org/terst/listen/ui/TimeText.kt new file mode 100644 index 0000000..f70f80f --- /dev/null +++ b/app/src/main/java/org/terst/listen/ui/TimeText.kt @@ -0,0 +1,41 @@ +package org.terst.listen.ui + +import java.util.concurrent.TimeUnit + +/** + * Milliseconds as the two kinds of time a listener reads. + * + * [formatClock] is a position -- where you are, or where a break edge is -- so it keeps seconds + * and reads like a transport: `1:02:03`. [formatDuration] is a length -- how long a thing is -- + * where seconds are noise and `1h 2m` is what answers "have I got time for this?". + * + * They lived in `NowPlayingBar.kt` and `EpisodeRow.kt`, the views that happened to want them + * first, and had grown five and three callers respectively. Both are pure, both are read on + * screens that share no code, and each file had defined its own minutes-per-hour -- so editing + * the now-playing bar meant editing the transcript gutter's formatter. One home, one set of + * constants, tested directly: the same arrangement as [SpeedChoices], [DescriptionPreview] and + * `WaitingForServerText`, which is already this app's rule for anything it says out loud. + */ +private const val MS_PER_SECOND = 1000 +private const val SECONDS_PER_MINUTE = 60 +private const val MINUTES_PER_HOUR = 60 + +/** A position on the transport: `0:06`, `10:12`, `1:02:03`. Hours appear only once there are any. */ +internal fun formatClock(ms: Long): String { + val totalSeconds = ms / MS_PER_SECOND + val seconds = totalSeconds % SECONDS_PER_MINUTE + val minutes = (totalSeconds / SECONDS_PER_MINUTE) % MINUTES_PER_HOUR + val hours = totalSeconds / (SECONDS_PER_MINUTE * MINUTES_PER_HOUR) + return if (hours > 0) { + "%d:%02d:%02d".format(hours, minutes, seconds) + } else { + "%d:%02d".format(minutes, seconds) + } +} + +/** A length, rounded to the minute: `45m`, `1h 2m`. */ +internal fun formatDuration(ms: Long): String { + val hours = TimeUnit.MILLISECONDS.toHours(ms) + val minutes = TimeUnit.MILLISECONDS.toMinutes(ms) % MINUTES_PER_HOUR + return if (hours > 0) "${hours}h ${minutes}m" else "${minutes}m" +} diff --git a/app/src/test/java/org/terst/listen/ui/ClockTextTest.kt b/app/src/test/java/org/terst/listen/ui/ClockTextTest.kt new file mode 100644 index 0000000..7b589d2 --- /dev/null +++ b/app/src/test/java/org/terst/listen/ui/ClockTextTest.kt @@ -0,0 +1,47 @@ +package org.terst.listen.ui + +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * The timestamp five screens read: the transport's elapsed and remaining, the timeline's break + * edges, a chapter's start in the overflow menu, the sleep timer's countdown, and the transcript + * gutter's time per line. + * + * It had no test until now, which is why the hour boundary is pinned from both sides. Everything + * in this library is under an hour except the thing this app exists for -- Hardcore History runs + * past four -- so the case most likely to be wrong is the one least likely to be noticed. + */ +class ClockTextTest { + + @Test + fun `under a minute still shows the minute`() { + assertEquals("0:00", formatClock(0)) + assertEquals("0:06", formatClock(6_000)) + } + + @Test + fun `seconds are two digits so the column does not jump`() { + assertEquals("0:09", formatClock(9_000)) + assertEquals("0:10", formatClock(10_000)) + assertEquals("10:12", formatClock(612_000)) + } + + /** A partial second belongs to the second it is in, not the next one. */ + @Test + fun `part of a second is not rounded up`() { + assertEquals("0:01", formatClock(1_999)) + } + + @Test + fun `an hour appears only once there is one`() { + assertEquals("59:59", formatClock(3_599_000)) + assertEquals("1:00:00", formatClock(3_600_000)) + } + + @Test + fun `past the hour the minutes restart rather than accumulating`() { + assertEquals("1:02:03", formatClock(3_723_000)) + assertEquals("4:05:06", formatClock(14_706_000)) + } +} diff --git a/app/src/test/java/org/terst/listen/ui/EpisodeProgressLabelTest.kt b/app/src/test/java/org/terst/listen/ui/EpisodeProgressLabelTest.kt new file mode 100644 index 0000000..8ad5d8f --- /dev/null +++ b/app/src/test/java/org/terst/listen/ui/EpisodeProgressLabelTest.kt @@ -0,0 +1,116 @@ +package org.terst.listen.ui + +import org.junit.Assert.assertEquals +import org.junit.Test +import org.terst.listen.data.DownloadState +import org.terst.listen.data.EpisodeEntity +import org.terst.listen.download.WaitingForServerText +import java.util.concurrent.TimeUnit + +/** + * The one line under every episode title -- in a show's list, on the episode screen, and in the + * queue. + * + * Untested until now, though it is the most-read sentence in the app. The cases pinned here are + * the ones a reader has to be able to tell apart at a glance: how long it is, how far in, and + * whether it is going to play at all. + */ +class EpisodeProgressLabelTest { + + private fun episode( + durationMs: Long? = null, + publishedAtMs: Long? = null, + positionMs: Long = 0, + playedAtMs: Long? = null, + state: DownloadState = DownloadState.NONE, + ) = EpisodeEntity( + id = "e1", subscriptionId = "s1", guid = "g", title = "One", mediaUrl = "https://cdn/1.mp3", + mediaType = "audio/mpeg", discoveredAtMs = 0, durationMs = durationMs, + publishedAtMs = publishedAtMs, positionMs = positionMs, playedAtMs = playedAtMs, + downloadState = state, + ) + + private fun daysAgo(days: Long) = System.currentTimeMillis() - TimeUnit.DAYS.toMillis(days) + + @Test + fun `an episode with nothing known of it says nothing`() { + assertEquals("", episode().progressLabel()) + } + + @Test + fun `an hour or more reads in hours and minutes`() { + assertEquals("45m", episode(durationMs = 2_700_000).progressLabel()) + assertEquals("1h 2m", episode(durationMs = 3_720_000).progressLabel()) + } + + /** + * The measured length wins over the feed's, which is the whole reason the two are kept apart: + * on a show using dynamic ad insertion the feed's figure is minutes short. + */ + @Test + fun `the measured length is preferred to the declared one`() { + val measured = episode(durationMs = 1_325_000).copy(actualDurationMs = 1_698_000) + assertEquals("28m", measured.progressLabel()) + } + + @Test + fun `how far in is shown only once there is somewhere to resume from`() { + assertEquals("45m", episode(durationMs = 2_700_000).progressLabel()) + assertEquals("45m · 12m in", episode(durationMs = 2_700_000, positionMs = 720_000).progressLabel()) + } + + @Test + fun `a finished episode says so`() { + assertEquals("45m · played", episode(durationMs = 2_700_000, playedAtMs = 1).progressLabel()) + } + + @Test + fun `a failed download says so, so the row explains why nothing plays`() { + assertEquals("download failed", episode(state = DownloadState.FAILED).progressLabel()) + } + + @Test + fun `an episode waiting on the server uses the same words the episode screen does`() { + assertEquals( + WaitingForServerText.ROW_LABEL, + episode(state = DownloadState.WAITING_FOR_SERVER).progressLabel(), + ) + } + + /** "is this the new one?" is the question an episode list is answering. */ + @Test + fun `the first week reads as an age rather than a date`() { + assertEquals("today", episode(publishedAtMs = daysAgo(0)).progressLabel()) + assertEquals("yesterday", episode(publishedAtMs = daysAgo(1)).progressLabel()) + assertEquals("3d ago", episode(publishedAtMs = daysAgo(3)).progressLabel()) + assertEquals("6d ago", episode(publishedAtMs = daysAgo(6)).progressLabel()) + } + + /** Past a week a growing day count is less use than the date itself. */ + @Test + fun `past a week it reads as a date`() { + assertEquals("1 Jan 2020", episode(publishedAtMs = 1_577_880_000_000).progressLabel()) + } + + /** + * A feed dating an episode in the future would otherwise read "today" forever, because a + * negative age floors to zero days. + */ + @Test + fun `a future date reads as a date rather than as today`() { + val label = episode(publishedAtMs = daysAgo(-30)).progressLabel() + assertEquals(false, label == "today") + assertEquals(true, label.contains(" ")) + } + + @Test + fun `the parts appear in the order a reader scans them`() { + val label = episode( + durationMs = 2_700_000, + publishedAtMs = daysAgo(2), + positionMs = 720_000, + playedAtMs = 1, + ).progressLabel() + assertEquals("2d ago · 45m · 12m in · played", label) + } +} -- cgit v1.2.3