fix(screensaver): address review findings

The dream's settings activity is named by class alone, so the gear opens the installed build rather than always the production package. The screen saver now formats time with the app's own 12/24-hour rule and the shared currentLocale(), sizes the numerals with the AM/PM marker counted, dates a next alarm a week or more away, and gives the analog face the time as its description. The screen saver header in Settings > Clock waits for its rows, and BackupSettings.screenSaver is required.
2026-10-22 08:18:35 +02:00
parent 1e29226364
commit d744fa8d98
10 changed files with 135 additions and 35 deletions
@@ -57,5 +57,5 @@ data class BackupSettings(
    val dynamicColor: Boolean,
    val defaults: ClockDefaults,
    val timerPresets: List<Duration>,
-    val screenSaver: ScreenSaverSettings = ScreenSaverSettings(),
+    val screenSaver: ScreenSaverSettings,
 )
@@ -0,0 +1,17 @@
+package de.jeanlucmakiola.clockula.domain.screensaver
+
+import java.time.LocalDate
+
+/**
+ * How the screen saver names the day of the next alarm. A weekday alone
+ * ("Thu") is only unambiguous for the coming six days: a weekly alarm whose
+ * time has passed today next rings in seven, and a skipped one in fourteen,
+ * and either would read as today's. Those carry their date as well.
+ */
+object ScreenSaverAlarmDay {
+
+    private const val DAYS_A_WEEKDAY_NAMES = 7L
+
+    fun weekdayIsEnough(fireDate: LocalDate, today: LocalDate): Boolean =
+        fireDate.isBefore(today.plusDays(DAYS_A_WEEKDAY_NAMES))
+}
@@ -2,6 +2,7 @@ package de.jeanlucmakiola.clockula.domain.widget

 import de.jeanlucmakiola.clockula.domain.alarm.UpcomingAlarm
 import java.time.DayOfWeek
+import java.time.LocalDate
 import java.time.ZoneId
 import java.time.ZonedDateTime
 import kotlin.time.Instant
@@ -10,7 +11,8 @@ import kotlin.time.toJavaInstant
 /**
 * What the next-alarm widget shows. [hour]/[minute]/[dayOfWeek] are absolute —
 * never a relative "in 3h" — so the widget cannot go stale between the writes
- * that refresh it (`docs/PLAN.md` §9).
+ * that refresh it (`docs/PLAN.md` §9). [date] is the fire's local date, for
+ * the screen saver, which names an alarm a week or more away by its date.
 */
 data class NextAlarmWidgetModel(
    val hasAlarm: Boolean,
@@ -19,6 +21,7 @@ data class NextAlarmWidgetModel(
    val dayOfWeek: DayOfWeek? = null,
    val isToday: Boolean = false,
    val label: String = "",
+    val date: LocalDate? = null,
 )

 object NextAlarmWidgetPresenter {
@@ -36,6 +39,7 @@ object NextAlarmWidgetPresenter {
            dayOfWeek = zonedFire.dayOfWeek,
            isToday = zonedFire.toLocalDate() == zonedNow.toLocalDate(),
            label = next.alarm.label,
+            date = zonedFire.toLocalDate(),
        )
    }
 }
@@ -6,6 +6,7 @@ import androidx.compose.animation.core.snap
 import androidx.compose.animation.core.tween
 import androidx.compose.foundation.background
 import androidx.compose.foundation.layout.Arrangement
+import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.BoxWithConstraints
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.Row
@@ -27,7 +28,6 @@ import androidx.compose.runtime.remember
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.layout.Layout
-import androidx.compose.ui.platform.LocalConfiguration
 import androidx.compose.ui.platform.LocalDensity
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.semantics.contentDescription
@@ -38,18 +38,21 @@ import androidx.compose.ui.text.font.FontWeight
 import androidx.compose.ui.text.withStyle
 import androidx.compose.ui.unit.Dp
 import androidx.compose.ui.unit.dp
-import androidx.core.os.ConfigurationCompat
 import de.jeanlucmakiola.clockula.R
+import de.jeanlucmakiola.clockula.domain.screensaver.ScreenSaverAlarmDay
 import de.jeanlucmakiola.clockula.domain.screensaver.ScreenSaverDrift
 import de.jeanlucmakiola.clockula.domain.screensaver.ScreenSaverStyle
 import de.jeanlucmakiola.clockula.domain.widget.NextAlarmWidgetModel
 import de.jeanlucmakiola.clockula.domain.worldclock.DayNight
 import de.jeanlucmakiola.clockula.ui.worldclock.WorldClockFace
 import de.jeanlucmakiola.floret.identity.rememberReduceMotion
