fix(worldclock): keep the seconds option from costing reorder and catch-up

Rows that tick every second re-seeded the reorderable column mid-drag, so it is handed ids now. A zone or clock change no longer waits for the next minute: TIME_SET, TIMEZONE_CHANGED and TIME_TICK are merged into the tick through a new ClockChanges seam. With seconds on, the row's time drops a size so the city name keeps its room. The truncation unit and the tick come from one place, and stale 200 ms comments are gone.
2026-10-21 18:57:10 +02:00
1 parent 785adf715b
commit a4cec23e5c
@@ -5,11 +5,13 @@ import dagger.Module
 import dagger.hilt.InstallIn
 import dagger.hilt.components.SingletonComponent
 import de.jeanlucmakiola.clockula.data.time.AndroidBootIdProvider
+import de.jeanlucmakiola.clockula.data.time.AndroidClockChanges
 import de.jeanlucmakiola.clockula.data.time.RealTicker
 import de.jeanlucmakiola.clockula.data.time.SystemElapsedRealtimeClock
 import de.jeanlucmakiola.clockula.data.time.SystemWallClock
 import de.jeanlucmakiola.clockula.data.time.SystemZoneProvider
 import de.jeanlucmakiola.clockula.domain.time.BootIdProvider
+import de.jeanlucmakiola.clockula.domain.time.ClockChanges
 import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock
 import de.jeanlucmakiola.clockula.domain.time.Ticker
 import de.jeanlucmakiola.clockula.domain.time.WallClock
@@ -39,4 +41,7 @@ abstract class TimeModule {

    @Binds
    abstract fun bindTicker(impl: RealTicker): Ticker
+
+    @Binds
+    abstract fun bindClockChanges(impl: AndroidClockChanges): ClockChanges
 }
