From 0148031907b9d17a56018fcfc608c1dced452e54 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 2 Nov 2026 19:40:06 +0100 Subject: [PATCH] fix: review findings on week start and settings motion - The last week start is saved with the activity, so a rotation no longer draws one frame in the locale's order before the stored day. - A backup with "week_start": null restores Automatic instead of rejecting the whole file. - Under reduced motion a settings sub-page fades in, matching its exit. - LocalWeekStart, rememberWeekStart and one shared rememberLocale move to ui/common; the picker, the day buttons and the summaries use them. - The "Week starts on" row shows the day Automatic resolves to. --- .../jeanlucmakiola/clockula/MainActivity.kt | 11 ++++-- .../clockula/data/backup/BackupDto.kt | 5 +-- .../clockula/ui/alarms/AlarmRow.kt | 33 +---------------- .../clockula/ui/alarms/RepeatDaySelector.kt | 8 ++-- .../clockula/ui/common/WeekStart.kt | 37 +++++++++++++++++++ .../ui/settings/ClockSettingsScreen.kt | 2 +- .../clockula/ui/settings/SettingsScreen.kt | 5 ++- .../clockula/ui/settings/WeekStartPicker.kt | 24 ++++++------ app/src/main/res/values/strings.xml | 1 + .../clockula/data/backup/BackupCodecTest.kt | 7 ++++ docs/BACKUP.md | 2 +- 11 files changed, 79 insertions(+), 56 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/ui/common/WeekStart.kt diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/MainActivity.kt b/app/src/main/java/de/jeanlucmakiola/clockula/MainActivity.kt index ac46a91..5ec3723 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/MainActivity.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/MainActivity.kt @@ -13,6 +13,7 @@ import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.LaunchedEffect 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.lifecycle.compose.collectAsStateWithLifecycle @@ -21,7 +22,7 @@ import de.jeanlucmakiola.clockula.bedtime.BedtimeIntents import de.jeanlucmakiola.clockula.data.prefs.SettingsPrefs import de.jeanlucmakiola.clockula.domain.WeekStartPref import de.jeanlucmakiola.clockula.interop.InteropIntents -import de.jeanlucmakiola.clockula.ui.alarms.LocalWeekStart +import de.jeanlucmakiola.clockula.ui.common.LocalWeekStart import de.jeanlucmakiola.clockula.ui.crash.CrashReportActivity import de.jeanlucmakiola.clockula.ui.shell.ClockulaShell import de.jeanlucmakiola.clockula.ui.shell.ShellNavigation @@ -100,9 +101,13 @@ class MainActivity : ComponentActivity() { .collectAsStateWithLifecycle(initialValue = Appearance()) val darkTheme = appearance.themeMode.isDark(isSystemInDarkTheme()) // Read here once rather than by each screen that lays out a week: - // the alarm list, the alarm editor and bedtime all do (#30). + // the alarm list, the alarm editor and bedtime all do (#30). The + // last value is saved with the activity, so a rotation or a theme + // change doesn't draw one frame in the locale's order first. + var lastWeekStart by rememberSaveable { mutableStateOf(WeekStartPref.AUTO) } val weekStart by settingsPrefs.weekStart - .collectAsStateWithLifecycle(initialValue = WeekStartPref.Auto) + .collectAsStateWithLifecycle(initialValue = WeekStartPref.parse(lastWeekStart)) + LaunchedEffect(weekStart) { lastWeekStart = weekStart.storageValue } // The system bars follow the *resolved* theme, not the device night // flag: with the device dark and ThemeMode.LIGHT stored, deriving the diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/backup/BackupDto.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/backup/BackupDto.kt index 40658ca..ac3c58a 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/backup/BackupDto.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/backup/BackupDto.kt @@ -1,6 +1,5 @@ package de.jeanlucmakiola.clockula.data.backup -import de.jeanlucmakiola.clockula.domain.WeekStartPref import de.jeanlucmakiola.clockula.domain.bedtime.BedtimeSettings import de.jeanlucmakiola.clockula.domain.ring.SilenceAfter import kotlinx.serialization.SerialName @@ -113,8 +112,8 @@ data class SettingsDto( @SerialName("screen_saver_night_mode") val screenSaverNightMode: Boolean = false, /** Absent (a file from before #26) or out of range keeps the tile's default. */ @SerialName("quick_tile_duration_millis") val quickTileDurationMillis: Long? = null, - /** Absent (a file from before #30) or unrecognised follows the locale, the default. */ - @SerialName("week_start") val weekStart: String = WeekStartPref.AUTO, + /** Absent (a file from before #30), null or unrecognised follows the locale, the default. */ + @SerialName("week_start") val weekStart: String? = null, ) /** diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt index 8ad28bc..e953b07 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt @@ -16,12 +16,8 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.runtime.Composable -import androidx.compose.runtime.ProvidableCompositionLocal -import androidx.compose.runtime.compositionLocalOf -import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.platform.LocalConfiguration import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.CustomAccessibilityAction @@ -40,17 +36,15 @@ import de.jeanlucmakiola.clockula.domain.PauseRange import de.jeanlucmakiola.clockula.domain.RepeatDays import de.jeanlucmakiola.clockula.domain.RepeatDaysSummary import de.jeanlucmakiola.clockula.domain.RepeatSummary -import de.jeanlucmakiola.clockula.domain.WeekStartPref import de.jeanlucmakiola.clockula.domain.format.NextFireLabel -import de.jeanlucmakiola.clockula.domain.resolveFirstDay import de.jeanlucmakiola.clockula.ui.common.SwipeToDelete import de.jeanlucmakiola.clockula.ui.common.rememberAlarmTimeFormatter +import de.jeanlucmakiola.clockula.ui.common.rememberLocale +import de.jeanlucmakiola.clockula.ui.common.rememberWeekStart import de.jeanlucmakiola.floret.components.GroupedListInset import de.jeanlucmakiola.floret.components.GroupedSurface import de.jeanlucmakiola.floret.components.Position -import java.time.DayOfWeek import java.time.format.TextStyle -import java.util.Locale /** * A standalone alarm card. The time and switch share the first line, leaving @@ -270,26 +264,3 @@ internal fun repeatSummaryText(repeatDays: RepeatDays): String { } } } - -/** - * The stored "Week starts on" choice (#30), provided once by `MainActivity` for - * every screen that lays out a week. Auto outside it, as in a preview. - */ -internal val LocalWeekStart: ProvidableCompositionLocal = - compositionLocalOf { WeekStartPref.Auto } - -/** The chosen first day of week, or the locale's own, re-read with the configuration. */ -@Composable -internal fun rememberWeekStart(): DayOfWeek { - val locale = rememberLocale() - val weekStart = LocalWeekStart.current - return remember(locale, weekStart) { weekStart.resolveFirstDay(locale) } -} - -@Composable -private fun rememberLocale(): Locale { - // Keyed on the configuration, so a language change re-reads both the - // locale's day names and where its week begins. - val configuration = LocalConfiguration.current - return remember(configuration) { Locale.getDefault() } -} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/RepeatDaySelector.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/RepeatDaySelector.kt index d179b33..3b226e2 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/RepeatDaySelector.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/RepeatDaySelector.kt @@ -8,20 +8,19 @@ import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi import androidx.compose.material3.Text import androidx.compose.material3.ToggleButton import androidx.compose.runtime.Composable -import androidx.compose.runtime.remember import androidx.compose.ui.Modifier -import androidx.compose.ui.platform.LocalConfiguration import androidx.compose.ui.semantics.contentDescription import androidx.compose.ui.semantics.semantics import androidx.compose.ui.unit.dp import de.jeanlucmakiola.clockula.domain.RepeatDays import de.jeanlucmakiola.clockula.domain.RepeatDaysOrder +import de.jeanlucmakiola.clockula.ui.common.rememberLocale +import de.jeanlucmakiola.clockula.ui.common.rememberWeekStart import de.jeanlucmakiola.floret.components.GroupedListInset import de.jeanlucmakiola.floret.components.GroupedSurface import de.jeanlucmakiola.floret.components.Position import java.time.DayOfWeek import java.time.format.TextStyle -import java.util.Locale /** * Seven equal toggle buttons in one row — the Expressive round↔squircle morph @@ -39,8 +38,7 @@ internal fun RepeatDaySelector( modifier: Modifier = Modifier, position: Position = Position.Alone, ) { - val configuration = LocalConfiguration.current - val locale = remember(configuration) { Locale.getDefault() } + val locale = rememberLocale() val days = RepeatDaysOrder.startingAt(rememberWeekStart()) GroupedSurface( diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/WeekStart.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/WeekStart.kt new file mode 100644 index 0000000..f4b412f --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/WeekStart.kt @@ -0,0 +1,37 @@ +package de.jeanlucmakiola.clockula.ui.common + +import androidx.compose.runtime.Composable +import androidx.compose.runtime.ProvidableCompositionLocal +import androidx.compose.runtime.compositionLocalOf +import androidx.compose.runtime.remember +import androidx.compose.ui.platform.LocalConfiguration +import de.jeanlucmakiola.clockula.domain.WeekStartPref +import de.jeanlucmakiola.clockula.domain.resolveFirstDay +import java.time.DayOfWeek +import java.util.Locale + +/** + * The stored "Week starts on" choice (#30), provided once by `MainActivity` for + * every screen that lays out a week. Auto outside it, as in a preview. + */ +internal val LocalWeekStart: ProvidableCompositionLocal = + compositionLocalOf { WeekStartPref.Auto } + +/** The chosen first day of week, or the locale's own, re-read with the configuration. */ +@Composable +internal fun rememberWeekStart(): DayOfWeek { + val locale = rememberLocale() + val weekStart = LocalWeekStart.current + return remember(locale, weekStart) { weekStart.resolveFirstDay(locale) } +} + +/** + * The locale day names and the week's first day are read in. Keyed on the + * configuration, so a language change re-reads both; shared, so the repeat-day + * buttons, the alarm summaries and the week-start picker never disagree. + */ +@Composable +internal fun rememberLocale(): Locale { + val configuration = LocalConfiguration.current + return remember(configuration) { Locale.getDefault() } +} 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 73ef158..0f2541b 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 @@ -101,7 +101,7 @@ internal fun ClockSettingsScreen( Spacer(Modifier.height(24.dp)) GroupedRow( title = stringResource(R.string.settings_week_start), - summary = weekStartText(state.weekStart), + summary = weekStartSummary(state.weekStart), position = Position.Alone, onClick = { showWeekStart = true }, ) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/SettingsScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/SettingsScreen.kt index 35b135a..f871d9e 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/SettingsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/SettingsScreen.kt @@ -63,6 +63,7 @@ import de.jeanlucmakiola.floret.crash.CrashReporter import de.jeanlucmakiola.floret.crash.openIssueTracker import de.jeanlucmakiola.floret.crash.submitCrashReport import de.jeanlucmakiola.floret.identity.predictiveBackExit +import de.jeanlucmakiola.floret.identity.rememberReduceMotion /** The settings sub-screens reached from the hub's category rows. */ private enum class SettingsSection { Appearance, Alarms, Timers, Clock } @@ -182,9 +183,11 @@ fun SettingsScreen( @Composable private fun SlideInSection(visible: Boolean, content: @Composable () -> Unit) { val slideSpec = MaterialTheme.motionScheme.defaultSpatialSpec() + // Reduced motion fades it in, as predictiveBackExit already fades it out. + val enter = if (rememberReduceMotion()) fadeIn() else slideInHorizontally(slideSpec) { it } + fadeIn() AnimatedVisibility( visible = visible, - enter = slideInHorizontally(slideSpec) { it } + fadeIn(), + enter = enter, exit = predictiveBackExit(), ) { content() } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/WeekStartPicker.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/WeekStartPicker.kt index 47b9eaa..57971fb 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/WeekStartPicker.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/WeekStartPicker.kt @@ -1,23 +1,31 @@ package de.jeanlucmakiola.clockula.ui.settings import androidx.compose.runtime.Composable -import androidx.compose.runtime.remember -import androidx.compose.ui.platform.LocalConfiguration import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import de.jeanlucmakiola.clockula.R import de.jeanlucmakiola.clockula.domain.WeekStartPref import de.jeanlucmakiola.clockula.domain.resolveFirstDay +import de.jeanlucmakiola.clockula.ui.common.rememberLocale import de.jeanlucmakiola.floret.components.GroupedHint import de.jeanlucmakiola.floret.components.OptionPicker import java.time.format.TextStyle -import java.util.Locale /** A pinned day is the locale's own weekday name, so no per-day string is needed. */ @Composable internal fun weekStartText(pref: WeekStartPref): String = when (pref) { WeekStartPref.Auto -> stringResource(R.string.settings_week_start_auto) - is WeekStartPref.Day -> pref.day.getDisplayName(TextStyle.FULL, rememberDefaultLocale()) + is WeekStartPref.Day -> pref.day.getDisplayName(TextStyle.FULL, rememberLocale()) +} + +/** The row's summary: a pinned day, or "Automatic (Monday)" with the day it resolves to. */ +@Composable +internal fun weekStartSummary(pref: WeekStartPref): String = when (pref) { + WeekStartPref.Auto -> stringResource( + R.string.settings_week_start_auto_row, + weekStartText(WeekStartPref.Day(pref.resolveFirstDay(rememberLocale()))), + ) + is WeekStartPref.Day -> weekStartText(pref) } /** @@ -32,7 +40,7 @@ internal fun WeekStartPicker( onSelect: (WeekStartPref) -> Unit, onDismiss: () -> Unit, ) { - val locale = rememberDefaultLocale() + val locale = rememberLocale() OptionPicker( title = stringResource(R.string.settings_week_start), header = { GroupedHint(stringResource(R.string.settings_week_start_hint), verticalPadding = 8.dp) }, @@ -53,9 +61,3 @@ internal fun WeekStartPicker( onDismiss = onDismiss, ) } - -@Composable -private fun rememberDefaultLocale(): Locale { - val configuration = LocalConfiguration.current - return remember(configuration) { Locale.getDefault() } -} diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 38e9a2e..20b994a 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -370,6 +370,7 @@ Week starts on Automatic Currently %1$s + Automatic (%1$s) The day every week begins on, in alarms and bedtime. Screen saver Clock style 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 c59bb79..2e1caeb 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 @@ -212,6 +212,13 @@ class BackupCodecTest { assertThat((decoded as BackupResult.Success).content.settings.weekStart).isEqualTo(WeekStartPref.Auto) } + @Test + fun `a null week start follows the locale rather than rejecting the file`() { + val decoded = BackupCodec.decode(settingsOnly(""""week_start": null""")) + + assertThat((decoded as BackupResult.Success).content.settings.weekStart).isEqualTo(WeekStartPref.Auto) + } + private fun settingsOnly(settings: String): String = """{ "format": "clockula-backup", "version": 1, "exportedAt": 0, "settings": { $settings } }""" diff --git a/docs/BACKUP.md b/docs/BACKUP.md index 2504de3..e02f718 100644 --- a/docs/BACKUP.md +++ b/docs/BACKUP.md @@ -221,7 +221,7 @@ alarm or timer inherits depends on it. The same goes for `week_start` is the first day of the week (#30): `AUTO`, which follows the locale, or a day name from `MONDAY` to `SUNDAY`. A file without it, or with a -value this version does not know, restores `AUTO`. +value this version does not know or `null`, restores `AUTO`. Every numeric/duration value is clamped on *apply* using the same ranges and encoding as the existing preference setters, never on parse.