+import de.jeanlucmakiola.floret.locale.currentLocale
 import java.time.LocalDateTime
 import java.time.LocalTime
+import java.time.chrono.IsoChronology
 import java.time.format.DateTimeFormatter
-import java.time.temporal.TemporalAdjusters
+import java.time.format.DateTimeFormatterBuilder
+import java.time.format.FormatStyle
 import java.util.Locale
 import kotlin.math.roundToInt

@@ -99,16 +102,11 @@ private fun Drifting(position: ScreenSaverDrift.Position, content: @Composable (

@Composable
 private fun Clock(state: ScreenSaverUiState, side: Dp) {
-    val locale = ConfigurationCompat.getLocales(LocalConfiguration.current)[0] ?: Locale.ROOT
+    val locale = currentLocale()
    Column(horizontalAlignment = Alignment.CenterHorizontally) {
        when (state.settings.style) {
            ScreenSaverStyle.DIGITAL -> DigitalTime(state.time.toLocalTime(), state.use24Hour, locale, side)
-            ScreenSaverStyle.ANALOG -> WorldClockFace(
-                time = state.time.toLocalTime(),
-                dayFraction = DayNight.fractionAt(state.time.toLocalTime()),
-                modifier = Modifier.size(side * ScreenSaverDefaults.FACE_FRACTION),
-                showSecondHand = false,
-            )
+            ScreenSaverStyle.ANALOG -> AnalogTime(state.time.toLocalTime(), state.use24Hour, locale, side)
        }
        Spacer(Modifier.height(ScreenSaverDefaults.Gap))
        Text(
@@ -132,15 +130,16 @@ private fun Clock(state: ScreenSaverUiState, side: Dp) {
@Composable
 private fun DigitalTime(time: LocalTime, use24Hour: Boolean, locale: Locale, side: Dp) {
    val parts = remember(time, use24Hour, locale) { timeParts(time, use24Hour, locale) }
-    val digits = parts.before.length + parts.after.length
-    // About 0.6em per tabular digit or colon, so the readout fills ~90 % of
-    // the shorter side at any length.
-    val size = minOf(side * 0.9f / (digits.coerceAtLeast(4) * 0.6f), side * 0.4f)
+    // About 0.6em per tabular digit or colon, the marker at its own smaller
+    // size, so the readout fills ~90 % of the shorter side at any length.
+    val ems = (parts.before.length + parts.after.length + parts.marker.length * MARKER_SCALE)
+        .coerceAtLeast(4f) * 0.6f
+    val size = minOf(side * 0.9f / ems, side * 0.4f)
    val fontSize = with(LocalDensity.current) { size.toSp() }
    val text = buildAnnotatedString {
        append(parts.before)
        if (parts.marker.isNotEmpty()) {
-            withStyle(SpanStyle(fontSize = fontSize * 0.3f)) { append(parts.marker) }
+            withStyle(SpanStyle(fontSize = fontSize * MARKER_SCALE)) { append(parts.marker) }
        }
        append(parts.after)
    }
@@ -159,6 +158,25 @@ private fun DigitalTime(time: LocalTime, use24Hour: Boolean, locale: Locale, sid
    )
 }

+/**
+ * The world clock's own face, without its second hand (the screen saver
+ * redraws once a minute). The face hides itself from screen readers because
+ * on the World clock tab a text readout sits under it; here nothing does, so
+ * the face is given the time as its description.
+ */
+@Composable
+private fun AnalogTime(time: LocalTime, use24Hour: Boolean, locale: Locale, side: Dp) {
+    val description = remember(time, use24Hour, locale) { timeFormatter(use24Hour, locale).format(time) }
+    Box(Modifier.semantics { contentDescription = description }) {
+        WorldClockFace(
+            time = time,
+            dayFraction = DayNight.fractionAt(time),
+            modifier = Modifier.size(side * ScreenSaverDefaults.FACE_FRACTION),
+            showSecondHand = false,
+        )
+    }
+}
+
@Composable
 private fun NextAlarm(model: NextAlarmWidgetModel, now: LocalDateTime, use24Hour: Boolean, locale: Locale) {
    val label = remember(model, now.toLocalDate(), use24Hour, locale) { nextAlarmLabel(model, now, use24Hour, locale) }
@@ -182,11 +200,30 @@ private fun NextAlarm(model: NextAlarmWidgetModel, now: LocalDateTime, use24Hour
    }
 }

+/** The AM/PM marker's size against the digits'. */
+private const val MARKER_SCALE = 0.3f
+
+/**
+ * The app's one 12/24-hour rule, as `rememberLocalTimeFormatter` applies it:
+ * `HH:mm`, or the locale's own short time. Spelled as a pattern here because
+ * the numerals need it split around the AM/PM marker; the same pattern, so the
+ * screen saver never writes a time the alarm list writes differently.
+ */
+private fun timePattern(use24Hour: Boolean, locale: Locale): String =
+    if (use24Hour) {
+        "HH:mm"
+    } else {
+        DateTimeFormatterBuilder.getLocalizedDateTimePattern(null, FormatStyle.SHORT, IsoChronology.INSTANCE, locale)
+    }
+
+private fun timeFormatter(use24Hour: Boolean, locale: Locale): DateTimeFormatter =
+    DateTimeFormatter.ofPattern(timePattern(use24Hour, locale), locale)
+
 /** A time split around its AM/PM marker; [marker] is empty in 24-hour time. */
 private data class TimeParts(val before: String, val marker: String, val after: String)

 private fun timeParts(time: LocalTime, use24Hour: Boolean, locale: Locale): TimeParts {
-    val pattern = DateFormat.getBestDateTimePattern(locale, if (use24Hour) "Hmm" else "hmma")
+    val pattern = timePattern(use24Hour, locale)
    val markerAt = pattern.indexOf('a')
    if (markerAt < 0) return TimeParts(format(time, pattern, locale), "", "")
    return runCatching {
@@ -207,16 +244,16 @@ private fun dateLabel(now: LocalDateTime, locale: Locale): String =

 /**
 * The next alarm as the platform's own screen saver writes it: the short
- * weekday and the time, "Tue 7:00 AM", in the locale's order. The weekday
- * is always there, so a time alone can never be read as today's.
+ * weekday and the time, "Tue 7:00 AM". The weekday is always there, so a
+ * time alone can never be read as today's, and an alarm a week or more away
+ * carries its date too ([ScreenSaverAlarmDay]), "Thu, Oct 29 7:00 AM".
 */
 private fun nextAlarmLabel(model: NextAlarmWidgetModel, now: LocalDateTime, use24Hour: Boolean, locale: Locale): String {
-    val day = model.dayOfWeek ?: now.dayOfWeek
-    val at = now.toLocalDate()
-        .with(TemporalAdjusters.nextOrSame(day))
-        .atTime(model.hour, model.minute)
-    val pattern = DateFormat.getBestDateTimePattern(locale, if (use24Hour) "EEEHmm" else "EEEhmma")
-    return DateTimeFormatter.ofPattern(pattern, locale).format(at)
+    val date = model.date ?: now.toLocalDate()
+    val daySkeleton = if (ScreenSaverAlarmDay.weekdayIsEnough(date, now.toLocalDate())) "EEE" else "EEEMMMd"
+    val day = DateTimeFormatter.ofPattern(DateFormat.getBestDateTimePattern(locale, daySkeleton), locale).format(date)
+    val time = timeFormatter(use24Hour, locale).format(LocalTime.of(model.hour, model.minute))
+    return "$day $time"
 }

 private object ScreenSaverDefaults {
@@ -3,27 +3,34 @@ package de.jeanlucmakiola.clockula.ui.screensaver
 import android.content.Context
 import android.content.Intent
 import android.provider.Settings
+import androidx.compose.foundation.layout.Spacer
+import androidx.compose.foundation.layout.height
 import androidx.compose.material3.Switch
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.runtime.setValue
+import androidx.compose.ui.Modifier
 import androidx.compose.ui.platform.LocalContext
 import androidx.compose.ui.res.stringResource
-import androidx.hilt.navigation.compose.hiltViewModel
+import androidx.compose.ui.unit.dp
+import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel
 import androidx.lifecycle.compose.collectAsStateWithLifecycle
 import de.jeanlucmakiola.clockula.R
 import de.jeanlucmakiola.clockula.domain.screensaver.ScreenSaverStyle
 import de.jeanlucmakiola.floret.components.GroupedRow
+import de.jeanlucmakiola.floret.components.GroupedSectionHeader
 import de.jeanlucmakiola.floret.components.OptionPicker
 import de.jeanlucmakiola.floret.components.Position

 /**
 * The screen saver's settings as a run of grouped rows: its style, night
- * mode, and a way to see it now. [inApp] adds a last row into Android's own
- * screen saver settings, where Clockula is chosen and its start condition
- * set; the system's settings screen opens this one, so there it is left out.
+ * mode, and a way to see it now. [inApp] — Settings → Clock — puts them
+ * under their own header, and adds a last row into Android's own screen
+ * saver settings, where Clockula is chosen and its start condition set; the
+ * system's settings screen opens this one, so there both are left out.
+ * Nothing is drawn, header included, until the settings have been read.
 */
@Composable
 internal fun ScreenSaverSettingsSection(
@@ -35,6 +42,11 @@ internal fun ScreenSaverSettingsSection(
    val context = LocalContext.current
    var showStyle by rememberSaveable { mutableStateOf(false) }

+    if (inApp) {
+        Spacer(Modifier.height(16.dp))
+        GroupedSectionHeader(stringResource(R.string.screen_saver_title))
+    }
+
    GroupedRow(
        title = stringResource(R.string.screen_saver_style),
        summary = styleLabel(current.style),
@@ -10,7 +10,6 @@ import de.jeanlucmakiola.clockula.R
 import de.jeanlucmakiola.clockula.ui.screensaver.ScreenSaverSettingsSection
 import de.jeanlucmakiola.floret.components.CollapsingScaffold
 import de.jeanlucmakiola.floret.components.GroupedRow
-import de.jeanlucmakiola.floret.components.GroupedSectionHeader
 import de.jeanlucmakiola.floret.components.Position

 /**
@@ -35,8 +34,6 @@ internal fun ClockSettingsScreen(
            onClick = viewModel::onOpenHomeZonePicker,
        )

-        Spacer(Modifier.height(16.dp))
-        GroupedSectionHeader(stringResource(R.string.screen_saver_title))
        ScreenSaverSettingsSection(inApp = true)
    }
 }
@@ -1,5 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <!-- The screen saver's entry in Settings → Display → Screen saver: the gear
-     beside it opens ScreenSaverSettingsActivity. -->
+     beside it opens ScreenSaverSettingsActivity. The class name alone, with
+     no "package/" prefix, so Settings pairs it with the installed build's own
+     package (the debug and releaseTest builds add an applicationId suffix). -->
 <dream xmlns:android="http://schemas.android.com/apk/res/android"
-    android:settingsActivity="de.jeanlucmakiola.clockula/.ui.screensaver.ScreenSaverSettingsActivity" />
+    android:settingsActivity="de.jeanlucmakiola.clockula.ui.screensaver.ScreenSaverSettingsActivity" />
@@ -87,6 +87,7 @@ class BackupCodecTest {
                dynamicColor = true,
                defaults = ClockDefaults(),
                timerPresets = emptyList(),
+                screenSaver = ScreenSaverSettings(),
            ),
        )
        val decoded = BackupCodec.decode(BackupCodec.encode(original))
@@ -0,0 +1,28 @@
+package de.jeanlucmakiola.clockula.domain.screensaver
+
+import com.google.common.truth.Truth.assertThat
+import org.junit.jupiter.api.Test
+import java.time.LocalDate
+
+class ScreenSaverAlarmDayTest {
+
+    /** A Thursday. */
+    private val today = LocalDate.parse("2026-10-22")
+
+    @Test
+    fun `today and the next six days are named by their weekday alone`() {
+        for (days in 0L..6L) {
+            assertThat(ScreenSaverAlarmDay.weekdayIsEnough(today.plusDays(days), today)).isTrue()
+        }
+    }
+
+    @Test
+    fun `a week away is the same weekday as today, so it carries its date`() {
+        assertThat(ScreenSaverAlarmDay.weekdayIsEnough(today.plusDays(7), today)).isFalse()
+    }
+
+    @Test
+    fun `a skipped weekly alarm two weeks away carries its date`() {
+        assertThat(ScreenSaverAlarmDay.weekdayIsEnough(today.plusDays(14), today)).isFalse()
+    }
+}
@@ -11,6 +11,7 @@ import de.jeanlucmakiola.clockula.testing.BERLIN
 import de.jeanlucmakiola.clockula.testing.NEW_YORK
 import de.jeanlucmakiola.clockula.testing.alarmEngineHarness
 import de.jeanlucmakiola.clockula.testing.instantAt
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableSharedFlow
 import kotlinx.coroutines.flow.toList
@@ -33,6 +34,7 @@ import kotlin.time.Duration.Companion.seconds
 * run to idle: an idle scheduler would skip through minute after minute of
 * virtual time.
 */
+@OptIn(ExperimentalCoroutinesApi::class)
 class ScreenSaverSourceTest {

    private class FakeClockChanges : ClockChanges {