@@ -0,0 +1,43 @@
+package de.jeanlucmakiola.clockula.data.time
+
+import android.content.BroadcastReceiver
+import android.content.Context
+import android.content.Intent
+import android.content.IntentFilter
+import androidx.core.content.ContextCompat
+import dagger.hilt.android.qualifiers.ApplicationContext
+import de.jeanlucmakiola.clockula.domain.time.ClockChanges
+import kotlinx.coroutines.channels.awaitClose
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.callbackFlow
+import javax.inject.Inject
+import javax.inject.Singleton
+
+/**
+ * `TIME_SET`, `TIMEZONE_CHANGED` and `TIME_TICK`, through a receiver that is
+ * registered only while somebody collects. `TIME_TICK` cannot be declared in
+ * the manifest at all, and the other two already have their manifest receiver
+ * (`SystemEventReceiver`) for the alarm engine; this one is for a readout.
+ */
+@Singleton
+class AndroidClockChanges @Inject constructor(
+    @param:ApplicationContext private val context: Context,
+) : ClockChanges {
+
+    override fun changes(): Flow<Unit> = callbackFlow {
+        val receiver = object : BroadcastReceiver() {
+            override fun onReceive(context: Context, intent: Intent) {
+                trySend(Unit)
+            }
+        }
+        val filter = IntentFilter().apply {
+            addAction(Intent.ACTION_TIME_CHANGED)
+            addAction(Intent.ACTION_TIMEZONE_CHANGED)
+            addAction(Intent.ACTION_TIME_TICK)
+        }
+        // Protected system broadcasts still arrive at a non-exported receiver,
+        // and nothing else on the device gets to poke the tab.
+        ContextCompat.registerReceiver(context, receiver, filter, ContextCompat.RECEIVER_NOT_EXPORTED)
+        awaitClose { context.unregisterReceiver(receiver) }
+    }
+}
@@ -98,8 +98,8 @@ class ZoneDirectory @Inject constructor(

    /**
     * One display name per id in [zoneIds], on the same memo. Batched for the
-     * same reason [entriesFor] is: the tab asks five times a second, and a
-     * dispatch and a lock per row would be a hundred and twenty-five of each.
+     * same reason [entriesFor] is: the tab asks on every tick (once a second with
+     * seconds on), and a dispatch and a lock per row would be twenty-five of each.
     */
    suspend fun displayNamesOf(zoneIds: List<String>, at: Instant): Map<String, String?> =
        withContext(io) {
@@ -17,8 +17,8 @@ object Zones {

    /**
     * Snapshotted once. `ZoneId.getAvailableZoneIds()` builds a fresh ~600-entry
-     * `HashSet` on every call, and the world clock asks per row per 200 ms tick
-     * — twenty-four cities would be a hundred and twenty-five throwaway sets a
+     * `HashSet` on every call, and the world clock asks per row per tick —
+     * with seconds on, twenty-four cities would be twenty-five throwaway sets a
     * second. The device's tzdata arrives as an APEX module that takes effect
     * at reboot, so the answer cannot change under a running process.
     */
@@ -49,3 +49,13 @@ interface Ticker {
     */
    fun alignedTicks(unit: Duration): Flow<Unit>
 }
+
+/**
+ * The system saying the wall clock may not read what a readout last drew: the
+ * time was set, the zone changed, or a minute passed while the device was
+ * awake. A readout on a long cadence merges this in so it does not wait out
+ * its own period to catch up. Emits only while collected.
+ */
+interface ClockChanges {
+    fun changes(): Flow<Unit>
+}
@@ -2,9 +2,9 @@ package de.jeanlucmakiola.clockula.ui.worldclock

 import androidx.compose.ui.unit.Dp
 import androidx.compose.ui.unit.dp
+import java.time.temporal.ChronoUnit
 import kotlin.time.Duration
-import kotlin.time.Duration.Companion.minutes
-import kotlin.time.Duration.Companion.seconds
+import kotlin.time.toKotlinDuration

 internal object WorldClockDefaults {
    /**
@@ -12,11 +12,17 @@ internal object WorldClockDefaults {
     * rather than up to a cadence late: the minute when seconds are off, the
     * second only when the user asked for them (D18, #23).
     */
-    val MinuteTick: Duration = 1.minutes
+    val MinuteTick: Duration = ChronoUnit.MINUTES.duration.toKotlinDuration()

-    val SecondTick: Duration = 1.seconds
+    val SecondTick: Duration = ChronoUnit.SECONDS.duration.toKotlinDuration()

-    fun tickFor(showSeconds: Boolean): Duration = if (showSeconds) SecondTick else MinuteTick
+    /**
+     * The unit every time on the tab is truncated to. The tick is derived from
+     * it, so the readouts and the cadence cannot drift apart.
+     */
+    fun unitFor(showSeconds: Boolean): ChronoUnit = if (showSeconds) ChronoUnit.SECONDS else ChronoUnit.MINUTES
+
+    fun tickFor(showSeconds: Boolean): Duration = unitFor(showSeconds).duration.toKotlinDuration()

    val HeroFaceSize: Dp = 200.dp

@@ -52,6 +52,7 @@ internal fun WorldClockRow(
    row: WorldClockRowState,
    position: Position,
    formatTime: (LocalTime) -> String,
+    showSeconds: Boolean,
    dragHandle: Modifier,
    isDragging: Boolean,
    onMoveUp: () -> Unit,
@@ -134,7 +135,15 @@ internal fun WorldClockRow(
                    text = time,
                    // Tabular, because M7 made the headline role tabular
                    // app-wide: a column of times must not jitter as it ticks.
-                    style = MaterialTheme.typography.headlineMedium,
+                    // A size down with seconds, which add three or more glyphs
+                    // the city name would otherwise pay for.
+                    style = if (showSeconds) {
+                        MaterialTheme.typography.headlineSmall
+                    } else {
+                        MaterialTheme.typography.headlineMedium
+                    },
+                    maxLines = 1,
+                    softWrap = false,
                    fontWeight = FontWeight.Bold,
                    color = if (isDragging) {
                        MaterialTheme.colorScheme.primary
@@ -11,7 +11,6 @@ import de.jeanlucmakiola.clockula.domain.worldclock.ZoneEntry
 import de.jeanlucmakiola.clockula.domain.worldclock.ZoneOffsetFormat
 import java.time.LocalTime
 import java.time.ZoneId
-import java.time.temporal.ChronoUnit
 import kotlin.time.Instant
 import kotlin.time.toJavaInstant

@@ -74,7 +73,7 @@ internal object WorldClockRows {
        now: Instant,
        entries: Map<String, ZoneEntry>,
        zoneNames: Map<String, String?>,
-        showSeconds: Boolean = false,
+        showSeconds: Boolean,
    ): List<WorldClockRowState> = clocks.map { clock ->
        val known = clock.isKnownZone
        val comparison = if (known) {
@@ -86,7 +85,7 @@ internal object WorldClockRows {
            id = clock.id,
            zoneId = clock.zoneId,
            name = clock.label ?: cityOf(clock.zoneId, entries),
-            time = comparison?.localDateTime?.toLocalTime()?.truncatedTo(unitFor(showSeconds)),
+            time = comparison?.localDateTime?.toLocalTime()?.truncatedTo(WorldClockDefaults.unitFor(showSeconds)),
            offset = comparison?.let { ZoneOffsetFormat.labelFor(it.offsetMinutes) },
            day = comparison?.let { ZoneOffsetFormat.dayLabelFor(it.dayDifference) },
            zoneName = zoneNames[clock.zoneId],
@@ -100,11 +99,11 @@ internal object WorldClockRows {
        now: Instant,
        entries: Map<String, ZoneEntry>,
        zoneNames: Map<String, String?>,
-        showSeconds: Boolean = false,
+        showSeconds: Boolean,
    ): HomeFaceState {
        // To the unit shown, as the rows are. Without seconds the face has no
        // second hand, so its hands and its readout move once a minute.
-        val local = now.toJavaInstant().atZone(homeZone).toLocalTime().truncatedTo(unitFor(showSeconds))
+        val local = now.toJavaInstant().atZone(homeZone).toLocalTime().truncatedTo(WorldClockDefaults.unitFor(showSeconds))
        return HomeFaceState(
            zoneId = homeZone.id,
            city = cityOf(homeZone.id, entries),
@@ -114,9 +113,6 @@ internal object WorldClockRows {
        )
    }

-    private fun unitFor(showSeconds: Boolean): ChronoUnit =
-        if (showSeconds) ChronoUnit.SECONDS else ChronoUnit.MINUTES
-
    private fun cityOf(zoneId: String, entries: Map<String, ZoneEntry>): String =
        entries[zoneId]?.city ?: ZoneCatalog.cityFallbackFor(zoneId)
 }
@@ -117,17 +117,23 @@ fun WorldClockScreen(
            if (loaded.rows.isEmpty()) {
                EmptyWorldClocks()
            } else {
+                // The column is handed ids, not rows: it re-seeds its working
+                // order whenever its items change, and rows that tick (every
+                // second with seconds on) would snap a drag back mid-gesture.
+                val rowsById = loaded.rows.associateBy { it.id }
                ReorderableColumn(
-                    items = loaded.rows,
-                    keyOf = { it.id },
-                    onReorder = { rows -> viewModel.onReorder(rows.map { it.id }) },
+                    items = loaded.rows.map { it.id },
+                    keyOf = { it },
+                    onReorder = viewModel::onReorder,
                    modifier = Modifier.fillMaxWidth(),
                    rowHeight = 80.dp,
-                ) { row, position, dragHandle, isDragging ->
+                ) { id, position, dragHandle, isDragging ->
+                    val row = rowsById[id] ?: return@ReorderableColumn
                    WorldClockRow(
                        row = row,
                        position = position,
                        formatTime = formatTime,
+                        showSeconds = loaded.showSeconds,
                        dragHandle = dragHandle,
                        isDragging = isDragging,
                        onMoveUp = { viewModel.onMoveUp(row.id) },
@@ -4,6 +4,7 @@ import de.jeanlucmakiola.clockula.data.prefs.SettingsPrefs
 import de.jeanlucmakiola.clockula.data.worldclocks.WorldClockRepository
 import de.jeanlucmakiola.clockula.data.zones.ZoneDirectory
 import de.jeanlucmakiola.clockula.domain.WorldClock
+import de.jeanlucmakiola.clockula.domain.time.ClockChanges
 import de.jeanlucmakiola.clockula.domain.time.Ticker
 import de.jeanlucmakiola.clockula.domain.time.WallClock
 import de.jeanlucmakiola.clockula.domain.time.ZoneProvider
@@ -17,6 +18,7 @@ import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.flatMapLatest
 import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.flow.merge
 import java.time.ZoneId
 import javax.inject.Inject
 import javax.inject.Singleton
@@ -30,6 +32,7 @@ internal class WorldClockSource @Inject constructor(
    private val zones: ZoneProvider,
    private val wallClock: WallClock,
    private val ticker: Ticker,
+    private val clockChanges: ClockChanges,
 ) {
    /** What the picker needs before it can draw a row; null while it is still being built. */
    private data class Catalog(val entries: List<ZoneEntry>, val deviceCity: String)
@@ -42,9 +45,10 @@ internal class WorldClockSource @Inject constructor(

    /**
     * The tab's state. Both the instant and the device zone are re-read on
-     * **every tick**, which is what makes `TIME_SET` and `TIMEZONE_CHANGED`
-     * free: the tab self-heals within one tick and `SystemEventReceiver` needs
-     * no new branch (D2).
+     * **every tick**, and [ClockChanges] is a tick of its own, so `TIME_SET`
+     * and `TIMEZONE_CHANGED` show at once rather than at the next minute, and
+     * `TIME_TICK` catches the minute up after a deep sleep stretched a `delay`.
+     * `SystemEventReceiver` still needs no new branch (D2).
     *
     * The tick is aligned to the wall clock and only as fine as the readouts:
     * once a minute, or once a second when the user turned seconds on (#23).
@@ -55,7 +59,7 @@ internal class WorldClockSource @Inject constructor(
    fun state(): Flow<WorldClockUiState.Loaded> =
        combine(worldClocks.worldClocks(), storedHomeZoneId, showSeconds, ::Triple)
            .flatMapLatest { (clocks, storedHome, seconds) ->
-                ticker.alignedTicks(WorldClockDefaults.tickFor(seconds))
+                merge(ticker.alignedTicks(WorldClockDefaults.tickFor(seconds)), clockChanges.changes())
                    .map { build(clocks, storedHome, seconds) }
            }
            .distinctUntilChanged()
@@ -38,7 +38,7 @@ class WorldClockViewModel @Inject internal constructor(

    /** `WhileSubscribed(5_000)`, seeded [WorldClockUiState.Loading]. */
    internal val state: StateFlow<WorldClockUiState> = source.state()
-        // `WhileSubscribed`, not `Eagerly`: the upstream is a 200 ms ticker and
+        // `WhileSubscribed`, not `Eagerly`: the upstream is a ticker and
        // eager sharing would keep it running with nobody looking. The timeout
        // is what gives rotation its resilience, as on every other tab.
        .stateIn(
@@ -1,5 +1,6 @@
 package de.jeanlucmakiola.clockula.testing

+import de.jeanlucmakiola.clockula.domain.time.ClockChanges
 import de.jeanlucmakiola.clockula.domain.time.Ticker
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableSharedFlow
@@ -44,3 +45,16 @@ class FakeTicker : Ticker {
        streams.toList().forEach { it.emit(Unit) }
    }
 }
+
+/** [ClockChanges] a test fires by hand, standing in for the system's broadcasts. */
+class FakeClockChanges : ClockChanges {
+
+    private val stream = MutableSharedFlow<Unit>(extraBufferCapacity = 64)
+
+    override fun changes(): Flow<Unit> = stream
+
+    /** One `TIME_SET`/`TIMEZONE_CHANGED`/`TIME_TICK` to every collector. */
+    suspend fun fire() {
+        stream.emit(Unit)
+    }
+}
@@ -36,6 +36,7 @@ class WorldClockHarness(
    val wallClock: FakeWallClock = FakeWallClock(now)
    val zones: FakeZoneProvider = FakeZoneProvider()
    val ticker: FakeTicker = FakeTicker()
+    val clockChanges: FakeClockChanges = FakeClockChanges()
    val names: FakeZoneNames = FakeZoneNames()

    val store: PrefStore = PrefStore(
@@ -58,6 +59,7 @@ class WorldClockHarness(
        zones = zones,
        wallClock = wallClock,
        ticker = ticker,
+        clockChanges = clockChanges,
    )

    val viewModel: WorldClockViewModel = WorldClockViewModel(
@@ -243,7 +243,7 @@ class WorldClockRowsTest {
    /** §5.7 #16 */
    @Test
    fun `the hero face is home, to the minute`() {
-        val home = WorldClockRows.home(BERLIN, T0, entries, emptyMap())
+        val home = WorldClockRows.home(BERLIN, T0, entries, emptyMap(), showSeconds = false)

        assertThat(home.zoneId).isEqualTo(berlin)
        assertThat(home.city).isEqualTo("Berlin")
@@ -261,7 +261,7 @@ class WorldClockRowsTest {
    /** §5.7 #17 */
    @Test
    fun `a home zone with no entry falls back to its last segment`() {
-        val home = WorldClockRows.home(ZoneId.of("Asia/Ho_Chi_Minh"), T0, entries, emptyMap())
+        val home = WorldClockRows.home(ZoneId.of("Asia/Ho_Chi_Minh"), T0, entries, emptyMap(), showSeconds = false)

        assertThat(home.city).isEqualTo("Ho Chi Minh")
        assertThat(home.zoneName).isNull()
@@ -270,8 +270,8 @@ class WorldClockRowsTest {
    /** §5.7 #18 */
    @Test
    fun `the dial's morph carries whether it is the middle of the night there`() {
-        val night = WorldClockRows.home(BERLIN, T0, entries, emptyMap())
-        val day = WorldClockRows.home(UTC_ZONE, midday, entries, emptyMap())
+        val night = WorldClockRows.home(BERLIN, T0, entries, emptyMap(), showSeconds = false)
+        val day = WorldClockRows.home(UTC_ZONE, midday, entries, emptyMap(), showSeconds = false)

        assertThat(night.dayFraction).isWithin(0.001f).of(0f)
        assertThat(day.dayFraction).isWithin(0.001f).of(1f)
@@ -399,6 +399,21 @@ class WorldClockViewModelTest {
            assertThat(harness.loaded()).isEqualTo(before)
        }

+    /** #23: a minute-long cadence must not make a zone change wait a minute. */
+    @Test
+    fun `a zone change shows at once even between minute ticks`(@TempDir tempDir: Path) =
+        runTest(main.dispatcher) {
+            val harness = collecting(worldClockHarness(tempDir))
+            advanceUntilIdle()
+            assertThat(harness.loaded().home.zoneId).isEqualTo("Europe/Berlin")
+
+            harness.zones.zone = ZoneId.of("Asia/Tokyo")
+            harness.clockChanges.fire()
+            advanceUntilIdle()
+
+            assertThat(harness.loaded().home.zoneId).isEqualTo("Asia/Tokyo")
+        }
+
    /** §5.9 #20, with #23's seconds on. */
    @Test
    fun `a second of wall clock moves the face and the rows when seconds are on`(@TempDir tempDir: Path) =
@@ -1427,7 +1427,9 @@ nothing but `delay`, which makes it testable under `runTest`'s virtual clock.
 after the immediate emission it waits `WallBoundaries.untilNext(now, unit)`,
 re-measured against `WallClock` after every wake, so it fires on the second or
 minute boundary itself and a long `delay` or a set clock does not accumulate.
-The world clock is its one caller (§16, "The cadence").
+The world clock is its one caller (§16, "The cadence"), and it merges in
+`ClockChanges`, the system's own word that the clock or zone moved, so a long
+cadence never means a long wait for those.

 ### The dismiss challenge, and why it cannot strand anyone

@@ -2453,11 +2455,15 @@ the flow stays `distinctUntilChanged`, which now only absorbs the odd early
 wake. The setting is read with the stored list and the home zone, so flipping
 it switches the cadence at once through the same `flatMapLatest`.

-`TIME_SET` and `TIMEZONE_CHANGED` still self-heal within one tick, which with
-seconds off is now up to a minute. That is only reachable by changing the
-clock in system settings and returning within the five-second
-`WhileSubscribed` grace; any longer and the flow restarts and reads the new
-time at once.
+A minute is too long to wait for `TIME_SET` or `TIMEZONE_CHANGED`, which can
+arrive with the tab on screen (a network zone change on landing, split
+screen). So `ClockChanges` (`AndroidClockChanges`, a receiver registered only
+while the tab collects) is merged into the tick: those two, plus `TIME_TICK`,
+make the state re-read the clock and the zone at once. `TIME_TICK` also
+covers the one gap an aligned `delay` has, a deep sleep inside the
+five-second `WhileSubscribed` grace, which stretches the wait because the
+main looper's clock stops while the device sleeps. Every one of these is
+an extra `build`, and `distinctUntilChanged` drops it if nothing moved.

 ### Twenty-four cities, and a reorder a screen reader can reach