From d744fa8d98420ad9a490d64b57c287928e8e7de2 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 08:18:35 +0200 Subject: [PATCH] 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. --- .../clockula/domain/backup/BackupContent.kt | 2 +- .../domain/screensaver/ScreenSaverAlarmDay.kt | 17 ++++ .../domain/widget/NextAlarmWidgetModel.kt | 6 +- .../ui/screensaver/ScreenSaverContent.kt | 85 +++++++++++++------ .../screensaver/ScreenSaverSettingsSection.kt | 20 ++++- .../ui/settings/ClockSettingsScreen.kt | 3 - app/src/main/res/xml/screen_saver_info.xml | 6 +- .../clockula/data/backup/BackupCodecTest.kt | 1 + .../screensaver/ScreenSaverAlarmDayTest.kt | 28 ++++++ .../ui/screensaver/ScreenSaverSourceTest.kt | 2 + 10 files changed, 135 insertions(+), 35 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/domain/screensaver/ScreenSaverAlarmDay.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/clockula/domain/screensaver/ScreenSaverAlarmDayTest.kt diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/backup/BackupContent.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/backup/BackupContent.kt index 79ff6a8..adc3f9a 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/backup/BackupContent.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/backup/BackupContent.kt @@ -57,5 +57,5 @@ data class BackupSettings( val dynamicColor: Boolean, val defaults: ClockDefaults, val timerPresets: List, - val screenSaver: ScreenSaverSettings = ScreenSaverSettings(), + val screenSaver: ScreenSaverSettings, ) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/screensaver/ScreenSaverAlarmDay.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/screensaver/ScreenSaverAlarmDay.kt new file mode 100644 index 0000000..36b6fd5 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/screensaver/ScreenSaverAlarmDay.kt @@ -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)) +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/widget/NextAlarmWidgetModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/widget/NextAlarmWidgetModel.kt index 85f3276..c0e0cc0 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/widget/NextAlarmWidgetModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/widget/NextAlarmWidgetModel.kt @@ -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(), ) } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/screensaver/ScreenSaverContent.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/screensaver/ScreenSaverContent.kt index f8d23cf..2dc90f3 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/screensaver/ScreenSaverContent.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/screensaver/ScreenSaverContent.kt @@ -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 { diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/screensaver/ScreenSaverSettingsSection.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/screensaver/ScreenSaverSettingsSection.kt index b86970b..f350309 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/screensaver/ScreenSaverSettingsSection.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/screensaver/ScreenSaverSettingsSection.kt @@ -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), diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/ClockSettingsScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/ClockSettingsScreen.kt index 20241ff..43d0bbc 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/ClockSettingsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/ClockSettingsScreen.kt @@ -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) } } diff --git a/app/src/main/res/xml/screen_saver_info.xml b/app/src/main/res/xml/screen_saver_info.xml index d231e54..65f7d1b 100644 --- a/app/src/main/res/xml/screen_saver_info.xml +++ b/app/src/main/res/xml/screen_saver_info.xml @@ -1,5 +1,7 @@ + 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). --> + android:settingsActivity="de.jeanlucmakiola.clockula.ui.screensaver.ScreenSaverSettingsActivity" /> diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/data/backup/BackupCodecTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/data/backup/BackupCodecTest.kt index 3b6ae54..4dd6f47 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/data/backup/BackupCodecTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/data/backup/BackupCodecTest.kt @@ -87,6 +87,7 @@ class BackupCodecTest { dynamicColor = true, defaults = ClockDefaults(), timerPresets = emptyList(), + screenSaver = ScreenSaverSettings(), ), ) val decoded = BackupCodec.decode(BackupCodec.encode(original)) diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/screensaver/ScreenSaverAlarmDayTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/screensaver/ScreenSaverAlarmDayTest.kt new file mode 100644 index 0000000..826e9f4 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/clockula/domain/screensaver/ScreenSaverAlarmDayTest.kt @@ -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() + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/screensaver/ScreenSaverSourceTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/screensaver/ScreenSaverSourceTest.kt index 1515427..cb56605 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/screensaver/ScreenSaverSourceTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/screensaver/ScreenSaverSourceTest.kt @@ -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 {