From 58c9bfebb14259f9421bd9cf0a6adefdc4871cad Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 18:51:49 +0200 Subject: [PATCH 1/9] feat(alarms): say how long until an alarm rings when you set it After a user action in the app sets an alarm, a snackbar says how long until it rings: "Alarm set for 7 hours and 12 minutes from now". The row already said "in 7h 12m", but nothing drew the eye to it, so an AM/PM or wrong-day slip went unnoticed until the morning. It is raised by switching an alarm on in the list, and in the editor by picking a time (or keeping a new alarm's default one), arming or disarming the skip, and changing the repeat days. Day taps wait a second to settle, so Mon-Fri is one snackbar rather than five; a newer countdown always replaces the one on screen. Switching the announced alarm back off takes it back. A switched-off, ringing or snoozed alarm says nothing, and the AlarmClock intents never reach it, since those already get the status-bar icon. AlarmSetCountdown decides what it says, purely, with the row's rounding; the engine gains upcomingFor(), a read-only resolve of one alarm. Fixes #19 --- CHANGELOG.md | 3 + .../clockula/alarm/AlarmEngine.kt | 11 ++ .../domain/alarm/AlarmSetCountdown.kt | 23 ++++ .../clockula/ui/alarms/AlarmEditorScreen.kt | 19 ++- .../ui/alarms/AlarmEditorViewModel.kt | 60 +++++++- .../clockula/ui/alarms/AlarmSetSnackbar.kt | 111 +++++++++++++++ .../clockula/ui/alarms/AlarmsScreen.kt | 7 + .../clockula/ui/alarms/AlarmsViewModel.kt | 11 +- app/src/main/res/values/strings.xml | 16 +++ .../domain/alarm/AlarmSetCountdownTest.kt | 79 +++++++++++ .../ui/alarms/AlarmEditorViewModelTest.kt | 130 ++++++++++++++++++ .../clockula/ui/alarms/AlarmsViewModelTest.kt | 66 +++++++++ docs/ARCHITECTURE.md | 12 +- 13 files changed, 540 insertions(+), 8 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdownTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 76d4d0d..e0208ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ The first release of Clockula: alarms, timers, a stopwatch and a world clock. can play it vibrates. - An alarm list that shows when each alarm will ring and marks the next one. Changes apply as you make them, with no Save button. +- Setting an alarm, switching it on or changing its time or repeat days says + how long until it rings, such as "Alarm set for 7 hours and 12 minutes from + now", so an AM/PM or wrong-day slip shows at once. - Repeat days that start on your locale's first day of the week, and a "skip next alarm" switch so you can sleep in once without turning the alarm off. - Per-alarm sound, vibration, gradual volume, snooze length and limit, and a diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt index d9ff0a9..d567025 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -204,6 +204,17 @@ class AlarmEngine @Inject constructor( ) } + /** + * One alarm's [upcoming] entry, resolved against the wall clock now. Read-only + * like [upcoming], and for the same reason it takes no lock: it writes + * nothing. Null for an id no alarm has. + */ + suspend fun upcomingFor(alarmId: Long): UpcomingAlarm? { + val alarm = alarms.find(alarmId) ?: return null + val resolution = AlarmResolver.resolve(alarm, states.state(alarmId), wallClock.now(), zones.current()) + return UpcomingAlarm(alarm, resolution.nextFire, resolution.source) + } + private suspend fun rescheduleLocked(): ScheduledFire? { val now = wallClock.now() val zone = zones.current() diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt new file mode 100644 index 0000000..3efd675 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt @@ -0,0 +1,23 @@ +package de.jeanlucmakiola.clockula.domain.alarm + +import de.jeanlucmakiola.clockula.domain.format.NextFireFormat +import de.jeanlucmakiola.clockula.domain.format.NextFireLabel +import kotlin.time.Instant + +/** + * What "Alarm set for 7 hours and 12 minutes from now" says once the user has + * set an alarm, or null when it should say nothing (#19). + * + * Only a natural occurrence is announced. A switched-off alarm will not ring at + * all; a ringing or snoozed one is answering the snooze, not the change the + * user just made, and "set for 9 minutes from now" would read as a mistake. + */ +object AlarmSetCountdown { + /** Pure. The rounding is [NextFireFormat]'s, so the snackbar and the row agree. */ + fun of(upcoming: UpcomingAlarm?, now: Instant): NextFireLabel? { + if (upcoming == null || !upcoming.alarm.enabled) return null + if (upcoming.source != FireSource.OCCURRENCE) return null + val nextFire = upcoming.nextFire ?: return null + return NextFireFormat.labelFor(nextFire, now) + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt index 3e4a3dc..44787a6 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt @@ -11,6 +11,8 @@ import androidx.compose.material3.AlertDialog import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.material3.TextButton @@ -18,6 +20,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier @@ -59,6 +62,8 @@ fun AlarmEditorScreen( val previewPlaying by viewModel.previewPlaying.collectAsStateWithLifecycle() var confirmingDelete by rememberSaveable { mutableStateOf(false) } val formatTime = rememberAlarmTimeFormatter() + val snackbarHost = remember { SnackbarHostState() } + AlarmSetSnackbarEffect(viewModel.snackbars, snackbarHost) // Registered unconditionally, and deliberately **not** inside the // `picker?.let` below: the picker's open flag is ViewModel state that does @@ -84,6 +89,10 @@ fun AlarmEditorScreen( onBack = onBack, modifier = modifier, predictiveBack = true, + // Lifted over the live pill, which floats above every screen. + snackbarHost = { + SnackbarHost(snackbarHost, Modifier.padding(bottom = LocalLivePillInset.current)) + }, actions = { if (state is AlarmEditorUiState.Editing) { IconButton(onClick = { confirmingDelete = true }) { @@ -144,6 +153,9 @@ private fun AlarmEditorContent( // about losing a half-entered time). var openPicker by rememberSaveable { mutableStateOf(null) } var pickingTime by rememberSaveable { mutableStateOf(openTimePicker) } + // True until the picker a new alarm opens on is first closed: cancelling + // that one still leaves a freshly set alarm to announce (#19). + var pickingNewAlarm by rememberSaveable { mutableStateOf(openTimePicker) } TimeCard(time = editing.time, onClick = { pickingTime = true }) Spacer(Modifier.height(16.dp)) @@ -225,8 +237,13 @@ private fun AlarmEditorContent( onConfirm = { hour, minute -> viewModel.onTimePicked(hour, minute) pickingTime = false + pickingNewAlarm = false + }, + onDismiss = { + if (pickingNewAlarm) viewModel.onNewAlarmTimeKept() + pickingTime = false + pickingNewAlarm = false }, - onDismiss = { pickingTime = false }, ) } when (openPicker) { diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt index be4879f..cffc860 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt @@ -14,10 +14,14 @@ import de.jeanlucmakiola.clockula.domain.ClockDefaults import de.jeanlucmakiola.clockula.domain.DismissChallenge import de.jeanlucmakiola.clockula.domain.Ringtones import de.jeanlucmakiola.clockula.domain.TimeOfDay +import de.jeanlucmakiola.clockula.domain.time.WallClock import de.jeanlucmakiola.clockula.ui.common.RingtonePicker import de.jeanlucmakiola.clockula.ui.common.RingtonePickerUiState import de.jeanlucmakiola.floret.di.ApplicationScope import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow @@ -26,6 +30,8 @@ import kotlinx.coroutines.flow.combine import kotlinx.coroutines.launch import java.time.DayOfWeek import javax.inject.Inject +import kotlin.time.Duration +import kotlin.time.Duration.Companion.seconds /** * The editor writes as the user goes: no Save, no dirty state, no discard @@ -41,6 +47,7 @@ class AlarmEditorViewModel @Inject internal constructor( private val engine: AlarmEngine, private val ringtones: RingtoneCatalog, private val preview: RingtonePreviewer, + wall: WallClock, // `@param:` — the qualifier has no explicit @Target, and K2 resolves a bare // annotation on a constructor `val` to the property instead. @param:ApplicationScope private val appScope: CoroutineScope, @@ -74,6 +81,18 @@ class AlarmEditorViewModel @Inject internal constructor( /** What the previewer is sounding, so the next preview knows to stop it. */ private var previewing: String? = null + private val announcer = AlarmSetAnnouncer(engine, wall) + + /** The announcement waiting out [ANNOUNCE_SETTLE], so the next change can replace it. */ + private var announcing: Job? = null + + /** + * One-shot: "Alarm set for … from now" after a change that moves when the + * alarm rings (#19). Nothing for a switched-off alarm, and nothing for the + * label or the sound: those move no fire time. + */ + val snackbars: Flow = announcer.snackbars + /** Memoised catalog answers, keyed on URI — see [metaOf]. */ private val ringtoneMeta = mutableMapOf() @@ -99,9 +118,17 @@ class AlarmEditorViewModel @Inject internal constructor( /** Clamped, never throws. */ fun onTimePicked(hour: Int, minute: Int) = - edit(reschedule = true) { it.copy(time = TimeOfDay.clamped(hour, minute)) } + edit(reschedule = true, announce = true) { it.copy(time = TimeOfDay.clamped(hour, minute)) } - fun onDayToggled(day: DayOfWeek) = edit(reschedule = true) { alarm -> + /** + * The picker a new alarm opens on was closed without a time. The alarm + * exists and is enabled at its default hour all the same — this is the + * moment it was set, so it is announced (#19). + */ + fun onNewAlarmTimeKept() = announce() + + /** Announced once the taps settle: setting Mon–Fri is one snackbar, not five (#19). */ + fun onDayToggled(day: DayOfWeek) = edit(reschedule = true, announce = true, settle = ANNOUNCE_SETTLE) { alarm -> val days = if (day in alarm.repeatDays) alarm.repeatDays - day else alarm.repeatDays + day alarm.copy( repeatDays = days, @@ -124,6 +151,9 @@ class AlarmEditorViewModel @Inject internal constructor( viewModelScope.launch { alarms.setSkipNextOccurrence(alarmId, skip) engine.reschedule() + // Skipping moves the next ring a day or more out, which is exactly + // the kind of jump worth saying out loud. + announce() } } @@ -235,12 +265,31 @@ class AlarmEditorViewModel @Inject internal constructor( /** * Every editor write. [reschedule] is on only for a change that can move a * fire time — so fiddling with sound settings costs no AlarmManager traffic - * (M5 D12). + * (M5 D12). [announce] is on for the same changes, after [settle]. */ - private fun edit(reschedule: Boolean = false, transform: (Alarm) -> Alarm) { + private fun edit( + reschedule: Boolean = false, + announce: Boolean = false, + settle: Duration = Duration.ZERO, + transform: (Alarm) -> Alarm, + ) { viewModelScope.launch { alarms.edit(alarmId, transform) if (reschedule) engine.reschedule() + if (announce) announce(settle) + } + } + + /** + * Replaces any announcement still settling, so a burst of changes ends in + * one snackbar about where they landed. Read from storage when it fires, + * not from the change that asked, so it is never about a superseded value. + */ + private fun announce(settle: Duration = Duration.ZERO) { + announcing?.cancel() + announcing = viewModelScope.launch { + delay(settle) + announcer.announce(alarmId) } } @@ -313,3 +362,6 @@ class AlarmEditorViewModel @Inject internal constructor( val accessMayLapse: Boolean, ) } + +/** How long the repeat days must stay untouched before the editor says when the alarm rings. */ +private val ANNOUNCE_SETTLE: Duration = 1.seconds diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt new file mode 100644 index 0000000..d13123a --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt @@ -0,0 +1,111 @@ +package de.jeanlucmakiola.clockula.ui.alarms + +import android.content.res.Resources +import androidx.compose.material3.SnackbarHostState +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.ui.platform.LocalResources +import de.jeanlucmakiola.clockula.R +import de.jeanlucmakiola.clockula.alarm.AlarmEngine +import de.jeanlucmakiola.clockula.domain.alarm.AlarmSetCountdown +import de.jeanlucmakiola.clockula.domain.format.NextFireLabel +import de.jeanlucmakiola.clockula.domain.time.WallClock +import kotlinx.coroutines.channels.BufferOverflow +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.asSharedFlow +import kotlinx.coroutines.flow.collectLatest + +/** A one-shot for the alarm screens' "Alarm set for … from now" snackbar (#19). */ +sealed interface AlarmSetSnackbar { + data class Show(val countdown: NextFireLabel) : AlarmSetSnackbar + + /** Takes back a "set for" that stopped being true: the alarm was switched off. */ + data object Hide : AlarmSetSnackbar +} + +/** + * Says how long until an alarm rings, right after a user action in the app set + * it — the moment an AM/PM or wrong-day slip is cheapest to notice (#19). + * + * Only the list and the editor own one, so an alarm set through the + * `AlarmClock` intents never announces itself: those already get the + * status-bar icon. Not thread-safe; both owners call it from `viewModelScope`. + */ +internal class AlarmSetAnnouncer( + private val engine: AlarmEngine, + private val wall: WallClock, +) { + // Dropped, not buffered, with nobody collecting: a countdown that surfaced + // minutes later, when the screen came back, would be stale — and about an + // action the user has long since moved on from. + private val pending = MutableSharedFlow( + extraBufferCapacity = 1, + onBufferOverflow = BufferOverflow.DROP_OLDEST, + ) + + val snackbars: Flow = pending.asSharedFlow() + + /** The alarm the last [AlarmSetSnackbar.Show] was about, so only it can be taken back. */ + private var announced: Long? = null + + /** Call once the write is stored. Says nothing for an alarm that will not ring. */ + suspend fun announce(alarmId: Long) { + val countdown = AlarmSetCountdown.of(engine.upcomingFor(alarmId), wall.now()) + when { + countdown != null -> { + announced = alarmId + pending.tryEmit(AlarmSetSnackbar.Show(countdown)) + } + announced == alarmId -> { + announced = null + pending.tryEmit(AlarmSetSnackbar.Hide) + } + } + } +} + +/** + * Shows [snackbars] on [host]. Latest wins: a new countdown replaces the one on + * screen rather than queueing behind it, so a burst of changes is one snackbar + * that ends up saying the right thing. + */ +@Composable +internal fun AlarmSetSnackbarEffect(snackbars: Flow, host: SnackbarHostState) { + val resources = LocalResources.current + LaunchedEffect(snackbars, host, resources) { + snackbars.collectLatest { snackbar -> + when (snackbar) { + is AlarmSetSnackbar.Show -> + alarmSetMessage(resources, snackbar.countdown)?.let { host.showSnackbar(it) } + // The previous Show was cancelled by collectLatest, which already + // dismissed it; this covers one shown before a recomposition. + AlarmSetSnackbar.Hide -> host.currentSnackbarData?.dismiss() + } + } + } +} + +/** + * The full sentence: proper plurals, and the unit pairs the row's countdown + * uses — minutes, hours and minutes, or days and hours. A zero second unit is + * left out, as the row leaves it out: "7 hours", not "7 hours and 0 minutes". + */ +internal fun alarmSetMessage(resources: Resources, countdown: NextFireLabel): String? { + fun days(n: Int) = resources.getQuantityString(R.plurals.alarm_set_days, n, n) + fun hours(n: Int) = resources.getQuantityString(R.plurals.alarm_set_hours, n, n) + fun minutes(n: Int) = resources.getQuantityString(R.plurals.alarm_set_minutes, n, n) + fun pair(first: String, second: String?) = + if (second == null) first else resources.getString(R.string.alarm_set_units_pair, first, second) + + val span = when (countdown) { + NextFireLabel.NotScheduled -> return null + NextFireLabel.Imminent -> return resources.getString(R.string.alarm_set_imminent) + is NextFireLabel.InMinutes -> minutes(countdown.minutes) + is NextFireLabel.InHoursMinutes -> + pair(hours(countdown.hours), countdown.minutes.takeIf { it > 0 }?.let(::minutes)) + is NextFireLabel.InDaysHours -> + pair(days(countdown.days), countdown.hours.takeIf { it > 0 }?.let(::hours)) + } + return resources.getString(R.string.alarm_set_for, span) +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt index bdb6c1d..d3ca118 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt @@ -15,10 +15,13 @@ import androidx.compose.material3.FloatingActionButton import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource @@ -59,12 +62,14 @@ fun AlarmsScreen( val context = LocalContext.current val pillInset = LocalLivePillInset.current val addAlarm = stringResource(R.string.alarms_add) + val snackbarHost = remember { SnackbarHostState() } LaunchedEffect(viewModel) { // The FAB creates the alarm and the editor edits a persisted row, so // the id to navigate to arrives from the ViewModel (M5 D2, D22). viewModel.openEditor.collectLatest(onCreateAlarm) } + AlarmSetSnackbarEffect(viewModel.snackbars, snackbarHost) CollapsingScaffold( title = stringResource(R.string.tab_alarms), @@ -78,6 +83,8 @@ fun AlarmsScreen( ) } }, + // The scaffold stacks it above the FAB, which already clears the pill. + snackbarHost = { SnackbarHost(snackbarHost) }, floatingActionButton = { FloatingActionButton( onClick = viewModel::onAddAlarm, diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt index d17e9fe..3152279 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt @@ -44,6 +44,11 @@ class AlarmsViewModel @Inject internal constructor( // into two navigations (M5 D22). private val newAlarms = Channel(Channel.CONFLATED) + private val announcer = AlarmSetAnnouncer(engine, wall) + + /** One-shot: "Alarm set for … from now" after the row's switch turns an alarm on (#19). */ + val snackbars: Flow = announcer.snackbars + /** One-shot: the id of a freshly created alarm, for the caller to navigate to. */ val openEditor: Flow = newAlarms.receiveAsFlow().onEach { creating = false } @@ -62,7 +67,8 @@ class AlarmsViewModel @Inject internal constructor( val time = AlarmDefaults.nextWholeHour(wall.now(), zones.current()) val id = alarms.create(AlarmDraft(time = time)) // Created enabled, so it schedules at once — which is what tapping - // "+" asked for (M5 D22). + // "+" asked for (M5 D22). Not announced here: the editor opens on + // its time picker, and announces whichever time the user keeps. engine.reschedule() newAlarms.send(id) } @@ -82,6 +88,9 @@ class AlarmsViewModel @Inject internal constructor( if (!enabled) stopRingIfRinging(alarmId) alarms.setEnabled(alarmId, enabled) engine.reschedule() + // Switching on says when it will ring; switching off takes back a + // "set for" still on screen about this alarm (#19). + announcer.announce(alarmId) } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index ad0a66c..c97e509 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -113,6 +113,22 @@ in %1$dd %2$dh Ringing now Snoozed \u00b7 %1$s + + Alarm set for %1$s from now + Alarm set for less than 1 minute from now + %1$s and %2$s + + %1$d day + %1$d days + + + %1$d hour + %1$d hours + + + %1$d minute + %1$d minutes + Skipping next Once Every day diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdownTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdownTest.kt new file mode 100644 index 0000000..411b010 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdownTest.kt @@ -0,0 +1,79 @@ +package de.jeanlucmakiola.clockula.domain.alarm + +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.clockula.domain.format.NextFireLabel +import de.jeanlucmakiola.clockula.testing.T0 +import de.jeanlucmakiola.clockula.testing.alarmAt +import org.junit.jupiter.api.Test +import kotlin.time.Duration.Companion.days +import kotlin.time.Duration.Companion.hours +import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.seconds +import kotlin.time.Instant + +/** + * "Alarm set for … from now" speaks only for an alarm that will ring at a + * natural occurrence, and rounds the way the row's countdown does (#19). + */ +class AlarmSetCountdownTest { + + private fun upcoming( + nextFire: Instant?, + source: FireSource = FireSource.OCCURRENCE, + enabled: Boolean = true, + ) = UpcomingAlarm(alarmAt(hour = 7, enabled = enabled), nextFire, source) + + @Test + fun `an alarm due in hours and minutes says both`() { + val countdown = AlarmSetCountdown.of(upcoming(T0 + 7.hours + 12.minutes), now = T0) + + assertThat(countdown).isEqualTo(NextFireLabel.InHoursMinutes(7, 12)) + } + + @Test + fun `under a minute away is imminent`() { + val countdown = AlarmSetCountdown.of(upcoming(T0 + 40.seconds), now = T0) + + assertThat(countdown).isEqualTo(NextFireLabel.Imminent) + } + + @Test + fun `minutes away rounds up, as the row does`() { + val countdown = AlarmSetCountdown.of(upcoming(T0 + 4.minutes + 1.seconds), now = T0) + + assertThat(countdown).isEqualTo(NextFireLabel.InMinutes(5)) + } + + @Test + fun `more than a day away says days and hours`() { + val countdown = AlarmSetCountdown.of(upcoming(T0 + 2.days + 3.hours), now = T0) + + assertThat(countdown).isEqualTo(NextFireLabel.InDaysHours(2, 3)) + } + + @Test + fun `a switched-off alarm says nothing`() { + val countdown = AlarmSetCountdown.of(upcoming(nextFire = null, FireSource.NONE, enabled = false), now = T0) + + assertThat(countdown).isNull() + } + + @Test + fun `a snoozed alarm says nothing about its snooze`() { + val countdown = AlarmSetCountdown.of(upcoming(T0 + 9.minutes, FireSource.SNOOZE), now = T0) + + assertThat(countdown).isNull() + } + + @Test + fun `a ringing alarm says nothing`() { + val countdown = AlarmSetCountdown.of(upcoming(nextFire = null, FireSource.RESUMED_RING), now = T0) + + assertThat(countdown).isNull() + } + + @Test + fun `an unknown alarm says nothing`() { + assertThat(AlarmSetCountdown.of(upcoming = null, now = T0)).isNull() + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt index 4e9c0b9..4759e80 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt @@ -1,12 +1,14 @@ package de.jeanlucmakiola.clockula.ui.alarms import androidx.lifecycle.SavedStateHandle +import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import de.jeanlucmakiola.clockula.data.alarms.AlarmMapper import de.jeanlucmakiola.clockula.domain.DismissChallenge import de.jeanlucmakiola.clockula.domain.RepeatDays import de.jeanlucmakiola.clockula.domain.Ringtones import de.jeanlucmakiola.clockula.domain.TimeOfDay +import de.jeanlucmakiola.clockula.domain.format.NextFireLabel import de.jeanlucmakiola.clockula.testing.AlarmEngineHarness import de.jeanlucmakiola.clockula.testing.BERLIN import de.jeanlucmakiola.clockula.testing.FakeRingtoneCatalog @@ -23,6 +25,7 @@ import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.setMain @@ -59,6 +62,7 @@ class AlarmEditorViewModelTest { engine = harness.engine, ringtones = catalog, preview = previewer, + wall = harness.wallClock, appScope = this, ) @@ -252,6 +256,132 @@ class AlarmEditorViewModelTest { assertThat(viewModel.editing().canSkipNext).isTrue() } + // --- "Alarm set for … from now" (#19) --- + + @Test + fun `picking a time says how long until it rings`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + val viewModel = editor(harness) + advanceUntilIdle() + + viewModel.snackbars.test { + viewModel.onTimePicked(6, 45) + advanceUntilIdle() + + assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(1, 45))) + } + } + + @Test + fun `a time picked earlier than now says it rings tomorrow`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + val viewModel = editor(harness) + advanceUntilIdle() + + viewModel.snackbars.test { + // The AM/PM slip the snackbar is there to catch: 4:30 is behind + // us, so it rings tomorrow, 23½ hours away. + viewModel.onTimePicked(4, 30) + advanceUntilIdle() + + assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(23, 30))) + } + } + + @Test + fun `keeping a new alarm's default time still says when it rings`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 6) + val viewModel = editor(harness) + advanceUntilIdle() + + viewModel.snackbars.test { + viewModel.onNewAlarmTimeKept() + advanceUntilIdle() + + assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(1, 0))) + } + } + + @Test + fun `a burst of repeat-day taps is announced once, after it settles`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + val viewModel = editor(harness) + advanceUntilIdle() + + viewModel.snackbars.test { + viewModel.onDayToggled(DayOfWeek.MONDAY) + advanceTimeBy(500) + viewModel.onDayToggled(DayOfWeek.TUESDAY) + advanceTimeBy(500) + viewModel.onDayToggled(DayOfWeek.WEDNESDAY) + advanceTimeBy(500) + expectNoEvents() + advanceUntilIdle() + + // 2026-06-10 is a Wednesday: 7:00 today, two hours out. + assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(2, 0))) + expectNoEvents() + } + } + + @Test + fun `arming a skip says it rings at the occurrence after it`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7, repeatDays = RepeatDays.EVERY_DAY) + harness.engine.reschedule() + val viewModel = editor(harness) + advanceUntilIdle() + + viewModel.snackbars.test { + viewModel.onSkipNextChanged(true) + advanceUntilIdle() + + assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InDaysHours(1, 2))) + } + } + + @Test + fun `a switched-off alarm's new time says nothing`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarm(id = 1L, hour = 7, enabled = false) + val viewModel = editor(harness) + advanceUntilIdle() + + viewModel.snackbars.test { + viewModel.onTimePicked(6, 45) + advanceUntilIdle() + + expectNoEvents() + } + } + + @Test + fun `a change that moves no fire time says nothing`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + val viewModel = editor(harness) + advanceUntilIdle() + + viewModel.snackbars.test { + viewModel.onLabelTyped("Gym") + viewModel.onSnoozeMinutesChanged(5) + advanceUntilIdle() + + expectNoEvents() + } + } + // --- label --- @Test diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt index 1317b97..4fc6f00 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt @@ -190,6 +190,72 @@ class AlarmsViewModelTest { assertThat(harness.scheduler.next?.fireAt).isEqualTo(sevenToday) } + @Test + fun `switching an alarm on says how long until it rings`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 7, minute = 12, enabled = false) + val viewModel = viewModel(harness) + advanceUntilIdle() + + viewModel.snackbars.test { + viewModel.onToggleEnabled(1L, enabled = true) + advanceUntilIdle() + + assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(2, 12))) + } + } + + @Test + fun `switching the announced alarm back off takes the announcement back`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 7, enabled = false) + val viewModel = viewModel(harness) + advanceUntilIdle() + + viewModel.snackbars.test { + viewModel.onToggleEnabled(1L, enabled = true) + advanceUntilIdle() + skipItems(1) + viewModel.onToggleEnabled(1L, enabled = false) + advanceUntilIdle() + + assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Hide) + } + } + + @Test + fun `switching off an alarm nobody was told about says nothing`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 7) + val viewModel = viewModel(harness) + advanceUntilIdle() + + viewModel.snackbars.test { + viewModel.onToggleEnabled(1L, enabled = false) + advanceUntilIdle() + + expectNoEvents() + } + } + + @Test + fun `adding an alarm leaves the announcement to the editor`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir) + val viewModel = viewModel(harness) + advanceUntilIdle() + + viewModel.snackbars.test { + viewModel.onAddAlarm() + advanceUntilIdle() + + expectNoEvents() + } + } + @Test fun `switching off the ringing alarm silences it first`(@TempDir tempDir: Path) = runTest(main.dispatcher) { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 97ef4e8..261deb5 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -86,7 +86,7 @@ kit module: the kit's roadmap keeps schedulers app-local. | Package | Holds | |---|---| | `domain/` | `Alarm.kt`, `Timer.kt`, `WorldClock.kt`, `Stopwatch.kt`, `ClockDefaults.kt`, `Ringtone.kt` (the silent sentinel), `RepeatSummary.kt`, `AlarmDefaults.kt`. Plain Kotlin, no Android | -| `domain/alarm/` | the alarm engine's pure half: occurrences, the resolver, the ring state, the presentation and scheduling policies, the challenge gate. The *shared* ring vocabulary moved out to `domain/ring/` in M6 | +| `domain/alarm/` | the alarm engine's pure half: occurrences, the resolver, the ring state, the presentation and scheduling policies, the challenge gate, and `AlarmSetCountdown`, what the "Alarm set for … from now" snackbar says (#19). The *shared* ring vocabulary moved out to `domain/ring/` in M6 | | `domain/ring/` | what both ring paths share: `RingAudio` (the ramp tick, the volume floor), `VolumeRamp`, `AudioSourcePolicy`/`RingtoneSourceKind`, `RingFallbackPolicy`, `VibrationPattern` | | `domain/stopwatch/` | the stopwatch path's pure half: `StopwatchReadings` (the one reading the pill, the shade and the tab share), `LapStats` (best and worst), `StopwatchLaps` (the lap cap), `StopwatchNotificationPolicy` | | `domain/timer/` | the timer path's pure half: `TimerReadings` (the app's one timer precedence), `TimerExpiry` (the sweep and the slot's value), `TimerRingPolicy`, `TimerNotificationPolicy`, `TimerPresets`, `TimerDurationEntry`, `TimerSettings`, `TimerRing` | @@ -133,7 +133,7 @@ kit module: the kit's roadmap keeps schedulers app-local. | `ui/theme/`, `ui/crash/` | the M0/M1 theme and the crash-report surface | | `ui/shell/` | the navigation policy (`ShellNavigation`), the adaptive shell, the live pill and its source/ViewModel, the notification-permission ask. `EmptyTabScreen` is gone: its KDoc said it existed only until each of M5–M8 replaced its own tab, and the world clock was its last caller | | `ui/common/` | `rememberLocalTimeFormatter`, the app's one 12/24-hour formatter, over a `java.time.LocalTime`, with `rememberAlarmTimeFormatter` delegating to it (M8 D17); and, since M6, the ringtone picker (`RingtonePickerState`, `RingtonePickerScreen`), which both editors need | -| `ui/alarms/` | the real Alarms tab (M5): the routes, the list's pure row builder and its source, the two ViewModels, the list, the editor, the M3 time-picker host, the repeat-day selector and the override pickers (the ringtone picker moved to `ui/common/` in M6), and M9's `DismissChooser`/`DismissAlarmDialog`, the "which alarm should I dismiss?" question | +| `ui/alarms/` | the real Alarms tab (M5): the routes, the list's pure row builder and its source, the two ViewModels, the list, the editor, the M3 time-picker host, the repeat-day selector and the override pickers (the ringtone picker moved to `ui/common/` in M6), and M9's `DismissChooser`/`DismissAlarmDialog`, the "which alarm should I dismiss?" question; and `AlarmSetAnnouncer`/`AlarmSetSnackbar`, the "Alarm set for … from now" snackbar both ViewModels raise after a user action (#19) | | `ui/timers/` | the real Timers tab (M6): the routes, the pure row builder and its source, the two ViewModels, the list, the row, the setup panel and its keypad, the editor | | `ui/stopwatch/` | the real Stopwatch tab (M7): the pure row builder (`StopwatchRows`) and its source, the ViewModel, the readout panel with its controls, the lap table and its column header, `StopwatchDefaults` | | `ui/worldclock/` | the real World clock tab (M8): the pure row builders (`WorldClockRows`, `ZonePickerRows`) and their state, `WorldClockSource`, the ViewModel, the tab, the analog face, the row and the zone picker, `WorldClockDefaults` | @@ -1116,6 +1116,14 @@ its own writes. `reschedule()` is called explicitly: from `ClockulaApp`, from the receivers, and from the alarm editor. `upcoming()` runs the same resolver in preview mode and *discards* the state it returns; a read must not write, and a test asserts the store recorded nothing across a full collection. +`upcomingFor(alarmId)` is the same read for one alarm, on demand: the list and +the editor ask it right after a user's change to say how long until that alarm +rings. `AlarmSetCountdown` (pure, in `domain/alarm/`) decides what that says — +only a natural occurrence of an enabled alarm, rounded as the row's countdown +is, never a snooze or a ring. The `AlarmClock` intents never reach it, since +they already get the status-bar icon (#19). The editor announces a time pick +at once, but waits for repeat-day taps to settle for a second, so Mon–Fri is +one snackbar rather than five. `upcoming(refresh: Flow)` takes its **re-read cadence as a parameter**. The resolver needs a `now`, and `upcoming` reads one inside a `combine` of the From 207475ef3783ee18cdc3e6e3337dc3d314634943 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 19:00:56 +0200 Subject: [PATCH 2/9] fix(alarms): keep the alarm-set notice until a screen has shown it The usual way to set an alarm is pick, OK, back, and back disposed the editor's snackbar a moment after it appeared. Day taps left within the one-second settle were never announced at all, and a rotation threw a showing snackbar away for good. The announcer is now an app-wide singleton holding the notice as state until a snackbar has run to the end, so the list picks up what the editor could not finish showing. An editor closed mid-settle announces on the way out. Announcing is serialised, so a quick on-then-off cannot land the "on" read last, and a notice nobody showed within ten seconds is dropped rather than shown stale. The engine's one-alarm read now shares upcoming()'s preview helper. Refs #19 --- .../clockula/alarm/AlarmEngine.kt | 23 ++-- .../clockula/ui/alarms/AlarmEditorScreen.kt | 2 +- .../ui/alarms/AlarmEditorViewModel.kt | 23 ++-- .../clockula/ui/alarms/AlarmSetSnackbar.kt | 129 +++++++++++------- .../clockula/ui/alarms/AlarmsScreen.kt | 2 +- .../clockula/ui/alarms/AlarmsViewModel.kt | 10 +- .../ui/alarms/AlarmEditorViewModelTest.kt | 119 +++++++++++----- .../clockula/ui/alarms/AlarmsViewModelTest.kt | 46 +++---- docs/ARCHITECTURE.md | 6 +- 9 files changed, 225 insertions(+), 135 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt index d567025..320512d 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -29,6 +29,7 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock +import java.time.ZoneId import kotlin.time.Duration.Companion.minutes import kotlin.time.Instant import javax.inject.Inject @@ -187,17 +188,7 @@ class AlarmEngine @Inject constructor( val zone = zones.current() val byId = states.associateBy { it.alarmId } alarms - .map { alarm -> - // The resolver's state is deliberately discarded: a read must - // not write, however much it resolved on the way past. - val resolution = AlarmResolver.resolve( - alarm, - byId[alarm.id] ?: AlarmRingState.initial(alarm.id), - now, - zone, - ) - UpcomingAlarm(alarm, resolution.nextFire, resolution.source) - } + .map { alarm -> preview(alarm, byId[alarm.id] ?: AlarmRingState.initial(alarm.id), now, zone) } .sortedWith( compareBy(nullsLast()) { it.nextFire } .thenBy { it.alarm.id }, @@ -211,7 +202,15 @@ class AlarmEngine @Inject constructor( */ suspend fun upcomingFor(alarmId: Long): UpcomingAlarm? { val alarm = alarms.find(alarmId) ?: return null - val resolution = AlarmResolver.resolve(alarm, states.state(alarmId), wallClock.now(), zones.current()) + return preview(alarm, states.state(alarmId), wallClock.now(), zones.current()) + } + + /** + * The resolver in preview mode. Its state is deliberately discarded: a read + * must not write, however much it resolved on the way past. + */ + private fun preview(alarm: Alarm, state: AlarmRingState, now: Instant, zone: ZoneId): UpcomingAlarm { + val resolution = AlarmResolver.resolve(alarm, state, now, zone) return UpcomingAlarm(alarm, resolution.nextFire, resolution.source) } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt index 44787a6..088a42a 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt @@ -63,7 +63,7 @@ fun AlarmEditorScreen( var confirmingDelete by rememberSaveable { mutableStateOf(false) } val formatTime = rememberAlarmTimeFormatter() val snackbarHost = remember { SnackbarHostState() } - AlarmSetSnackbarEffect(viewModel.snackbars, snackbarHost) + AlarmSetSnackbarEffect(viewModel.alarmSetNotice, snackbarHost, viewModel::onAlarmSetNoticeShown) // Registered unconditionally, and deliberately **not** inside the // `picker?.let` below: the picker's open flag is ViewModel state that does diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt index cffc860..f8c2ba1 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt @@ -14,7 +14,6 @@ import de.jeanlucmakiola.clockula.domain.ClockDefaults import de.jeanlucmakiola.clockula.domain.DismissChallenge import de.jeanlucmakiola.clockula.domain.Ringtones import de.jeanlucmakiola.clockula.domain.TimeOfDay -import de.jeanlucmakiola.clockula.domain.time.WallClock import de.jeanlucmakiola.clockula.ui.common.RingtonePicker import de.jeanlucmakiola.clockula.ui.common.RingtonePickerUiState import de.jeanlucmakiola.floret.di.ApplicationScope @@ -47,7 +46,7 @@ class AlarmEditorViewModel @Inject internal constructor( private val engine: AlarmEngine, private val ringtones: RingtoneCatalog, private val preview: RingtonePreviewer, - wall: WallClock, + private val announcer: AlarmSetAnnouncer, // `@param:` — the qualifier has no explicit @Target, and K2 resolves a bare // annotation on a constructor `val` to the property instead. @param:ApplicationScope private val appScope: CoroutineScope, @@ -81,17 +80,20 @@ class AlarmEditorViewModel @Inject internal constructor( /** What the previewer is sounding, so the next preview knows to stop it. */ private var previewing: String? = null - private val announcer = AlarmSetAnnouncer(engine, wall) - /** The announcement waiting out [ANNOUNCE_SETTLE], so the next change can replace it. */ private var announcing: Job? = null + /** True while [announcing] has not yet announced — what [onCleared] flushes. */ + private var announcementPending = false + /** - * One-shot: "Alarm set for … from now" after a change that moves when the - * alarm rings (#19). Nothing for a switched-off alarm, and nothing for the - * label or the sound: those move no fire time. + * "Alarm set for … from now" after a change that moves when the alarm rings + * (#19). Nothing for a switched-off alarm, and nothing for the label or the + * sound: those move no fire time. */ - val snackbars: Flow = announcer.snackbars + val alarmSetNotice: Flow = announcer.notice + + fun onAlarmSetNoticeShown(notice: AlarmSetNotice) = announcer.consumed(notice) /** Memoised catalog answers, keyed on URI — see [metaOf]. */ private val ringtoneMeta = mutableMapOf() @@ -254,6 +256,9 @@ class AlarmEditorViewModel @Inject internal constructor( labelWrites.value?.let { text -> appScope.launch { alarms.edit(alarmId) { it.copy(label = text) } } } + // Back within a second of the last day tap: the announcement still + // settling is made now, for the list to show (#19). + if (announcementPending) appScope.launch { announcer.announce(alarmId) } super.onCleared() } @@ -287,8 +292,10 @@ class AlarmEditorViewModel @Inject internal constructor( */ private fun announce(settle: Duration = Duration.ZERO) { announcing?.cancel() + announcementPending = true announcing = viewModelScope.launch { delay(settle) + announcementPending = false announcer.announce(alarmId) } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt index d13123a..cc26029 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt @@ -4,84 +4,119 @@ import android.content.res.Resources import androidx.compose.material3.SnackbarHostState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.ui.platform.LocalResources import de.jeanlucmakiola.clockula.R import de.jeanlucmakiola.clockula.alarm.AlarmEngine import de.jeanlucmakiola.clockula.domain.alarm.AlarmSetCountdown import de.jeanlucmakiola.clockula.domain.format.NextFireLabel import de.jeanlucmakiola.clockula.domain.time.WallClock -import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.asSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.collectLatest +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import javax.inject.Inject +import javax.inject.Singleton +import kotlin.time.Duration.Companion.seconds +import kotlin.time.Instant -/** A one-shot for the alarm screens' "Alarm set for … from now" snackbar (#19). */ -sealed interface AlarmSetSnackbar { - data class Show(val countdown: NextFireLabel) : AlarmSetSnackbar - - /** Takes back a "set for" that stopped being true: the alarm was switched off. */ - data object Hide : AlarmSetSnackbar -} +/** "Alarm set for … from now", about one alarm, until a screen has shown it (#19). */ +data class AlarmSetNotice( + val alarmId: Long, + val countdown: NextFireLabel, + val announcedAt: Instant, +) /** * Says how long until an alarm rings, right after a user action in the app set * it — the moment an AM/PM or wrong-day slip is cheapest to notice (#19). * - * Only the list and the editor own one, so an alarm set through the + * Only the list and the editor call it, so an alarm set through the * `AlarmClock` intents never announces itself: those already get the - * status-bar icon. Not thread-safe; both owners call it from `viewModelScope`. + * status-bar icon. + * + * A **state**, held app-wide, not an event: the notice stays until a screen + * has shown it to the end ([consumed]). The usual way to set an alarm is + * pick → OK → back, and back disposes the editor's snackbar a moment after it + * appeared; a one-shot event would be gone with it, while this one is picked + * up again by the list underneath. A rotation mid-snackbar re-shows it the + * same way. */ -internal class AlarmSetAnnouncer( +@Singleton +internal class AlarmSetAnnouncer @Inject constructor( private val engine: AlarmEngine, private val wall: WallClock, ) { - // Dropped, not buffered, with nobody collecting: a countdown that surfaced - // minutes later, when the screen came back, would be stale — and about an - // action the user has long since moved on from. - private val pending = MutableSharedFlow( - extraBufferCapacity = 1, - onBufferOverflow = BufferOverflow.DROP_OLDEST, - ) + private val current = MutableStateFlow(null) - val snackbars: Flow = pending.asSharedFlow() + /** + * Serialises [announce]: it reads storage and suspends before it writes + * [current], and two switch taps in quick succession must not let the + * earlier read land last — "set for 9 hours" about an alarm that is off. + */ + private val lock = Mutex() - /** The alarm the last [AlarmSetSnackbar.Show] was about, so only it can be taken back. */ - private var announced: Long? = null + /** + * The notice to show, or null. Past [FRESH_FOR] it is dropped unshown: a + * countdown that surfaced when the user came back to the app an hour + * later would be wrong, and about something they did long ago. + */ + val notice: Flow = current.map { notice -> + notice?.takeIf { wall.now() - it.announcedAt < FRESH_FOR } + } - /** Call once the write is stored. Says nothing for an alarm that will not ring. */ - suspend fun announce(alarmId: Long) { - val countdown = AlarmSetCountdown.of(engine.upcomingFor(alarmId), wall.now()) - when { - countdown != null -> { - announced = alarmId - pending.tryEmit(AlarmSetSnackbar.Show(countdown)) - } - announced == alarmId -> { - announced = null - pending.tryEmit(AlarmSetSnackbar.Hide) - } + /** + * Call once the write is stored. Reads the alarm as it is now, so it is never + * about a superseded value. An alarm that will not ring takes back a notice + * still pending about it, and otherwise says nothing. + */ + suspend fun announce(alarmId: Long): Unit = lock.withLock { + val now = wall.now() + val countdown = AlarmSetCountdown.of(engine.upcomingFor(alarmId), now) + if (countdown != null) { + current.value = AlarmSetNotice(alarmId, countdown, announcedAt = now) + } else if (current.value?.alarmId == alarmId) { + current.value = null } } + + /** [notice] was shown until it timed out or was swiped away. */ + fun consumed(notice: AlarmSetNotice) { + current.compareAndSet(notice, null) + } + + private companion object { + /** Comfortably longer than a short snackbar, so a hand-over between screens fits. */ + val FRESH_FOR = 10.seconds + } } /** - * Shows [snackbars] on [host]. Latest wins: a new countdown replaces the one on + * Shows [notice] on [host]. Latest wins: a new countdown replaces the one on * screen rather than queueing behind it, so a burst of changes is one snackbar - * that ends up saying the right thing. + * that ends up saying the right thing, and a null takes it down. + * + * [onShown] runs only when the snackbar ended on its own or was swiped away. + * One cut short — by leaving the screen, or a rotation — stays pending for the + * next screen to show. */ @Composable -internal fun AlarmSetSnackbarEffect(snackbars: Flow, host: SnackbarHostState) { +internal fun AlarmSetSnackbarEffect( + notice: Flow, + host: SnackbarHostState, + onShown: (AlarmSetNotice) -> Unit, +) { val resources = LocalResources.current - LaunchedEffect(snackbars, host, resources) { - snackbars.collectLatest { snackbar -> - when (snackbar) { - is AlarmSetSnackbar.Show -> - alarmSetMessage(resources, snackbar.countdown)?.let { host.showSnackbar(it) } - // The previous Show was cancelled by collectLatest, which already - // dismissed it; this covers one shown before a recomposition. - AlarmSetSnackbar.Hide -> host.currentSnackbarData?.dismiss() - } + val shown by rememberUpdatedState(onShown) + LaunchedEffect(notice, host, resources) { + notice.collectLatest { current -> + if (current == null) return@collectLatest + val message = alarmSetMessage(resources, current.countdown) + if (message != null) host.showSnackbar(message) + shown(current) } } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt index d3ca118..519bb2f 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt @@ -69,7 +69,7 @@ fun AlarmsScreen( // the id to navigate to arrives from the ViewModel (M5 D2, D22). viewModel.openEditor.collectLatest(onCreateAlarm) } - AlarmSetSnackbarEffect(viewModel.snackbars, snackbarHost) + AlarmSetSnackbarEffect(viewModel.alarmSetNotice, snackbarHost, viewModel::onAlarmSetNoticeShown) CollapsingScaffold( title = stringResource(R.string.tab_alarms), diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt index 3152279..cd16481 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt @@ -28,6 +28,7 @@ class AlarmsViewModel @Inject internal constructor( private val wall: WallClock, private val zones: ZoneProvider, private val source: AlarmListSource, + private val announcer: AlarmSetAnnouncer, ) : ViewModel() { val state: StateFlow = source.rows() @@ -44,10 +45,13 @@ class AlarmsViewModel @Inject internal constructor( // into two navigations (M5 D22). private val newAlarms = Channel(Channel.CONFLATED) - private val announcer = AlarmSetAnnouncer(engine, wall) + /** + * "Alarm set for … from now", after the row's switch turns an alarm on — or + * handed over by the editor the user has just left (#19). + */ + val alarmSetNotice: Flow = announcer.notice - /** One-shot: "Alarm set for … from now" after the row's switch turns an alarm on (#19). */ - val snackbars: Flow = announcer.snackbars + fun onAlarmSetNoticeShown(notice: AlarmSetNotice) = announcer.consumed(notice) /** One-shot: the id of a freshly created alarm, for the caller to navigate to. */ val openEditor: Flow = newAlarms.receiveAsFlow().onEach { creating = false } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt index 4759e80..02ad01b 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt @@ -23,6 +23,7 @@ import de.jeanlucmakiola.clockula.testing.instantAt import de.jeanlucmakiola.clockula.testing.offsetInstant import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceTimeBy @@ -34,6 +35,7 @@ import org.junit.jupiter.api.extension.RegisterExtension import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import java.time.DayOfWeek +import kotlin.time.Duration.Companion.hours /** * The editor over the **real** alarm engine: every control writes at once @@ -54,7 +56,11 @@ class AlarmEditorViewModelTest { private val sevenToday = offsetInstant("2026-06-10T07:00+02:00") private val sevenTomorrow = offsetInstant("2026-06-11T07:00+02:00") - private fun TestScope.editor(harness: AlarmEngineHarness, alarmId: Long = 1L) = + private fun TestScope.editor( + harness: AlarmEngineHarness, + alarmId: Long = 1L, + announcer: AlarmSetAnnouncer = AlarmSetAnnouncer(harness.engine, harness.wallClock), + ) = AlarmEditorViewModel( savedStateHandle = SavedStateHandle(mapOf(AlarmRoutes.ARG_ALARM_ID to alarmId)), alarms = harness.alarms, @@ -62,7 +68,7 @@ class AlarmEditorViewModelTest { engine = harness.engine, ringtones = catalog, preview = previewer, - wall = harness.wallClock, + announcer = announcer, appScope = this, ) @@ -258,6 +264,8 @@ class AlarmEditorViewModelTest { // --- "Alarm set for … from now" (#19) --- + private suspend fun AlarmEditorViewModel.countdown() = alarmSetNotice.first()?.countdown + @Test fun `picking a time says how long until it rings`(@TempDir tempDir: Path) = runTest(main.dispatcher) { @@ -266,12 +274,10 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onTimePicked(6, 45) - advanceUntilIdle() + viewModel.onTimePicked(6, 45) + advanceUntilIdle() - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(1, 45))) - } + assertThat(viewModel.countdown()).isEqualTo(NextFireLabel.InHoursMinutes(1, 45)) } @Test @@ -282,14 +288,12 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { - // The AM/PM slip the snackbar is there to catch: 4:30 is behind - // us, so it rings tomorrow, 23½ hours away. - viewModel.onTimePicked(4, 30) - advanceUntilIdle() + // The AM/PM slip the snackbar is there to catch: 4:30 is behind + // us, so it rings tomorrow, 23½ hours away. + viewModel.onTimePicked(4, 30) + advanceUntilIdle() - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(23, 30))) - } + assertThat(viewModel.countdown()).isEqualTo(NextFireLabel.InHoursMinutes(23, 30)) } @Test @@ -300,12 +304,10 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onNewAlarmTimeKept() - advanceUntilIdle() + viewModel.onNewAlarmTimeKept() + advanceUntilIdle() - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(1, 0))) - } + assertThat(viewModel.countdown()).isEqualTo(NextFireLabel.InHoursMinutes(1, 0)) } @Test @@ -316,7 +318,8 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { + viewModel.alarmSetNotice.test { + assertThat(awaitItem()).isNull() viewModel.onDayToggled(DayOfWeek.MONDAY) advanceTimeBy(500) viewModel.onDayToggled(DayOfWeek.TUESDAY) @@ -327,11 +330,29 @@ class AlarmEditorViewModelTest { advanceUntilIdle() // 2026-06-10 is a Wednesday: 7:00 today, two hours out. - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(2, 0))) + assertThat(awaitItem()?.countdown).isEqualTo(NextFireLabel.InHoursMinutes(2, 0)) expectNoEvents() } } + @Test + fun `leaving while the days settle still announces them, for the list`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + val announcer = AlarmSetAnnouncer(harness.engine, harness.wallClock) + val viewModel = editor(harness, announcer = announcer) + advanceUntilIdle() + + viewModel.onDayToggled(DayOfWeek.MONDAY) + advanceTimeBy(200) + viewModel.onCleared() + advanceUntilIdle() + + // Wednesday 05:00 to Monday 07:00. + assertThat(announcer.notice.first()?.countdown).isEqualTo(NextFireLabel.InDaysHours(5, 2)) + } + @Test fun `arming a skip says it rings at the occurrence after it`(@TempDir tempDir: Path) = runTest(main.dispatcher) { @@ -341,12 +362,10 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onSkipNextChanged(true) - advanceUntilIdle() + viewModel.onSkipNextChanged(true) + advanceUntilIdle() - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InDaysHours(1, 2))) - } + assertThat(viewModel.countdown()).isEqualTo(NextFireLabel.InDaysHours(1, 2)) } @Test @@ -357,12 +376,10 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onTimePicked(6, 45) - advanceUntilIdle() + viewModel.onTimePicked(6, 45) + advanceUntilIdle() - expectNoEvents() - } + assertThat(viewModel.alarmSetNotice.first()).isNull() } @Test @@ -373,13 +390,41 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onLabelTyped("Gym") - viewModel.onSnoozeMinutesChanged(5) - advanceUntilIdle() + viewModel.onLabelTyped("Gym") + viewModel.onSnoozeMinutesChanged(5) + advanceUntilIdle() - expectNoEvents() - } + assertThat(viewModel.alarmSetNotice.first()).isNull() + } + + @Test + fun `a notice shown to the end is gone`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + val viewModel = editor(harness) + advanceUntilIdle() + viewModel.onTimePicked(6, 45) + advanceUntilIdle() + + viewModel.onAlarmSetNoticeShown(checkNotNull(viewModel.alarmSetNotice.first())) + + assertThat(viewModel.alarmSetNotice.first()).isNull() + } + + @Test + fun `a notice nobody showed in time is dropped, not shown stale`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + val viewModel = editor(harness) + advanceUntilIdle() + viewModel.onTimePicked(6, 45) + advanceUntilIdle() + + harness.wallClock.instant = fiveToday + 1.hours + + assertThat(viewModel.alarmSetNotice.first()).isNull() } // --- label --- diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt index 4fc6f00..64b0276 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt @@ -13,6 +13,7 @@ import de.jeanlucmakiola.clockula.testing.alarmEngineHarness import de.jeanlucmakiola.clockula.testing.instantAt import de.jeanlucmakiola.clockula.testing.offsetInstant import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope @@ -52,6 +53,7 @@ class AlarmsViewModelTest { wall = harness.wallClock, zones = harness.zones, source = AlarmListSource(harness.engine, harness.wallClock, harness.ticker), + announcer = AlarmSetAnnouncer(harness.engine, harness.wallClock), ).also { viewModel -> backgroundScope.launch { viewModel.state.collect {} } } private fun AlarmsUiState.rows(): List = (this as AlarmsUiState.Loaded).rows @@ -198,12 +200,10 @@ class AlarmsViewModelTest { val viewModel = viewModel(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onToggleEnabled(1L, enabled = true) - advanceUntilIdle() + viewModel.onToggleEnabled(1L, enabled = true) + advanceUntilIdle() - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Show(NextFireLabel.InHoursMinutes(2, 12))) - } + assertThat(viewModel.alarmSetNotice.first()?.countdown).isEqualTo(NextFireLabel.InHoursMinutes(2, 12)) } @Test @@ -214,31 +214,29 @@ class AlarmsViewModelTest { val viewModel = viewModel(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onToggleEnabled(1L, enabled = true) - advanceUntilIdle() - skipItems(1) - viewModel.onToggleEnabled(1L, enabled = false) - advanceUntilIdle() + // Both taps before either has run: the "on" read must not land last. + viewModel.onToggleEnabled(1L, enabled = true) + viewModel.onToggleEnabled(1L, enabled = false) + advanceUntilIdle() - assertThat(awaitItem()).isEqualTo(AlarmSetSnackbar.Hide) - } + assertThat(viewModel.alarmSetNotice.first()).isNull() } @Test - fun `switching off an alarm nobody was told about says nothing`(@TempDir tempDir: Path) = + fun `switching off another alarm leaves the announcement standing`(@TempDir tempDir: Path) = runTest(main.dispatcher) { val harness = alarmEngineHarness(tempDir) - harness.givenAlarm(id = 1L, hour = 7) + harness.givenAlarm(id = 1L, hour = 7, enabled = false) + harness.givenAlarm(id = 2L, hour = 9) val viewModel = viewModel(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onToggleEnabled(1L, enabled = false) - advanceUntilIdle() + viewModel.onToggleEnabled(1L, enabled = true) + advanceUntilIdle() + viewModel.onToggleEnabled(2L, enabled = false) + advanceUntilIdle() - expectNoEvents() - } + assertThat(viewModel.alarmSetNotice.first()?.alarmId).isEqualTo(1L) } @Test @@ -248,12 +246,10 @@ class AlarmsViewModelTest { val viewModel = viewModel(harness) advanceUntilIdle() - viewModel.snackbars.test { - viewModel.onAddAlarm() - advanceUntilIdle() + viewModel.onAddAlarm() + advanceUntilIdle() - expectNoEvents() - } + assertThat(viewModel.alarmSetNotice.first()).isNull() } @Test diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 261deb5..2051aee 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1123,7 +1123,11 @@ only a natural occurrence of an enabled alarm, rounded as the row's countdown is, never a snooze or a ring. The `AlarmClock` intents never reach it, since they already get the status-bar icon (#19). The editor announces a time pick at once, but waits for repeat-day taps to settle for a second, so Mon–Fri is -one snackbar rather than five. +one snackbar rather than five. `AlarmSetAnnouncer` is an app-wide singleton +holding the notice as *state* until a screen has shown it to the end: pick → +OK → back disposes the editor's snackbar a moment after it appears, and the +list underneath picks the same notice up. A notice older than ten seconds is +dropped unshown rather than shown with a stale countdown. `upcoming(refresh: Flow)` takes its **re-read cadence as a parameter**. The resolver needs a `now`, and `upcoming` reads one inside a `combine` of the From fa5ce55503e7b9e79b377334fd670166278d255e Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 19:10:42 +0200 Subject: [PATCH 3/9] fix(alarms): settle and show the alarm-set notice where it can't be lost The repeat-day settle ran on the editor's scope, so leaving mid-settle needed a flush that a cleared scope could still race. The announcer now owns the settle on the application scope. Screens show the notice only while resumed, so a cross-fade never shows it twice and a backgrounded app does not use it up unseen. The editor shows only notices about its own alarm, since the sentence names none. Deleting an alarm, or the AlarmClock chooser dismissing it, takes its notice back. Freshness is measured on the monotonic clock, one instant is used for both ends of the countdown, and an identical countdown keeps the standing notice instead of flickering it. Refs #19 --- .../clockula/alarm/AlarmEngine.kt | 11 +- .../domain/alarm/AlarmSetCountdown.kt | 5 +- .../ui/alarms/AlarmEditorViewModel.kt | 52 +++----- .../clockula/ui/alarms/AlarmSetSnackbar.kt | 125 +++++++++++++----- .../clockula/ui/alarms/AlarmsViewModel.kt | 9 +- .../ui/alarms/AlarmEditorViewModelTest.kt | 72 +++++++++- .../clockula/ui/alarms/AlarmsViewModelTest.kt | 21 ++- docs/ARCHITECTURE.md | 9 +- 8 files changed, 220 insertions(+), 84 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt index 320512d..070b660 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -196,13 +196,14 @@ class AlarmEngine @Inject constructor( } /** - * One alarm's [upcoming] entry, resolved against the wall clock now. Read-only - * like [upcoming], and for the same reason it takes no lock: it writes - * nothing. Null for an id no alarm has. + * One alarm's [upcoming] entry, resolved against [now]. Read-only like + * [upcoming], and for the same reason it takes no lock: it writes nothing. + * Null for an id no alarm has. A caller that measures a countdown passes + * the `now` it measures from, so both ends of it are the same instant. */ - suspend fun upcomingFor(alarmId: Long): UpcomingAlarm? { + suspend fun upcomingFor(alarmId: Long, now: Instant = wallClock.now()): UpcomingAlarm? { val alarm = alarms.find(alarmId) ?: return null - return preview(alarm, states.state(alarmId), wallClock.now(), zones.current()) + return preview(alarm, states.state(alarmId), now, zones.current()) } /** diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt index 3efd675..52e70af 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt @@ -15,8 +15,9 @@ import kotlin.time.Instant object AlarmSetCountdown { /** Pure. The rounding is [NextFireFormat]'s, so the snackbar and the row agree. */ fun of(upcoming: UpcomingAlarm?, now: Instant): NextFireLabel? { - if (upcoming == null || !upcoming.alarm.enabled) return null - if (upcoming.source != FireSource.OCCURRENCE) return null + // A switched-off alarm resolves to FireSource.NONE, so this one check + // also covers it. + if (upcoming?.source != FireSource.OCCURRENCE) return null val nextFire = upcoming.nextFire ?: return null return NextFireFormat.labelFor(nextFire, now) } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt index f8c2ba1..3f495eb 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt @@ -18,14 +18,13 @@ import de.jeanlucmakiola.clockula.ui.common.RingtonePicker import de.jeanlucmakiola.clockula.ui.common.RingtonePickerUiState import de.jeanlucmakiola.floret.di.ApplicationScope import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Job -import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch import java.time.DayOfWeek import javax.inject.Inject @@ -80,18 +79,15 @@ class AlarmEditorViewModel @Inject internal constructor( /** What the previewer is sounding, so the next preview knows to stop it. */ private var previewing: String? = null - /** The announcement waiting out [ANNOUNCE_SETTLE], so the next change can replace it. */ - private var announcing: Job? = null - - /** True while [announcing] has not yet announced — what [onCleared] flushes. */ - private var announcementPending = false - /** * "Alarm set for … from now" after a change that moves when the alarm rings * (#19). Nothing for a switched-off alarm, and nothing for the label or the - * sound: those move no fire time. + * sound: those move no fire time. Only ever about *this* alarm — the + * countdown names no alarm, so one about the row the user just switched + * on in the list would read as this one's. */ - val alarmSetNotice: Flow = announcer.notice + val alarmSetNotice: Flow = + announcer.notice.map { notice -> notice?.takeIf { it.alarmId == alarmId } } fun onAlarmSetNoticeShown(notice: AlarmSetNotice) = announcer.consumed(notice) @@ -120,17 +116,17 @@ class AlarmEditorViewModel @Inject internal constructor( /** Clamped, never throws. */ fun onTimePicked(hour: Int, minute: Int) = - edit(reschedule = true, announce = true) { it.copy(time = TimeOfDay.clamped(hour, minute)) } + edit(reschedule = true, announces = true) { it.copy(time = TimeOfDay.clamped(hour, minute)) } /** * The picker a new alarm opens on was closed without a time. The alarm * exists and is enabled at its default hour all the same — this is the * moment it was set, so it is announced (#19). */ - fun onNewAlarmTimeKept() = announce() + fun onNewAlarmTimeKept() = announcer.announce(alarmId) /** Announced once the taps settle: setting Mon–Fri is one snackbar, not five (#19). */ - fun onDayToggled(day: DayOfWeek) = edit(reschedule = true, announce = true, settle = ANNOUNCE_SETTLE) { alarm -> + fun onDayToggled(day: DayOfWeek) = edit(reschedule = true, announces = true, settle = ANNOUNCE_SETTLE) { alarm -> val days = if (day in alarm.repeatDays) alarm.repeatDays - day else alarm.repeatDays + day alarm.copy( repeatDays = days, @@ -155,7 +151,7 @@ class AlarmEditorViewModel @Inject internal constructor( engine.reschedule() // Skipping moves the next ring a day or more out, which is exactly // the kind of jump worth saying out loud. - announce() + announcer.announce(alarmId) } } @@ -244,6 +240,8 @@ class AlarmEditorViewModel @Inject internal constructor( if (engine.ringSession()?.alarm?.id == alarmId) engine.dismiss(alarmId) alarms.delete(alarmId) engine.reschedule() + // "Set for 2 hours from now" must not outlive the alarm on the list. + announcer.retract(alarmId) _state.value = AlarmEditorUiState.Gone } } @@ -256,9 +254,6 @@ class AlarmEditorViewModel @Inject internal constructor( labelWrites.value?.let { text -> appScope.launch { alarms.edit(alarmId) { it.copy(label = text) } } } - // Back within a second of the last day tap: the announcement still - // settling is made now, for the list to show (#19). - if (announcementPending) appScope.launch { announcer.announce(alarmId) } super.onCleared() } @@ -270,33 +265,20 @@ class AlarmEditorViewModel @Inject internal constructor( /** * Every editor write. [reschedule] is on only for a change that can move a * fire time — so fiddling with sound settings costs no AlarmManager traffic - * (M5 D12). [announce] is on for the same changes, after [settle]. + * (M5 D12). [announces] is on for the same changes, after [settle] — on + * the announcer's application scope, so leaving the editor while the + * repeat days settle still has them said, for the list to show (#19). */ private fun edit( reschedule: Boolean = false, - announce: Boolean = false, + announces: Boolean = false, settle: Duration = Duration.ZERO, transform: (Alarm) -> Alarm, ) { viewModelScope.launch { alarms.edit(alarmId, transform) if (reschedule) engine.reschedule() - if (announce) announce(settle) - } - } - - /** - * Replaces any announcement still settling, so a burst of changes ends in - * one snackbar about where they landed. Read from storage when it fires, - * not from the change that asked, so it is never about a superseded value. - */ - private fun announce(settle: Duration = Duration.ZERO) { - announcing?.cancel() - announcementPending = true - announcing = viewModelScope.launch { - delay(settle) - announcementPending = false - announcer.announce(alarmId) + if (announces) announcer.announce(alarmId, settle) } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt index cc26029..f9a8a55 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt @@ -7,27 +7,38 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.rememberUpdatedState import androidx.compose.ui.platform.LocalResources +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.compose.LocalLifecycleOwner +import androidx.lifecycle.repeatOnLifecycle import de.jeanlucmakiola.clockula.R import de.jeanlucmakiola.clockula.alarm.AlarmEngine import de.jeanlucmakiola.clockula.domain.alarm.AlarmSetCountdown import de.jeanlucmakiola.clockula.domain.format.NextFireLabel +import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock import de.jeanlucmakiola.clockula.domain.time.WallClock +import de.jeanlucmakiola.floret.di.ApplicationScope +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import javax.inject.Inject import javax.inject.Singleton +import kotlin.time.Duration import kotlin.time.Duration.Companion.seconds -import kotlin.time.Instant /** "Alarm set for … from now", about one alarm, until a screen has shown it (#19). */ data class AlarmSetNotice( val alarmId: Long, val countdown: NextFireLabel, - val announcedAt: Instant, + /** On the monotonic clock: a wall clock set back must not make an old notice fresh. */ + val announcedAt: Duration, ) /** @@ -38,56 +49,96 @@ data class AlarmSetNotice( * `AlarmClock` intents never announces itself: those already get the * status-bar icon. * - * A **state**, held app-wide, not an event: the notice stays until a screen - * has shown it to the end ([consumed]). The usual way to set an alarm is + * App-wide, and a **state** rather than an event: the notice stays until a + * screen has shown it to the end ([consumed]). The usual way to set an alarm is * pick → OK → back, and back disposes the editor's snackbar a moment after it - * appeared; a one-shot event would be gone with it, while this one is picked - * up again by the list underneath. A rotation mid-snackbar re-shows it the - * same way. + * appeared; the list underneath picks the same notice up. Announcing runs on + * the application scope for the same reason — leaving the editor while its + * repeat days settle must not lose them. */ @Singleton internal class AlarmSetAnnouncer @Inject constructor( private val engine: AlarmEngine, private val wall: WallClock, + private val elapsed: ElapsedRealtimeClock, + // `@param:` — the qualifier has no explicit @Target, and K2 resolves a bare + // annotation on a constructor `val` to the property instead. + @param:ApplicationScope private val appScope: CoroutineScope, ) { private val current = MutableStateFlow(null) /** - * Serialises [announce]: it reads storage and suspends before it writes - * [current], and two switch taps in quick succession must not let the - * earlier read land last — "set for 9 hours" about an alarm that is off. + * Serialises the read-then-publish: two switch taps in quick succession + * must not let the earlier read land last — "set for 9 hours" about an + * alarm that is off. */ private val lock = Mutex() + /** The request still settling, and the alarm it is about. Guarded by `this`. */ + private var pending: Job? = null + private var pendingFor: Long? = null + /** * The notice to show, or null. Past [FRESH_FOR] it is dropped unshown: a - * countdown that surfaced when the user came back to the app an hour - * later would be wrong, and about something they did long ago. + * countdown that surfaced when the user came back to the app a while later + * would be wrong, and about something they did long ago. */ val notice: Flow = current.map { notice -> - notice?.takeIf { wall.now() - it.announcedAt < FRESH_FOR } + notice?.takeIf { elapsed.elapsedRealtime() - it.announcedAt < FRESH_FOR } } /** - * Call once the write is stored. Reads the alarm as it is now, so it is never - * about a superseded value. An alarm that will not ring takes back a notice - * still pending about it, and otherwise says nothing. + * Call once the write is stored. Announces [alarmId] after [settle], unless + * a newer request replaces this one first — latest wins, so a burst of + * changes is one snackbar. It reads the alarm as it is then, so it is never + * about a superseded value; an alarm that will not ring takes back a notice + * still standing about it, and otherwise says nothing. */ - suspend fun announce(alarmId: Long): Unit = lock.withLock { - val now = wall.now() - val countdown = AlarmSetCountdown.of(engine.upcomingFor(alarmId), now) - if (countdown != null) { - current.value = AlarmSetNotice(alarmId, countdown, announcedAt = now) - } else if (current.value?.alarmId == alarmId) { - current.value = null + fun announce(alarmId: Long, settle: Duration = Duration.ZERO) { + synchronized(this) { + pending?.cancel() + pendingFor = alarmId + pending = appScope.launch { + delay(settle) + publish(alarmId) + } } } + /** Takes back anything said or about to be said about [alarmId]: it was deleted or dismissed. */ + fun retract(alarmId: Long) { + synchronized(this) { + if (pendingFor == alarmId) { + pending?.cancel() + pending = null + pendingFor = null + } + } + current.update { notice -> notice?.takeUnless { it.alarmId == alarmId } } + } + /** [notice] was shown until it timed out or was swiped away. */ fun consumed(notice: AlarmSetNotice) { current.compareAndSet(notice, null) } + private suspend fun publish(alarmId: Long) = lock.withLock { + // One `now` for both ends of the countdown, or a resolve that lands a + // hair past the occurrence would read as a day and an hour. + val now = wall.now() + val countdown = AlarmSetCountdown.of(engine.upcomingFor(alarmId, now), now) + current.update { standing -> + when { + // The same words about the same alarm stay as they are, rather + // than taking the snackbar down and putting it straight back. + countdown != null && standing?.alarmId == alarmId && standing.countdown == countdown -> standing + countdown != null -> AlarmSetNotice(alarmId, countdown, elapsed.elapsedRealtime()) + standing?.alarmId == alarmId -> null + else -> standing + } + } + } + private companion object { /** Comfortably longer than a short snackbar, so a hand-over between screens fits. */ val FRESH_FOR = 10.seconds @@ -95,13 +146,15 @@ internal class AlarmSetAnnouncer @Inject constructor( } /** - * Shows [notice] on [host]. Latest wins: a new countdown replaces the one on - * screen rather than queueing behind it, so a burst of changes is one snackbar - * that ends up saying the right thing, and a null takes it down. + * Shows [notice] on [host] while the screen is resumed. Latest wins: a new + * countdown replaces the one on screen rather than queueing behind it, and a + * null takes it down. * - * [onShown] runs only when the snackbar ended on its own or was swiped away. - * One cut short — by leaving the screen, or a rotation — stays pending for the - * next screen to show. + * Resumed, not merely composed: during a navigation cross-fade only the screen + * arriving is resumed, so the two never show it at once, and in the background + * nobody would see it. [onShown] runs only when the snackbar ended on its own or + * was swiped away; one cut short — by leaving, a rotation, the app going to the + * background — stays pending for whichever screen resumes next. */ @Composable internal fun AlarmSetSnackbarEffect( @@ -110,13 +163,15 @@ internal fun AlarmSetSnackbarEffect( onShown: (AlarmSetNotice) -> Unit, ) { val resources = LocalResources.current + val lifecycle = LocalLifecycleOwner.current.lifecycle val shown by rememberUpdatedState(onShown) - LaunchedEffect(notice, host, resources) { - notice.collectLatest { current -> - if (current == null) return@collectLatest - val message = alarmSetMessage(resources, current.countdown) - if (message != null) host.showSnackbar(message) - shown(current) + LaunchedEffect(notice, host, resources, lifecycle) { + lifecycle.repeatOnLifecycle(Lifecycle.State.RESUMED) { + notice.collectLatest { current -> + if (current == null) return@collectLatest + host.showSnackbar(alarmSetMessage(resources, current.countdown) ?: return@collectLatest) + shown(current) + } } } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt index cd16481..2b38175 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt @@ -83,7 +83,14 @@ class AlarmsViewModel @Inject internal constructor( // One engine verb, so the contract's dismissal and the user's are the // same code: it closes a ring, arms a repeating alarm's skip, or // disables a one-shot — each of which moves state only the engine owns. - viewModelScope.launch { engine.dismissUpcoming(alarmId) } + viewModelScope.launch { + engine.dismissUpcoming(alarmId) + // Whatever the dismissal did — disabled it, or skipped the very + // occurrence a notice counts down to — "set for" no longer holds. + // Not re-announced: this is the `AlarmClock` contract's request + // (M9 D13), which the snackbar stays out of (#19). + announcer.retract(alarmId) + } } /** Dismisses a ring first when switching off (M5 D11), then re-resolves. */ diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt index 02ad01b..48d5f32 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt @@ -1,6 +1,7 @@ package de.jeanlucmakiola.clockula.ui.alarms import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.viewModelScope import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import de.jeanlucmakiola.clockula.data.alarms.AlarmMapper @@ -11,6 +12,7 @@ import de.jeanlucmakiola.clockula.domain.TimeOfDay import de.jeanlucmakiola.clockula.domain.format.NextFireLabel import de.jeanlucmakiola.clockula.testing.AlarmEngineHarness import de.jeanlucmakiola.clockula.testing.BERLIN +import de.jeanlucmakiola.clockula.testing.FakeElapsedRealtimeClock import de.jeanlucmakiola.clockula.testing.FakeRingtoneCatalog import de.jeanlucmakiola.clockula.testing.FakeRingtonePreviewer import de.jeanlucmakiola.clockula.testing.MainDispatcherExtension @@ -23,6 +25,7 @@ import de.jeanlucmakiola.clockula.testing.instantAt import de.jeanlucmakiola.clockula.testing.offsetInstant import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope @@ -36,6 +39,7 @@ import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import java.time.DayOfWeek import kotlin.time.Duration.Companion.hours +import kotlin.time.Duration.Companion.seconds /** * The editor over the **real** alarm engine: every control writes at once @@ -56,10 +60,15 @@ class AlarmEditorViewModelTest { private val sevenToday = offsetInstant("2026-06-10T07:00+02:00") private val sevenTomorrow = offsetInstant("2026-06-11T07:00+02:00") + private val elapsed = FakeElapsedRealtimeClock() + + private fun TestScope.announcer(harness: AlarmEngineHarness) = + AlarmSetAnnouncer(harness.engine, harness.wallClock, elapsed, appScope = this) + private fun TestScope.editor( harness: AlarmEngineHarness, alarmId: Long = 1L, - announcer: AlarmSetAnnouncer = AlarmSetAnnouncer(harness.engine, harness.wallClock), + announcer: AlarmSetAnnouncer = announcer(harness), ) = AlarmEditorViewModel( savedStateHandle = SavedStateHandle(mapOf(AlarmRoutes.ARG_ALARM_ID to alarmId)), @@ -340,12 +349,14 @@ class AlarmEditorViewModelTest { runTest(main.dispatcher) { val harness = alarmEngineHarness(tempDir, now = fiveToday) harness.givenAlarmWith(hour = 7) - val announcer = AlarmSetAnnouncer(harness.engine, harness.wallClock) + val announcer = announcer(harness) val viewModel = editor(harness, announcer = announcer) advanceUntilIdle() viewModel.onDayToggled(DayOfWeek.MONDAY) advanceTimeBy(200) + // What `ViewModel.clear()` does first: the settle must not die with it. + viewModel.viewModelScope.cancel() viewModel.onCleared() advanceUntilIdle() @@ -422,11 +433,66 @@ class AlarmEditorViewModelTest { viewModel.onTimePicked(6, 45) advanceUntilIdle() - harness.wallClock.instant = fiveToday + 1.hours + // Monotonic: a wall clock set back an hour must not make it fresh again. + elapsed.advance(1.hours) + harness.wallClock.instant = fiveToday - 1.hours assertThat(viewModel.alarmSetNotice.first()).isNull() } + @Test + fun `a notice about another alarm is not shown in this editor`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(id = 1L, hour = 7) + harness.givenAlarmWith(id = 2L, hour = 22) + val announcer = announcer(harness) + val viewModel = editor(harness, alarmId = 2L, announcer = announcer) + advanceUntilIdle() + + announcer.announce(1L) + advanceUntilIdle() + + assertThat(announcer.notice.first()?.alarmId to viewModel.alarmSetNotice.first()) + .isEqualTo(1L to null) + } + + @Test + fun `deleting the alarm takes its notice back`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + val announcer = announcer(harness) + val viewModel = editor(harness, announcer = announcer) + advanceUntilIdle() + viewModel.onTimePicked(6, 45) + advanceUntilIdle() + + viewModel.onDelete() + advanceUntilIdle() + + assertThat(announcer.notice.first()).isNull() + } + + @Test + fun `the same countdown again leaves the standing notice alone`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + val viewModel = editor(harness) + advanceUntilIdle() + viewModel.onTimePicked(6, 45) + advanceUntilIdle() + val first = viewModel.alarmSetNotice.first() + + // A snackbar taken down and put straight back would flicker. + elapsed.advance(1.seconds) + viewModel.onTimePicked(6, 45) + advanceUntilIdle() + + assertThat(viewModel.alarmSetNotice.first()).isSameInstanceAs(first) + } + // --- label --- @Test diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt index 64b0276..64530dd 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt @@ -7,6 +7,7 @@ import de.jeanlucmakiola.clockula.domain.TimeOfDay import de.jeanlucmakiola.clockula.domain.format.NextFireLabel import de.jeanlucmakiola.clockula.testing.AlarmEngineHarness import de.jeanlucmakiola.clockula.testing.BERLIN +import de.jeanlucmakiola.clockula.testing.FakeElapsedRealtimeClock import de.jeanlucmakiola.clockula.testing.MainDispatcherExtension import de.jeanlucmakiola.clockula.testing.RingEvent import de.jeanlucmakiola.clockula.testing.alarmEngineHarness @@ -53,7 +54,7 @@ class AlarmsViewModelTest { wall = harness.wallClock, zones = harness.zones, source = AlarmListSource(harness.engine, harness.wallClock, harness.ticker), - announcer = AlarmSetAnnouncer(harness.engine, harness.wallClock), + announcer = AlarmSetAnnouncer(harness.engine, harness.wallClock, FakeElapsedRealtimeClock(), this), ).also { viewModel -> backgroundScope.launch { viewModel.state.collect {} } } private fun AlarmsUiState.rows(): List = (this as AlarmsUiState.Loaded).rows @@ -239,6 +240,24 @@ class AlarmsViewModelTest { assertThat(viewModel.alarmSetNotice.first()?.alarmId).isEqualTo(1L) } + @Test + fun `dismissing the announced alarm takes the announcement back`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 7, repeatDays = RepeatDays.EVERY_DAY, enabled = false) + val viewModel = viewModel(harness) + advanceUntilIdle() + viewModel.onToggleEnabled(1L, enabled = true) + advanceUntilIdle() + + viewModel.onDismissUpcoming(1L) + advanceUntilIdle() + + // Skipped, still on — and still not re-announced: the dismissal is + // the AlarmClock contract's, which the snackbar stays out of. + assertThat(viewModel.alarmSetNotice.first()).isNull() + } + @Test fun `adding an alarm leaves the announcement to the editor`(@TempDir tempDir: Path) = runTest(main.dispatcher) { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2051aee..f63c3bc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1126,8 +1126,13 @@ at once, but waits for repeat-day taps to settle for a second, so Mon–Fri is one snackbar rather than five. `AlarmSetAnnouncer` is an app-wide singleton holding the notice as *state* until a screen has shown it to the end: pick → OK → back disposes the editor's snackbar a moment after it appears, and the -list underneath picks the same notice up. A notice older than ten seconds is -dropped unshown rather than shown with a stale countdown. +list underneath picks the same notice up. The settle runs on the application +scope, so leaving the editor mid-settle still announces; a screen shows the +notice only while resumed, so a cross-fade never shows it twice; the editor +shows only notices about its own alarm; deleting an alarm, or the `AlarmClock` +chooser dismissing it, takes its notice back. A notice older than ten seconds +on the monotonic clock is dropped unshown rather than shown with a stale +countdown. `upcoming(refresh: Flow)` takes its **re-read cadence as a parameter**. The resolver needs a `now`, and `upcoming` reads one inside a `combine` of the From d2bf4349c54d5f889ac62a22cdf005ed52465059 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 19:18:19 +0200 Subject: [PATCH 4/9] fix(alarms): never announce an alarm that has since stopped ringing A notice is now re-checked against the engine before a screen shows it, so an alarm switched off or deleted by any path - the ring screen, a notification, an intent, a restore - is never announced as set. A retract takes the publish lock, so a publish already past its read cannot put the notice back. Settling is per alarm: switching one alarm off in the list no longer cancels the repeat days still settling on another. The identical-words dedupe is gone, since it could swallow a real confirmation. The editor's writes that move a fire time, with their reschedule and announcement, run on the application scope. Pick, OK, back could otherwise cancel the reschedule half-way and leave a stored time the system had not been given. upcomingFor() reads alarm and state under the engine lock, so a fire cannot land between the two reads. Refs #19 --- .../clockula/alarm/AlarmEngine.kt | 17 +++-- .../ui/alarms/AlarmEditorViewModel.kt | 16 +++-- .../clockula/ui/alarms/AlarmSetSnackbar.kt | 65 ++++++++++++------- .../ui/alarms/AlarmEditorViewModelTest.kt | 12 ++-- .../clockula/ui/alarms/AlarmsViewModelTest.kt | 27 +++++++- docs/ARCHITECTURE.md | 16 +++-- 6 files changed, 100 insertions(+), 53 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt index 070b660..20b849f 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -197,13 +197,18 @@ class AlarmEngine @Inject constructor( /** * One alarm's [upcoming] entry, resolved against [now]. Read-only like - * [upcoming], and for the same reason it takes no lock: it writes nothing. - * Null for an id no alarm has. A caller that measures a countdown passes - * the `now` it measures from, so both ends of it are the same instant. + * [upcoming]; null for an id no alarm has. A caller that measures a + * countdown passes the `now` it measures from, so both ends of it are the + * same instant. + * + * It writes nothing, but it does take [lock]: the alarm and its ring state + * are two reads, and a fire landing between them would pair an alarm with + * a state that is not its own. `upcoming()` gets the same consistency from + * combining the two flows. */ - suspend fun upcomingFor(alarmId: Long, now: Instant = wallClock.now()): UpcomingAlarm? { - val alarm = alarms.find(alarmId) ?: return null - return preview(alarm, states.state(alarmId), now, zones.current()) + suspend fun upcomingFor(alarmId: Long, now: Instant = wallClock.now()): UpcomingAlarm? = lock.withLock { + val alarm = alarms.find(alarmId) ?: return@withLock null + preview(alarm, states.state(alarmId), now, zones.current()) } /** diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt index 3f495eb..c525fff 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt @@ -146,7 +146,9 @@ class AlarmEditorViewModel @Inject internal constructor( } fun onSkipNextChanged(skip: Boolean) { - viewModelScope.launch { + // On the application scope, like every write that moves a fire time: + // see [edit]. + appScope.launch { alarms.setSkipNextOccurrence(alarmId, skip) engine.reschedule() // Skipping moves the next ring a day or more out, which is exactly @@ -265,9 +267,13 @@ class AlarmEditorViewModel @Inject internal constructor( /** * Every editor write. [reschedule] is on only for a change that can move a * fire time — so fiddling with sound settings costs no AlarmManager traffic - * (M5 D12). [announces] is on for the same changes, after [settle] — on - * the announcer's application scope, so leaving the editor while the - * repeat days settle still has them said, for the list to show (#19). + * (M5 D12). [announces] is on for the same changes, after [settle] (#19). + * + * A change that moves a fire time runs on the application scope. Pick → + * OK → back is the usual way to set an alarm, and back clears this + * ViewModel: on its own scope the reschedule — and the "Alarm set for …" + * the list is meant to pick up — could be cancelled half-way, leaving a + * stored time the system had not been given yet. */ private fun edit( reschedule: Boolean = false, @@ -275,7 +281,7 @@ class AlarmEditorViewModel @Inject internal constructor( settle: Duration = Duration.ZERO, transform: (Alarm) -> Alarm, ) { - viewModelScope.launch { + (if (reschedule) appScope else viewModelScope).launch { alarms.edit(alarmId, transform) if (reschedule) engine.reschedule() if (announces) announcer.announce(alarmId, settle) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt index f9a8a55..bfb3df4 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt @@ -74,17 +74,23 @@ internal class AlarmSetAnnouncer @Inject constructor( */ private val lock = Mutex() - /** The request still settling, and the alarm it is about. Guarded by `this`. */ - private var pending: Job? = null - private var pendingFor: Long? = null + /** + * Requests still settling, one per alarm: a request about one alarm must + * not cancel another's — switching B off in the list would otherwise + * swallow the repeat days just set on A. Guarded by `this`. + */ + private val pending = mutableMapOf() /** * The notice to show, or null. Past [FRESH_FOR] it is dropped unshown: a * countdown that surfaced when the user came back to the app a while later - * would be wrong, and about something they did long ago. + * would be wrong, and about something they did long ago. It is checked + * against the engine again on the way out, so an alarm switched off or + * deleted by any path — the ring screen, a notification, an intent, a + * restore — is never announced as set. */ val notice: Flow = current.map { notice -> - notice?.takeIf { elapsed.elapsedRealtime() - it.announcedAt < FRESH_FOR } + notice?.takeIf { isFresh(it) && stillRings(it) } } /** @@ -96,25 +102,26 @@ internal class AlarmSetAnnouncer @Inject constructor( */ fun announce(alarmId: Long, settle: Duration = Duration.ZERO) { synchronized(this) { - pending?.cancel() - pendingFor = alarmId - pending = appScope.launch { + pending.remove(alarmId)?.cancel() + val job = appScope.launch { delay(settle) publish(alarmId) } + pending[alarmId] = job + job.invokeOnCompletion { synchronized(this) { pending.remove(alarmId, job) } } } } - /** Takes back anything said or about to be said about [alarmId]: it was deleted or dismissed. */ - fun retract(alarmId: Long) { - synchronized(this) { - if (pendingFor == alarmId) { - pending?.cancel() - pending = null - pendingFor = null - } + /** + * Takes back anything said or about to be said about [alarmId]: it was + * deleted or dismissed. Under the same lock as [publish], so a publish + * already past its read cannot put the notice back afterwards. + */ + suspend fun retract(alarmId: Long) { + synchronized(this) { pending.remove(alarmId)?.cancel() } + lock.withLock { + current.update { notice -> notice?.takeUnless { it.alarmId == alarmId } } } - current.update { notice -> notice?.takeUnless { it.alarmId == alarmId } } } /** [notice] was shown until it timed out or was swiped away. */ @@ -123,15 +130,9 @@ internal class AlarmSetAnnouncer @Inject constructor( } private suspend fun publish(alarmId: Long) = lock.withLock { - // One `now` for both ends of the countdown, or a resolve that lands a - // hair past the occurrence would read as a day and an hour. - val now = wall.now() - val countdown = AlarmSetCountdown.of(engine.upcomingFor(alarmId, now), now) + val countdown = countdownOf(alarmId) current.update { standing -> when { - // The same words about the same alarm stay as they are, rather - // than taking the snackbar down and putting it straight back. - countdown != null && standing?.alarmId == alarmId && standing.countdown == countdown -> standing countdown != null -> AlarmSetNotice(alarmId, countdown, elapsed.elapsedRealtime()) standing?.alarmId == alarmId -> null else -> standing @@ -139,6 +140,18 @@ internal class AlarmSetAnnouncer @Inject constructor( } } + private fun isFresh(notice: AlarmSetNotice): Boolean = + elapsed.elapsedRealtime() - notice.announcedAt < FRESH_FOR + + private suspend fun stillRings(notice: AlarmSetNotice): Boolean = countdownOf(notice.alarmId) != null + + private suspend fun countdownOf(alarmId: Long): NextFireLabel? { + // One `now` for both ends of the countdown, or a resolve that lands a + // hair past the occurrence would read as a day and an hour. + val now = wall.now() + return AlarmSetCountdown.of(engine.upcomingFor(alarmId, now), now) + } + private companion object { /** Comfortably longer than a short snackbar, so a hand-over between screens fits. */ val FRESH_FOR = 10.seconds @@ -169,7 +182,9 @@ internal fun AlarmSetSnackbarEffect( lifecycle.repeatOnLifecycle(Lifecycle.State.RESUMED) { notice.collectLatest { current -> if (current == null) return@collectLatest - host.showSnackbar(alarmSetMessage(resources, current.countdown) ?: return@collectLatest) + // A notice with nothing to say is used up all the same, so it + // cannot sit pending for ever. + alarmSetMessage(resources, current.countdown)?.let { host.showSnackbar(it) } shown(current) } } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt index 48d5f32..f245c7a 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt @@ -39,7 +39,6 @@ import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import java.time.DayOfWeek import kotlin.time.Duration.Companion.hours -import kotlin.time.Duration.Companion.seconds /** * The editor over the **real** alarm engine: every control writes at once @@ -475,7 +474,7 @@ class AlarmEditorViewModelTest { } @Test - fun `the same countdown again leaves the standing notice alone`(@TempDir tempDir: Path) = + fun `an alarm switched off by another path is not announced as set`(@TempDir tempDir: Path) = runTest(main.dispatcher) { val harness = alarmEngineHarness(tempDir, now = fiveToday) harness.givenAlarmWith(hour = 7) @@ -483,14 +482,11 @@ class AlarmEditorViewModelTest { advanceUntilIdle() viewModel.onTimePicked(6, 45) advanceUntilIdle() - val first = viewModel.alarmSetNotice.first() - // A snackbar taken down and put straight back would flicker. - elapsed.advance(1.seconds) - viewModel.onTimePicked(6, 45) - advanceUntilIdle() + // The ring screen, a notification or an intent: none of them retracts. + harness.alarms.setEnabled(1L, enabled = false) - assertThat(viewModel.alarmSetNotice.first()).isSameInstanceAs(first) + assertThat(viewModel.alarmSetNotice.first()).isNull() } // --- label --- diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt index 64530dd..b3465e2 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt @@ -26,6 +26,7 @@ import org.junit.jupiter.api.extension.RegisterExtension import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.seconds /** * The list over the **real** alarm engine: what the rows say, that the readout @@ -48,13 +49,16 @@ class AlarmsViewModelTest { * `collectAsStateWithLifecycle`; without one there is no subscriber and * nothing upstream would run. */ - private fun TestScope.viewModel(harness: AlarmEngineHarness) = AlarmsViewModel( + private fun TestScope.viewModel( + harness: AlarmEngineHarness, + announcer: AlarmSetAnnouncer = AlarmSetAnnouncer(harness.engine, harness.wallClock, FakeElapsedRealtimeClock(), this), + ) = AlarmsViewModel( alarms = harness.alarms, engine = harness.engine, wall = harness.wallClock, zones = harness.zones, source = AlarmListSource(harness.engine, harness.wallClock, harness.ticker), - announcer = AlarmSetAnnouncer(harness.engine, harness.wallClock, FakeElapsedRealtimeClock(), this), + announcer = announcer, ).also { viewModel -> backgroundScope.launch { viewModel.state.collect {} } } private fun AlarmsUiState.rows(): List = (this as AlarmsUiState.Loaded).rows @@ -258,6 +262,25 @@ class AlarmsViewModelTest { assertThat(viewModel.alarmSetNotice.first()).isNull() } + @Test + fun `switching another alarm off keeps one still settling`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 7) + harness.givenAlarm(id = 2L, hour = 9) + val announcer = AlarmSetAnnouncer(harness.engine, harness.wallClock, FakeElapsedRealtimeClock(), this) + val viewModel = viewModel(harness, announcer) + advanceUntilIdle() + + // The editor's repeat days on alarm 1, still settling... + announcer.announce(1L, settle = 1.seconds) + // ...when alarm 2 is switched off in the list. + viewModel.onToggleEnabled(2L, enabled = false) + advanceUntilIdle() + + assertThat(viewModel.alarmSetNotice.first()?.alarmId).isEqualTo(1L) + } + @Test fun `adding an alarm leaves the announcement to the editor`(@TempDir tempDir: Path) = runTest(main.dispatcher) { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index f63c3bc..cfa5939 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1126,13 +1126,15 @@ at once, but waits for repeat-day taps to settle for a second, so Mon–Fri is one snackbar rather than five. `AlarmSetAnnouncer` is an app-wide singleton holding the notice as *state* until a screen has shown it to the end: pick → OK → back disposes the editor's snackbar a moment after it appears, and the -list underneath picks the same notice up. The settle runs on the application -scope, so leaving the editor mid-settle still announces; a screen shows the -notice only while resumed, so a cross-fade never shows it twice; the editor -shows only notices about its own alarm; deleting an alarm, or the `AlarmClock` -chooser dismissing it, takes its notice back. A notice older than ten seconds -on the monotonic clock is dropped unshown rather than shown with a stale -countdown. +list underneath picks the same notice up. The editor's fire-time-moving +writes, their reschedule and the per-alarm settle all run on the application +scope, so leaving the editor at once neither loses the announcement nor leaves +a stored time unscheduled. A screen shows the notice only while resumed, so a +cross-fade never shows it twice; the editor shows only notices about its own +alarm; a notice is re-checked against the engine before it is shown, so an +alarm switched off or deleted by any path is never announced as set. A notice +older than ten seconds on the monotonic clock is dropped unshown rather than +shown with a stale countdown. `upcoming(refresh: Flow)` takes its **re-read cadence as a parameter**. The resolver needs a `now`, and `upcoming` reads one inside a `combine` of the From ff49667d39b22ed2866f4bba8511647bad5409f4 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 19:26:24 +0200 Subject: [PATCH 5/9] fix(alarms): keep the alarm-set notice live and editor writes in order The notice now follows the engine's upcoming() list, so a snackbar already on screen comes down when its alarm is switched off by another path, and one whose ring moved, such as a skip armed from the notification, shows the new countdown. Consuming matches the announcement, not the words. Editor writes that move a fire time, skip-next and delete run on the application scope through one fair queue, entered undispatched so they commit in tap order. Without it, "skip next" followed at once by untapping the only repeat day could land out of order and leave a one-shot with a skip armed, which disables it. Refs #19 --- .../ui/alarms/AlarmEditorViewModel.kt | 35 +++++++++++++++---- .../clockula/ui/alarms/AlarmSetSnackbar.kt | 33 ++++++++++------- .../ui/alarms/AlarmEditorViewModelTest.kt | 32 +++++++++++++++++ 3 files changed, 82 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt index c525fff..e26c246 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt @@ -18,6 +18,7 @@ import de.jeanlucmakiola.clockula.ui.common.RingtonePicker import de.jeanlucmakiola.clockula.ui.common.RingtonePickerUiState import de.jeanlucmakiola.floret.di.ApplicationScope import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -26,6 +27,8 @@ import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import java.time.DayOfWeek import javax.inject.Inject import kotlin.time.Duration @@ -91,6 +94,9 @@ class AlarmEditorViewModel @Inject internal constructor( fun onAlarmSetNoticeShown(notice: AlarmSetNotice) = announcer.consumed(notice) + /** Orders [scheduleWrite]s. */ + private val scheduleWrites = Mutex() + /** Memoised catalog answers, keyed on URI — see [metaOf]. */ private val ringtoneMeta = mutableMapOf() @@ -146,9 +152,9 @@ class AlarmEditorViewModel @Inject internal constructor( } fun onSkipNextChanged(skip: Boolean) { - // On the application scope, like every write that moves a fire time: - // see [edit]. - appScope.launch { + // Absolute, so it must land in tap order against a day toggle that + // disarms the skip: see [scheduleWrite]. + scheduleWrite { alarms.setSkipNextOccurrence(alarmId, skip) engine.reschedule() // Skipping moves the next ring a day or more out, which is exactly @@ -238,7 +244,7 @@ class AlarmEditorViewModel @Inject internal constructor( /** Dismisses a ring first (M5 D11), deletes, and the state becomes [AlarmEditorUiState.Gone]. */ fun onDelete() { - viewModelScope.launch { + scheduleWrite { if (engine.ringSession()?.alarm?.id == alarmId) engine.dismiss(alarmId) alarms.delete(alarmId) engine.reschedule() @@ -281,13 +287,30 @@ class AlarmEditorViewModel @Inject internal constructor( settle: Duration = Duration.ZERO, transform: (Alarm) -> Alarm, ) { - (if (reschedule) appScope else viewModelScope).launch { + if (!reschedule) { + viewModelScope.launch { alarms.edit(alarmId, transform) } + return + } + scheduleWrite { alarms.edit(alarmId, transform) - if (reschedule) engine.reschedule() + engine.reschedule() if (announces) announcer.announce(alarmId, settle) } } + /** + * A write that moves a fire time, its reschedule and whatever follows, on + * the application scope (see [edit]) and **in tap order**: started + * undispatched, each takes its place in [scheduleWrites]' fair queue on the + * caller's thread, before the first suspension. The application scope is + * multi-threaded, so without it "skip next" followed at once by untapping + * the only repeat day could commit out of order, leaving a one-shot with a + * skip armed — which the resolver answers by disabling it (D14). + */ + private fun scheduleWrite(block: suspend () -> Unit) { + appScope.launch(start = CoroutineStart.UNDISPATCHED) { scheduleWrites.withLock { block() } } + } + private suspend fun render(inputs: Inputs): AlarmEditorUiState { val alarm = inputs.alarm ?: return AlarmEditorUiState.Gone return AlarmEditorUiState.Editing( diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt index bfb3df4..fbdf471 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt @@ -23,7 +23,8 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.collectLatest -import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex @@ -84,14 +85,19 @@ internal class AlarmSetAnnouncer @Inject constructor( /** * The notice to show, or null. Past [FRESH_FOR] it is dropped unshown: a * countdown that surfaced when the user came back to the app a while later - * would be wrong, and about something they did long ago. It is checked - * against the engine again on the way out, so an alarm switched off or - * deleted by any path — the ring screen, a notification, an intent, a - * restore — is never announced as set. + * would be wrong, and about something they did long ago. + * + * Live against the engine: every alarm write re-reads it, so an alarm + * switched off or deleted by any path — the ring screen, a notification, + * an intent, a restore — takes a snackbar down even while it is showing, + * and one whose ring moved (a skip armed from the notification) says the + * new countdown rather than the one it was announced with. */ - val notice: Flow = current.map { notice -> - notice?.takeIf { isFresh(it) && stillRings(it) } - } + val notice: Flow = combine(current, engine.upcoming()) { notice, upcoming -> + if (notice == null || !isFresh(notice)) return@combine null + val entry = upcoming.firstOrNull { it.alarm.id == notice.alarmId } + AlarmSetCountdown.of(entry, wall.now())?.let { notice.copy(countdown = it) } + }.distinctUntilChanged() /** * Call once the write is stored. Announces [alarmId] after [settle], unless @@ -124,9 +130,14 @@ internal class AlarmSetAnnouncer @Inject constructor( } } - /** [notice] was shown until it timed out or was swiped away. */ + /** + * [notice] was shown until it timed out or was swiped away. Matched on the + * announcement, not the words: the countdown shown may be a re-read one. + */ fun consumed(notice: AlarmSetNotice) { - current.compareAndSet(notice, null) + current.update { standing -> + standing?.takeUnless { it.alarmId == notice.alarmId && it.announcedAt == notice.announcedAt } + } } private suspend fun publish(alarmId: Long) = lock.withLock { @@ -143,8 +154,6 @@ internal class AlarmSetAnnouncer @Inject constructor( private fun isFresh(notice: AlarmSetNotice): Boolean = elapsed.elapsedRealtime() - notice.announcedAt < FRESH_FOR - private suspend fun stillRings(notice: AlarmSetNotice): Boolean = countdownOf(notice.alarmId) != null - private suspend fun countdownOf(alarmId: Long): NextFireLabel? { // One `now` for both ends of the countdown, or a resolve that lands a // hair past the occurrence would read as a day and an hour. diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt index f245c7a..0ab4d95 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt @@ -473,6 +473,38 @@ class AlarmEditorViewModelTest { assertThat(announcer.notice.first()).isNull() } + @Test + fun `a ring moved by another path is announced with its new countdown`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7, repeatDays = RepeatDays.EVERY_DAY) + val viewModel = editor(harness) + advanceUntilIdle() + viewModel.onTimePicked(7, 0) + advanceUntilIdle() + + // A skip armed from the upcoming-alarm notification: no retract. + harness.alarms.setSkipNextOccurrence(1L, true) + + assertThat(viewModel.countdown()).isEqualTo(NextFireLabel.InDaysHours(1, 2)) + } + + @Test + fun `skip then untapping the only day lands in tap order`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7, repeatDays = RepeatDays.of(DayOfWeek.MONDAY)) + val viewModel = editor(harness) + advanceUntilIdle() + + viewModel.onSkipNextChanged(true) + viewModel.onDayToggled(DayOfWeek.MONDAY) + advanceUntilIdle() + + val stored = harness.alarmDao.stored.single() + assertThat(stored.enabled to stored.skipNextOccurrence).isEqualTo(true to false) + } + @Test fun `an alarm switched off by another path is not announced as set`(@TempDir tempDir: Path) = runTest(main.dispatcher) { From 9516dc119893977e07baa9f9f52ffb27ac877f23 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 19:35:29 +0200 Subject: [PATCH 6/9] refactor(alarms): derive the alarm-set notice from the engine alone The announcer now records only which alarm a user action set, and when. What the snackbar says is derived from upcoming() while a screen shows it, re-subscribed per announcement so the first read is from after the write. That removes the second resolve path through the engine lock (upcomingFor is gone), the publish lock, and the flash of a pre-write countdown. The engine is observed only while an announcement stands. Freshness is judged when a screen picks the announcement up, so one already showing is not cut off half-way. Consuming matches a sequence number. Latest action wins across alarms again: the sentence names no alarm, so a settling announcement must never land on top of a newer action on another one. Switching an alarm off announces nothing, and a dismissal retracts before it skips. Refs #19 --- .../clockula/alarm/AlarmEngine.kt | 38 ++--- .../clockula/ui/alarms/AlarmSetSnackbar.kt | 154 ++++++++---------- .../clockula/ui/alarms/AlarmsViewModel.kt | 20 +-- docs/ARCHITECTURE.md | 35 ++-- 4 files changed, 106 insertions(+), 141 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt index 20b849f..d9ff0a9 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -29,7 +29,6 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock -import java.time.ZoneId import kotlin.time.Duration.Companion.minutes import kotlin.time.Instant import javax.inject.Inject @@ -188,38 +187,23 @@ class AlarmEngine @Inject constructor( val zone = zones.current() val byId = states.associateBy { it.alarmId } alarms - .map { alarm -> preview(alarm, byId[alarm.id] ?: AlarmRingState.initial(alarm.id), now, zone) } + .map { alarm -> + // The resolver's state is deliberately discarded: a read must + // not write, however much it resolved on the way past. + val resolution = AlarmResolver.resolve( + alarm, + byId[alarm.id] ?: AlarmRingState.initial(alarm.id), + now, + zone, + ) + UpcomingAlarm(alarm, resolution.nextFire, resolution.source) + } .sortedWith( compareBy(nullsLast()) { it.nextFire } .thenBy { it.alarm.id }, ) } - /** - * One alarm's [upcoming] entry, resolved against [now]. Read-only like - * [upcoming]; null for an id no alarm has. A caller that measures a - * countdown passes the `now` it measures from, so both ends of it are the - * same instant. - * - * It writes nothing, but it does take [lock]: the alarm and its ring state - * are two reads, and a fire landing between them would pair an alarm with - * a state that is not its own. `upcoming()` gets the same consistency from - * combining the two flows. - */ - suspend fun upcomingFor(alarmId: Long, now: Instant = wallClock.now()): UpcomingAlarm? = lock.withLock { - val alarm = alarms.find(alarmId) ?: return@withLock null - preview(alarm, states.state(alarmId), now, zones.current()) - } - - /** - * The resolver in preview mode. Its state is deliberately discarded: a read - * must not write, however much it resolved on the way past. - */ - private fun preview(alarm: Alarm, state: AlarmRingState, now: Instant, zone: ZoneId): UpcomingAlarm { - val resolution = AlarmResolver.resolve(alarm, state, now, zone) - return UpcomingAlarm(alarm, resolution.nextFire, resolution.source) - } - private suspend fun rescheduleLocked(): ScheduledFire? { val now = wallClock.now() val zone = zones.current() diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt index fbdf471..1020152 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt @@ -18,28 +18,29 @@ import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock import de.jeanlucmakiola.clockula.domain.time.WallClock import de.jeanlucmakiola.floret.di.ApplicationScope import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.collectLatest -import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock import javax.inject.Inject import javax.inject.Singleton import kotlin.time.Duration import kotlin.time.Duration.Companion.seconds -/** "Alarm set for … from now", about one alarm, until a screen has shown it (#19). */ +/** "Alarm set for … from now", about one alarm, as a screen should show it (#19). */ data class AlarmSetNotice( val alarmId: Long, val countdown: NextFireLabel, - /** On the monotonic clock: a wall clock set back must not make an old notice fresh. */ - val announcedAt: Duration, + /** Which announcement this is, so [AlarmSetAnnouncer.consumed] takes back only that one. */ + val sequence: Long, ) /** @@ -50,12 +51,17 @@ data class AlarmSetNotice( * `AlarmClock` intents never announces itself: those already get the * status-bar icon. * - * App-wide, and a **state** rather than an event: the notice stays until a - * screen has shown it to the end ([consumed]). The usual way to set an alarm is - * pick → OK → back, and back disposes the editor's snackbar a moment after it - * appeared; the list underneath picks the same notice up. Announcing runs on - * the application scope for the same reason — leaving the editor while its - * repeat days settle must not lose them. + * It records only *which* alarm was set, and when. What to say is derived from + * the engine while a screen shows it, so it is never a stale or superseded + * value, and an alarm switched off or deleted by any path — the row's switch, + * the ring screen, a notification, an intent, a restore — says nothing. + * + * App-wide, and a **state** rather than an event: the announcement stands + * until a screen has shown it to the end ([consumed]). The usual way to set an + * alarm is pick → OK → back, and back disposes the editor's snackbar a moment + * after it appeared; the list underneath picks the same announcement up. The + * settle runs on the application scope for the same reason — leaving the + * editor while its repeat days settle must not lose them. */ @Singleton internal class AlarmSetAnnouncer @Inject constructor( @@ -66,99 +72,79 @@ internal class AlarmSetAnnouncer @Inject constructor( // annotation on a constructor `val` to the property instead. @param:ApplicationScope private val appScope: CoroutineScope, ) { - private val current = MutableStateFlow(null) + /** One user action's announcement. [at] is monotonic: a wall clock set back must not refresh it. */ + private data class Announcement(val alarmId: Long, val at: Duration, val sequence: Long) + + private val current = MutableStateFlow(null) + + /** The request still settling, and the alarm it is about. Guarded by `this`. */ + private var pending: Job? = null + private var pendingFor: Long? = null + private var sequence = 0L /** - * Serialises the read-then-publish: two switch taps in quick succession - * must not let the earlier read land last — "set for 9 hours" about an - * alarm that is off. - */ - private val lock = Mutex() - - /** - * Requests still settling, one per alarm: a request about one alarm must - * not cancel another's — switching B off in the list would otherwise - * swallow the repeat days just set on A. Guarded by `this`. - */ - private val pending = mutableMapOf() - - /** - * The notice to show, or null. Past [FRESH_FOR] it is dropped unshown: a - * countdown that surfaced when the user came back to the app a while later - * would be wrong, and about something they did long ago. + * The notice to show, or null. * - * Live against the engine: every alarm write re-reads it, so an alarm - * switched off or deleted by any path — the ring screen, a notification, - * an intent, a restore — takes a snackbar down even while it is showing, - * and one whose ring moved (a skip armed from the notification) says the - * new countdown rather than the one it was announced with. + * Freshness is judged once, when a screen picks the announcement up: past + * [FRESH_FOR] it is dropped unshown — a countdown surfacing when the user + * came back to the app a while later would be about something they did + * long ago — but one already showing is not cut off half-way. The engine + * is observed only while an announcement stands, and a new announcement + * re-subscribes, so its first read is from after the write that asked. */ - val notice: Flow = combine(current, engine.upcoming()) { notice, upcoming -> - if (notice == null || !isFresh(notice)) return@combine null - val entry = upcoming.firstOrNull { it.alarm.id == notice.alarmId } - AlarmSetCountdown.of(entry, wall.now())?.let { notice.copy(countdown = it) } - }.distinctUntilChanged() + @OptIn(ExperimentalCoroutinesApi::class) + val notice: Flow = current + .flatMapLatest { announcement -> + if (announcement == null || elapsed.elapsedRealtime() - announcement.at >= FRESH_FOR) { + flowOf(null) + } else { + engine.upcoming().map { upcoming -> + val entry = upcoming.firstOrNull { it.alarm.id == announcement.alarmId } + AlarmSetCountdown.of(entry, wall.now()) + ?.let { AlarmSetNotice(announcement.alarmId, it, announcement.sequence) } + } + } + } + .distinctUntilChanged() /** * Call once the write is stored. Announces [alarmId] after [settle], unless - * a newer request replaces this one first — latest wins, so a burst of - * changes is one snackbar. It reads the alarm as it is then, so it is never - * about a superseded value; an alarm that will not ring takes back a notice - * still standing about it, and otherwise says nothing. + * a newer request replaces this one first. Latest wins, across alarms too: + * the snackbar names no alarm, so a settling announcement about one must + * never land on top of the user's newer action on another and read as + * that one's countdown. */ fun announce(alarmId: Long, settle: Duration = Duration.ZERO) { synchronized(this) { - pending.remove(alarmId)?.cancel() - val job = appScope.launch { + pending?.cancel() + pendingFor = alarmId + pending = appScope.launch { delay(settle) - publish(alarmId) + synchronized(this@AlarmSetAnnouncer) { + current.value = Announcement(alarmId, elapsed.elapsedRealtime(), ++sequence) + } } - pending[alarmId] = job - job.invokeOnCompletion { synchronized(this) { pending.remove(alarmId, job) } } } } /** - * Takes back anything said or about to be said about [alarmId]: it was - * deleted or dismissed. Under the same lock as [publish], so a publish - * already past its read cannot put the notice back afterwards. + * Takes back anything said or about to be said about [alarmId]. For a + * change of state the engine would read as "still rings" — a dismissal + * that skips the occurrence being counted down to. */ - suspend fun retract(alarmId: Long) { - synchronized(this) { pending.remove(alarmId)?.cancel() } - lock.withLock { - current.update { notice -> notice?.takeUnless { it.alarmId == alarmId } } + fun retract(alarmId: Long) { + synchronized(this) { + if (pendingFor == alarmId) { + pending?.cancel() + pendingFor = null + } + current.update { standing -> standing?.takeUnless { it.alarmId == alarmId } } } } - /** - * [notice] was shown until it timed out or was swiped away. Matched on the - * announcement, not the words: the countdown shown may be a re-read one. - */ + /** [notice] was shown until it timed out or was swiped away. */ fun consumed(notice: AlarmSetNotice) { - current.update { standing -> - standing?.takeUnless { it.alarmId == notice.alarmId && it.announcedAt == notice.announcedAt } - } - } - - private suspend fun publish(alarmId: Long) = lock.withLock { - val countdown = countdownOf(alarmId) - current.update { standing -> - when { - countdown != null -> AlarmSetNotice(alarmId, countdown, elapsed.elapsedRealtime()) - standing?.alarmId == alarmId -> null - else -> standing - } - } - } - - private fun isFresh(notice: AlarmSetNotice): Boolean = - elapsed.elapsedRealtime() - notice.announcedAt < FRESH_FOR - - private suspend fun countdownOf(alarmId: Long): NextFireLabel? { - // One `now` for both ends of the countdown, or a resolve that lands a - // hair past the occurrence would read as a day and an hour. - val now = wall.now() - return AlarmSetCountdown.of(engine.upcomingFor(alarmId, now), now) + current.update { standing -> standing?.takeUnless { it.sequence == notice.sequence } } } private companion object { diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt index 2b38175..a0ba1a2 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt @@ -83,14 +83,11 @@ class AlarmsViewModel @Inject internal constructor( // One engine verb, so the contract's dismissal and the user's are the // same code: it closes a ring, arms a repeating alarm's skip, or // disables a one-shot — each of which moves state only the engine owns. - viewModelScope.launch { - engine.dismissUpcoming(alarmId) - // Whatever the dismissal did — disabled it, or skipped the very - // occurrence a notice counts down to — "set for" no longer holds. - // Not re-announced: this is the `AlarmClock` contract's request - // (M9 D13), which the snackbar stays out of (#19). - announcer.retract(alarmId) - } + // First, so the snackbar never re-reads the skip and counts down to the + // occurrence after it. Not re-announced: this is the `AlarmClock` + // contract's request (M9 D13), which the snackbar stays out of (#19). + announcer.retract(alarmId) + viewModelScope.launch { engine.dismissUpcoming(alarmId) } } /** Dismisses a ring first when switching off (M5 D11), then re-resolves. */ @@ -99,9 +96,10 @@ class AlarmsViewModel @Inject internal constructor( if (!enabled) stopRingIfRinging(alarmId) alarms.setEnabled(alarmId, enabled) engine.reschedule() - // Switching on says when it will ring; switching off takes back a - // "set for" still on screen about this alarm (#19). - announcer.announce(alarmId) + // Switching on says when it will ring (#19). Switching off needs + // nothing: the notice is read from the engine, so one about this + // alarm falls silent by itself, and one about another stands. + if (enabled) announcer.announce(alarmId) } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index cfa5939..9e7a5c8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1116,25 +1116,22 @@ its own writes. `reschedule()` is called explicitly: from `ClockulaApp`, from the receivers, and from the alarm editor. `upcoming()` runs the same resolver in preview mode and *discards* the state it returns; a read must not write, and a test asserts the store recorded nothing across a full collection. -`upcomingFor(alarmId)` is the same read for one alarm, on demand: the list and -the editor ask it right after a user's change to say how long until that alarm -rings. `AlarmSetCountdown` (pure, in `domain/alarm/`) decides what that says — -only a natural occurrence of an enabled alarm, rounded as the row's countdown -is, never a snooze or a ring. The `AlarmClock` intents never reach it, since -they already get the status-bar icon (#19). The editor announces a time pick -at once, but waits for repeat-day taps to settle for a second, so Mon–Fri is -one snackbar rather than five. `AlarmSetAnnouncer` is an app-wide singleton -holding the notice as *state* until a screen has shown it to the end: pick → -OK → back disposes the editor's snackbar a moment after it appears, and the -list underneath picks the same notice up. The editor's fire-time-moving -writes, their reschedule and the per-alarm settle all run on the application -scope, so leaving the editor at once neither loses the announcement nor leaves -a stored time unscheduled. A screen shows the notice only while resumed, so a -cross-fade never shows it twice; the editor shows only notices about its own -alarm; a notice is re-checked against the engine before it is shown, so an -alarm switched off or deleted by any path is never announced as set. A notice -older than ten seconds on the monotonic clock is dropped unshown rather than -shown with a stale countdown. +The "Alarm set for … from now" snackbar (#19) is built on `upcoming()` too. +`AlarmSetAnnouncer`, an app-wide singleton, records only *which* alarm a user +action in the list or the editor just set; while a resumed screen shows it, +what it says is derived from `upcoming()`, through `AlarmSetCountdown` (pure, +in `domain/alarm/`): only a natural occurrence of an enabled alarm, rounded as +the row's countdown is, never a snooze or a ring. So an alarm switched off or +deleted by any path is never announced as set, and the `AlarmClock` intents, +which already get the status-bar icon, never reach it. The announcement is +*state*, standing until a screen has shown it to the end: pick → OK → back +disposes the editor's snackbar a moment after it appears, and the list +underneath picks it up. The editor announces a time pick at once and waits a +second for repeat-day taps to settle, so Mon–Fri is one snackbar; the settle, +and the editor's fire-time-moving writes with their reschedule, run in tap +order on the application scope, so leaving the editor at once neither loses +the announcement nor leaves a stored time unscheduled. An announcement older +than ten seconds on the monotonic clock is dropped unshown. `upcoming(refresh: Flow)` takes its **re-read cadence as a parameter**. The resolver needs a `now`, and `upcoming` reads one inside a `combine` of the From d2e73901c17d497852a16ff1a5bbdb01e050f58b Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 19:43:35 +0200 Subject: [PATCH 7/9] fix(alarms): let a silent alarm-set announcement go for good An announcement whose alarm has nothing to say, such as one switched on and straight back off, is now dropped when a screen reads it. Before, it stood, and something else switching the alarm back on, such as an AlarmClock intent, would have announced it after all. A settle job that was cancelled while waiting for the monitor no longer writes. Refs #19 --- .../clockula/ui/alarms/AlarmSetSnackbar.kt | 19 +++++++++++++++---- .../clockula/ui/alarms/AlarmsViewModelTest.kt | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt index 1020152..26ef529 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt @@ -24,11 +24,12 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.collectLatest +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map -import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.update +import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import javax.inject.Inject import javax.inject.Singleton @@ -100,8 +101,12 @@ internal class AlarmSetAnnouncer @Inject constructor( } else { engine.upcoming().map { upcoming -> val entry = upcoming.firstOrNull { it.alarm.id == announcement.alarmId } - AlarmSetCountdown.of(entry, wall.now()) - ?.let { AlarmSetNotice(announcement.alarmId, it, announcement.sequence) } + val countdown = AlarmSetCountdown.of(entry, wall.now()) + // Nothing to say is the answer for good: an announcement + // left standing would speak up later, long after the + // action, should something else switch the alarm back on. + if (countdown == null) dropIfCurrent(announcement) + countdown?.let { AlarmSetNotice(announcement.alarmId, it, announcement.sequence) } } } } @@ -121,7 +126,9 @@ internal class AlarmSetAnnouncer @Inject constructor( pending = appScope.launch { delay(settle) synchronized(this@AlarmSetAnnouncer) { - current.value = Announcement(alarmId, elapsed.elapsedRealtime(), ++sequence) + // A retract or a newer request may have cancelled this one + // after its delay, while it waited for the monitor. + if (isActive) current.value = Announcement(alarmId, elapsed.elapsedRealtime(), ++sequence) } } } @@ -147,6 +154,10 @@ internal class AlarmSetAnnouncer @Inject constructor( current.update { standing -> standing?.takeUnless { it.sequence == notice.sequence } } } + private fun dropIfCurrent(announcement: Announcement) { + current.update { standing -> standing?.takeUnless { it.sequence == announcement.sequence } } + } + private companion object { /** Comfortably longer than a short snackbar, so a hand-over between screens fits. */ val FRESH_FOR = 10.seconds diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt index b3465e2..17a990f 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt @@ -281,6 +281,25 @@ class AlarmsViewModelTest { assertThat(viewModel.alarmSetNotice.first()?.alarmId).isEqualTo(1L) } + @Test + fun `an announcement that fell silent does not speak up when something else re-enables the alarm`( + @TempDir tempDir: Path, + ) = runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 7, enabled = false) + val viewModel = viewModel(harness) + advanceUntilIdle() + viewModel.onToggleEnabled(1L, enabled = true) + viewModel.onToggleEnabled(1L, enabled = false) + advanceUntilIdle() + check(viewModel.alarmSetNotice.first() == null) + + // An AlarmClock intent switching it back on: intents never announce. + harness.alarms.setEnabled(1L, enabled = true) + + assertThat(viewModel.alarmSetNotice.first()).isNull() + } + @Test fun `adding an alarm leaves the announcement to the editor`(@TempDir tempDir: Path) = runTest(main.dispatcher) { From 706a3847041c57b845cb4caee06d3deb90e28084 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 19:51:47 +0200 Subject: [PATCH 8/9] fix(alarms): word the alarm-set snackbar once per announcement Every write re-read the engine and re-worded a standing announcement, so tapping Mon-Fri while the snackbar showed restarted it on each tap, as did a label keystroke across a minute. The words now come from the first read; later writes can only take the snackbar down, when the alarm stops ringing. The settled announcement still replaces it once. Deleting no longer retracts by hand: a deleted alarm already reads as nothing to say. Refs #19 --- .../ui/alarms/AlarmEditorViewModel.kt | 2 -- .../clockula/ui/alarms/AlarmSetSnackbar.kt | 28 ++++++++++++------- .../ui/alarms/AlarmEditorViewModelTest.kt | 25 +++++++++++++++++ 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt index e26c246..c49df9a 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt @@ -248,8 +248,6 @@ class AlarmEditorViewModel @Inject internal constructor( if (engine.ringSession()?.alarm?.id == alarmId) engine.dismiss(alarmId) alarms.delete(alarmId) engine.reschedule() - // "Set for 2 hours from now" must not outlive the alarm on the list. - announcer.retract(alarmId) _state.value = AlarmEditorUiState.Gone } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt index 26ef529..ac7cd17 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt @@ -99,14 +99,24 @@ internal class AlarmSetAnnouncer @Inject constructor( if (announcement == null || elapsed.elapsedRealtime() - announcement.at >= FRESH_FOR) { flowOf(null) } else { + // Worded once, from the first read: later writes — the next + // repeat-day tap, a label keystroke across a minute — must not + // restart a snackbar already showing. They can only take it + // down, when the alarm stops ringing at all. + var worded: AlarmSetNotice? = null engine.upcoming().map { upcoming -> val entry = upcoming.firstOrNull { it.alarm.id == announcement.alarmId } val countdown = AlarmSetCountdown.of(entry, wall.now()) - // Nothing to say is the answer for good: an announcement - // left standing would speak up later, long after the - // action, should something else switch the alarm back on. - if (countdown == null) dropIfCurrent(announcement) - countdown?.let { AlarmSetNotice(announcement.alarmId, it, announcement.sequence) } + if (countdown == null) { + // Nothing to say is the answer for good: left standing, + // it would speak up long after the action, should + // something else switch the alarm back on. + clear(announcement.sequence) + null + } else { + worded ?: AlarmSetNotice(announcement.alarmId, countdown, announcement.sequence) + .also { worded = it } + } } } } @@ -150,12 +160,10 @@ internal class AlarmSetAnnouncer @Inject constructor( } /** [notice] was shown until it timed out or was swiped away. */ - fun consumed(notice: AlarmSetNotice) { - current.update { standing -> standing?.takeUnless { it.sequence == notice.sequence } } - } + fun consumed(notice: AlarmSetNotice) = clear(notice.sequence) - private fun dropIfCurrent(announcement: Announcement) { - current.update { standing -> standing?.takeUnless { it.sequence == announcement.sequence } } + private fun clear(sequence: Long) { + current.update { standing -> standing?.takeUnless { it.sequence == sequence } } } private companion object { diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt index 0ab4d95..9e36626 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt @@ -343,6 +343,31 @@ class AlarmEditorViewModelTest { } } + @Test + fun `day taps while a notice shows do not restart it before they settle`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + val viewModel = editor(harness) + advanceUntilIdle() + viewModel.onTimePicked(9, 0) + advanceUntilIdle() + + viewModel.alarmSetNotice.test { + assertThat(awaitItem()?.countdown).isEqualTo(NextFireLabel.InHoursMinutes(4, 0)) + viewModel.onDayToggled(DayOfWeek.MONDAY) + advanceTimeBy(300) + viewModel.onDayToggled(DayOfWeek.TUESDAY) + advanceTimeBy(300) + expectNoEvents() + advanceUntilIdle() + + // One more snackbar, once settled: Wednesday 05:00 to Monday 09:00. + assertThat(awaitItem()?.countdown).isEqualTo(NextFireLabel.InDaysHours(5, 4)) + expectNoEvents() + } + } + @Test fun `leaving while the days settle still announces them, for the list`(@TempDir tempDir: Path) = runTest(main.dispatcher) { From 219116603509a597898eee766bd456455a85d87c Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 26 Oct 2026 20:21:56 +0100 Subject: [PATCH 9/9] fix(alarms): show the alarm-set countdown as the family's SnackChip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The countdown used the stock Material snackbar, while Agendula and Calendula confirm with floret-kit's SnackChip. It now opens as that chip: bottom-start in the FAB's band on the list, ellipsising short of the FAB, and in the editor's snackbar slot so it follows the predictive-back preview. It fills the band's height, as an Undo chip does in the other apps, and sits on the secondary container to stand apart from the cards. The chip has no timer, so AlarmSetChipBand holds it for a short snackbar's four seconds, stretched as the accessibility settings ask. The hand-over is unchanged: a chip cut short stays pending for the next resumed screen. Beside the FAB there is room for about 308 dp, so the message is now "Alarm in 7 hours and 12 minutes" rather than "Alarm set for … from now", which would have lost its number to the ellipsis. Refs #19 --- .../domain/alarm/AlarmSetCountdown.kt | 6 +- .../clockula/ui/alarms/AlarmEditorScreen.kt | 14 +- .../ui/alarms/AlarmEditorViewModel.kt | 6 +- .../{AlarmSetSnackbar.kt => AlarmSetChip.kt} | 97 ++++++++++--- .../clockula/ui/alarms/AlarmsScreen.kt | 132 ++++++++++-------- .../clockula/ui/alarms/AlarmsViewModel.kt | 6 +- app/src/main/res/values/strings.xml | 6 +- .../domain/alarm/AlarmSetCountdownTest.kt | 2 +- .../ui/alarms/AlarmEditorViewModelTest.kt | 6 +- .../clockula/ui/alarms/AlarmsViewModelTest.kt | 2 +- docs/ARCHITECTURE.md | 10 +- 11 files changed, 182 insertions(+), 105 deletions(-) rename app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/{AlarmSetSnackbar.kt => AlarmSetChip.kt} (71%) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt index 52e70af..6eccb85 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdown.kt @@ -5,15 +5,15 @@ import de.jeanlucmakiola.clockula.domain.format.NextFireLabel import kotlin.time.Instant /** - * What "Alarm set for 7 hours and 12 minutes from now" says once the user has + * What "Alarm in 7 hours and 12 minutes" says once the user has * set an alarm, or null when it should say nothing (#19). * * Only a natural occurrence is announced. A switched-off alarm will not ring at * all; a ringing or snoozed one is answering the snooze, not the change the - * user just made, and "set for 9 minutes from now" would read as a mistake. + * user just made, and "Alarm in 9 minutes" would read as a mistake. */ object AlarmSetCountdown { - /** Pure. The rounding is [NextFireFormat]'s, so the snackbar and the row agree. */ + /** Pure. The rounding is [NextFireFormat]'s, so the chip and the row agree. */ fun of(upcoming: UpcomingAlarm?, now: Instant): NextFireLabel? { // A switched-off alarm resolves to FireSource.NONE, so this one check // also covers it. diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt index 088a42a..7cfa77e 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScreen.kt @@ -11,8 +11,6 @@ import androidx.compose.material3.AlertDialog import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.SnackbarHost -import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.material3.TextButton @@ -20,7 +18,6 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier @@ -62,8 +59,6 @@ fun AlarmEditorScreen( val previewPlaying by viewModel.previewPlaying.collectAsStateWithLifecycle() var confirmingDelete by rememberSaveable { mutableStateOf(false) } val formatTime = rememberAlarmTimeFormatter() - val snackbarHost = remember { SnackbarHostState() } - AlarmSetSnackbarEffect(viewModel.alarmSetNotice, snackbarHost, viewModel::onAlarmSetNoticeShown) // Registered unconditionally, and deliberately **not** inside the // `picker?.let` below: the picker's open flag is ViewModel state that does @@ -89,9 +84,14 @@ fun AlarmEditorScreen( onBack = onBack, modifier = modifier, predictiveBack = true, - // Lifted over the live pill, which floats above every screen. + // In the snackbar slot, so it follows the predictive-back preview, and + // lifted over the live pill, which floats above every screen. snackbarHost = { - SnackbarHost(snackbarHost, Modifier.padding(bottom = LocalLivePillInset.current)) + AlarmSetChipBand( + notice = viewModel.alarmSetNotice, + onShown = viewModel::onAlarmSetNoticeShown, + modifier = Modifier.padding(bottom = LocalLivePillInset.current), + ) }, actions = { if (state is AlarmEditorUiState.Editing) { diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt index c49df9a..4ed4f61 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModel.kt @@ -83,7 +83,7 @@ class AlarmEditorViewModel @Inject internal constructor( private var previewing: String? = null /** - * "Alarm set for … from now" after a change that moves when the alarm rings + * "Alarm in …" after a change that moves when the alarm rings * (#19). Nothing for a switched-off alarm, and nothing for the label or the * sound: those move no fire time. Only ever about *this* alarm — the * countdown names no alarm, so one about the row the user just switched @@ -131,7 +131,7 @@ class AlarmEditorViewModel @Inject internal constructor( */ fun onNewAlarmTimeKept() = announcer.announce(alarmId) - /** Announced once the taps settle: setting Mon–Fri is one snackbar, not five (#19). */ + /** Announced once the taps settle: setting Mon–Fri is one chip, not five (#19). */ fun onDayToggled(day: DayOfWeek) = edit(reschedule = true, announces = true, settle = ANNOUNCE_SETTLE) { alarm -> val days = if (day in alarm.repeatDays) alarm.repeatDays - day else alarm.repeatDays + day alarm.copy( @@ -275,7 +275,7 @@ class AlarmEditorViewModel @Inject internal constructor( * * A change that moves a fire time runs on the application scope. Pick → * OK → back is the usual way to set an alarm, and back clears this - * ViewModel: on its own scope the reschedule — and the "Alarm set for …" + * ViewModel: on its own scope the reschedule — and the "Alarm in …" * the list is meant to pick up — could be cancelled half-way, leaving a * stored time the system had not been given yet. */ diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetChip.kt similarity index 71% rename from app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt rename to app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetChip.kt index ac7cd17..ee3993b 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetChip.kt @@ -1,12 +1,25 @@ package de.jeanlucmakiola.clockula.ui.alarms import android.content.res.Resources -import androidx.compose.material3.SnackbarHostState +import androidx.compose.foundation.layout.BoxWithConstraints +import androidx.compose.foundation.layout.fillMaxHeight +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalAccessibilityManager import androidx.compose.ui.platform.LocalResources +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.dp import androidx.lifecycle.Lifecycle import androidx.lifecycle.compose.LocalLifecycleOwner import androidx.lifecycle.repeatOnLifecycle @@ -16,6 +29,9 @@ import de.jeanlucmakiola.clockula.domain.alarm.AlarmSetCountdown import de.jeanlucmakiola.clockula.domain.format.NextFireLabel import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock import de.jeanlucmakiola.clockula.domain.time.WallClock +import de.jeanlucmakiola.floret.components.SnackChip +import de.jeanlucmakiola.floret.components.SnackChipHeight +import de.jeanlucmakiola.floret.components.SnackChipMargin import de.jeanlucmakiola.floret.di.ApplicationScope import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -36,7 +52,7 @@ import javax.inject.Singleton import kotlin.time.Duration import kotlin.time.Duration.Companion.seconds -/** "Alarm set for … from now", about one alarm, as a screen should show it (#19). */ +/** "Alarm in …", about one alarm, as a screen should show it (#19). */ data class AlarmSetNotice( val alarmId: Long, val countdown: NextFireLabel, @@ -59,7 +75,7 @@ data class AlarmSetNotice( * * App-wide, and a **state** rather than an event: the announcement stands * until a screen has shown it to the end ([consumed]). The usual way to set an - * alarm is pick → OK → back, and back disposes the editor's snackbar a moment + * alarm is pick → OK → back, and back disposes the editor's chip a moment * after it appeared; the list underneath picks the same announcement up. The * settle runs on the application scope for the same reason — leaving the * editor while its repeat days settle must not lose them. @@ -101,7 +117,7 @@ internal class AlarmSetAnnouncer @Inject constructor( } else { // Worded once, from the first read: later writes — the next // repeat-day tap, a label keystroke across a minute — must not - // restart a snackbar already showing. They can only take it + // restart a chip already showing. They can only take it // down, when the alarm stops ringing at all. var worded: AlarmSetNotice? = null engine.upcoming().map { upcoming -> @@ -125,7 +141,7 @@ internal class AlarmSetAnnouncer @Inject constructor( /** * Call once the write is stored. Announces [alarmId] after [settle], unless * a newer request replaces this one first. Latest wins, across alarms too: - * the snackbar names no alarm, so a settling announcement about one must + * the chip names no alarm, so a settling announcement about one must * never land on top of the user's newer action on another and read as * that one's countdown. */ @@ -159,7 +175,7 @@ internal class AlarmSetAnnouncer @Inject constructor( } } - /** [notice] was shown until it timed out or was swiped away. */ + /** [notice] was shown until it timed out. */ fun consumed(notice: AlarmSetNotice) = clear(notice.sequence) private fun clear(sequence: Long) { @@ -167,44 +183,93 @@ internal class AlarmSetAnnouncer @Inject constructor( } private companion object { - /** Comfortably longer than a short snackbar, so a hand-over between screens fits. */ + /** Comfortably longer than the chip stands, so a hand-over between screens fits. */ val FRESH_FOR = 10.seconds } } /** - * Shows [notice] on [host] while the screen is resumed. Latest wins: a new - * countdown replaces the one on screen rather than queueing behind it, and a - * null takes it down. + * Shows [notice] as the family's [SnackChip] while the screen is resumed, in a + * FAB-height band along the bottom edge. Latest wins: a new countdown replaces + * the one on screen rather than queueing behind it, and a null takes it down. * * Resumed, not merely composed: during a navigation cross-fade only the screen * arriving is resumed, so the two never show it at once, and in the background - * nobody would see it. [onShown] runs only when the snackbar ended on its own or - * was swiped away; one cut short — by leaving, a rotation, the app going to the + * nobody would see it. [onShown] runs only when the chip stood for its full + * time; one cut short — by leaving, a rotation, the app going to the * background — stays pending for whichever screen resumes next. + * + * The chip carries no timer of its own, so the stand is a short snackbar's, + * stretched the way the accessibility settings ask. + * + * [endClearance] is what the band keeps free at its end for a FAB sharing it; + * the message ellipsises short of it instead of running underneath. */ @Composable -internal fun AlarmSetSnackbarEffect( +internal fun AlarmSetChipBand( notice: Flow, - host: SnackbarHostState, onShown: (AlarmSetNotice) -> Unit, + modifier: Modifier = Modifier, + endClearance: Dp = 0.dp, ) { val resources = LocalResources.current val lifecycle = LocalLifecycleOwner.current.lifecycle + val accessibility = LocalAccessibilityManager.current val shown by rememberUpdatedState(onShown) - LaunchedEffect(notice, host, resources, lifecycle) { + // Kept after the chip hides, so its exit animation still has words in it. + var message by remember { mutableStateOf(null) } + var visible by remember { mutableStateOf(false) } + LaunchedEffect(notice, resources, lifecycle, accessibility) { lifecycle.repeatOnLifecycle(Lifecycle.State.RESUMED) { notice.collectLatest { current -> if (current == null) return@collectLatest // A notice with nothing to say is used up all the same, so it // cannot sit pending for ever. - alarmSetMessage(resources, current.countdown)?.let { host.showSnackbar(it) } + alarmSetMessage(resources, current.countdown)?.let { text -> + message = text + visible = true + try { + delay( + accessibility?.calculateRecommendedTimeoutMillis( + SHOWN_FOR_MILLIS, + containsIcons = false, + containsText = true, + containsControls = false, + ) ?: SHOWN_FOR_MILLIS, + ) + } finally { + visible = false + } + } shown(current) } } } + BoxWithConstraints( + modifier = modifier + .fillMaxWidth() + .padding(start = SnackChipMargin, end = SnackChipMargin, bottom = SnackChipMargin) + .height(SnackChipHeight), + contentAlignment = Alignment.CenterStart, + ) { + message?.let { text -> + // The band's full height, as a chip with an Undo stands in the + // other apps; on the secondary container, so it reads apart from + // the alarm cards behind it. + SnackChip( + visible = visible, + message = text, + modifier = Modifier.fillMaxHeight(), + maxWidth = maxWidth - endClearance, + color = MaterialTheme.colorScheme.secondaryContainer, + ) + } + } } +/** A short snackbar's stand. */ +private const val SHOWN_FOR_MILLIS = 4_000L + /** * The full sentence: proper plurals, and the unit pairs the row's countdown * uses — minutes, hours and minutes, or days and hours. A zero second unit is diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt index 519bb2f..d1965cc 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt @@ -1,11 +1,17 @@ package de.jeanlucmakiola.clockula.ui.alarms import android.content.Intent +import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.WindowInsets +import androidx.compose.foundation.layout.WindowInsetsSides import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.only import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.systemBars +import androidx.compose.foundation.layout.windowInsetsPadding import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.material.icons.Icons @@ -15,13 +21,11 @@ import androidx.compose.material3.FloatingActionButton import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.SnackbarHost -import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue -import androidx.compose.runtime.remember +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource @@ -62,73 +66,81 @@ fun AlarmsScreen( val context = LocalContext.current val pillInset = LocalLivePillInset.current val addAlarm = stringResource(R.string.alarms_add) - val snackbarHost = remember { SnackbarHostState() } LaunchedEffect(viewModel) { // The FAB creates the alarm and the editor edits a persisted row, so // the id to navigate to arrives from the ViewModel (M5 D2, D22). viewModel.openEditor.collectLatest(onCreateAlarm) } - AlarmSetSnackbarEffect(viewModel.alarmSetNotice, snackbarHost, viewModel::onAlarmSetNoticeShown) - CollapsingScaffold( - title = stringResource(R.string.tab_alarms), - modifier = modifier, - scrollable = false, - actions = { - IconButton(onClick = onOpenSettings) { - Icon( - imageVector = Icons.Rounded.Settings, - contentDescription = stringResource(R.string.settings_title), - ) - } - }, - // The scaffold stacks it above the FAB, which already clears the pill. - snackbarHost = { SnackbarHost(snackbarHost) }, - floatingActionButton = { - FloatingActionButton( - onClick = viewModel::onAddAlarm, - // The pill is bottom-centre and the FAB bottom-end, so on a - // narrow phone they would otherwise overlap (M5 D6). - modifier = Modifier.padding(bottom = pillInset), - ) { - Icon(Icons.Rounded.Add, contentDescription = addAlarm) - } - }, - ) { - LazyColumn( - modifier = Modifier.fillMaxSize(), - verticalArrangement = Arrangement.spacedBy(12.dp), - contentPadding = PaddingValues( - top = AlarmsDefaults.ListBottomPadding, - bottom = AlarmsDefaults.ListBottomPadding + AlarmsDefaults.FabBand + pillInset, - ), + Box(modifier = modifier.fillMaxSize()) { + CollapsingScaffold( + title = stringResource(R.string.tab_alarms), + scrollable = false, + actions = { + IconButton(onClick = onOpenSettings) { + Icon( + imageVector = Icons.Rounded.Settings, + contentDescription = stringResource(R.string.settings_title), + ) + } + }, + floatingActionButton = { + FloatingActionButton( + onClick = viewModel::onAddAlarm, + // The pill is bottom-centre and the FAB bottom-end, so on a + // narrow phone they would otherwise overlap (M5 D6). + modifier = Modifier.padding(bottom = pillInset), + ) { + Icon(Icons.Rounded.Add, contentDescription = addAlarm) + } + }, ) { - if (rows != null && rows.isEmpty()) { - item { AlarmsEmptyState() } - } - items(rows.orEmpty(), key = { row -> row.id }) { row -> - AlarmRow( - row = row, - // A ringing alarm's row leads back to its ring screen, the - // way out if its notification was swiped away. - onClick = { - if (row.ringing) { - context.startActivity( - Intent(context, AlarmRingActivity::class.java) - .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK), - ) - } else { - onEditAlarm(row.id) - } - }, - onToggle = { enabled -> viewModel.onToggleEnabled(row.id, enabled) }, - // An alarm whose time changed slides to its new place - // instead of teleporting. - modifier = animateItemMotion(), - ) + LazyColumn( + modifier = Modifier.fillMaxSize(), + verticalArrangement = Arrangement.spacedBy(12.dp), + contentPadding = PaddingValues( + top = AlarmsDefaults.ListBottomPadding, + bottom = AlarmsDefaults.ListBottomPadding + AlarmsDefaults.FabBand + pillInset, + ), + ) { + if (rows != null && rows.isEmpty()) { + item { AlarmsEmptyState() } + } + items(rows.orEmpty(), key = { row -> row.id }) { row -> + AlarmRow( + row = row, + // A ringing alarm's row leads back to its ring screen, the + // way out if its notification was swiped away. + onClick = { + if (row.ringing) { + context.startActivity( + Intent(context, AlarmRingActivity::class.java) + .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK), + ) + } else { + onEditAlarm(row.id) + } + }, + onToggle = { enabled -> viewModel.onToggleEnabled(row.id, enabled) }, + // An alarm whose time changed slides to its new place + // instead of teleporting. + modifier = animateItemMotion(), + ) + } } } + // Beside the FAB, not stacked above it as a snackbar would be: the band is + // the FAB's own, measured from the same insets the scaffold gives it. + AlarmSetChipBand( + notice = viewModel.alarmSetNotice, + onShown = viewModel::onAlarmSetNoticeShown, + endClearance = AlarmsDefaults.FabBand, + modifier = Modifier + .align(Alignment.BottomStart) + .windowInsetsPadding(WindowInsets.systemBars.only(WindowInsetsSides.Bottom)) + .padding(bottom = pillInset), + ) } // "Which alarm should I dismiss?" — asked only when a search named more diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt index a0ba1a2..889c94a 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt @@ -46,7 +46,7 @@ class AlarmsViewModel @Inject internal constructor( private val newAlarms = Channel(Channel.CONFLATED) /** - * "Alarm set for … from now", after the row's switch turns an alarm on — or + * "Alarm in …", after the row's switch turns an alarm on — or * handed over by the editor the user has just left (#19). */ val alarmSetNotice: Flow = announcer.notice @@ -83,9 +83,9 @@ class AlarmsViewModel @Inject internal constructor( // One engine verb, so the contract's dismissal and the user's are the // same code: it closes a ring, arms a repeating alarm's skip, or // disables a one-shot — each of which moves state only the engine owns. - // First, so the snackbar never re-reads the skip and counts down to the + // First, so the chip never re-reads the skip and counts down to the // occurrence after it. Not re-announced: this is the `AlarmClock` - // contract's request (M9 D13), which the snackbar stays out of (#19). + // contract's request (M9 D13), which the chip stays out of (#19). announcer.retract(alarmId) viewModelScope.launch { engine.dismissUpcoming(alarmId) } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index c97e509..05d19d3 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -113,9 +113,9 @@ in %1$dd %2$dh Ringing now Snoozed \u00b7 %1$s - - Alarm set for %1$s from now - Alarm set for less than 1 minute from now + + Alarm in %1$s + Alarm in less than 1 minute %1$s and %2$s %1$d day diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdownTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdownTest.kt index 411b010..88bb8d4 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdownTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmSetCountdownTest.kt @@ -12,7 +12,7 @@ import kotlin.time.Duration.Companion.seconds import kotlin.time.Instant /** - * "Alarm set for … from now" speaks only for an alarm that will ring at a + * "Alarm in …" speaks only for an alarm that will ring at a * natural occurrence, and rounds the way the row's countdown does (#19). */ class AlarmSetCountdownTest { diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt index 9e36626..66476d6 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorViewModelTest.kt @@ -270,7 +270,7 @@ class AlarmEditorViewModelTest { assertThat(viewModel.editing().canSkipNext).isTrue() } - // --- "Alarm set for … from now" (#19) --- + // --- "Alarm in …" (#19) --- private suspend fun AlarmEditorViewModel.countdown() = alarmSetNotice.first()?.countdown @@ -296,7 +296,7 @@ class AlarmEditorViewModelTest { val viewModel = editor(harness) advanceUntilIdle() - // The AM/PM slip the snackbar is there to catch: 4:30 is behind + // The AM/PM slip the chip is there to catch: 4:30 is behind // us, so it rings tomorrow, 23½ hours away. viewModel.onTimePicked(4, 30) advanceUntilIdle() @@ -362,7 +362,7 @@ class AlarmEditorViewModelTest { expectNoEvents() advanceUntilIdle() - // One more snackbar, once settled: Wednesday 05:00 to Monday 09:00. + // One more chip, once settled: Wednesday 05:00 to Monday 09:00. assertThat(awaitItem()?.countdown).isEqualTo(NextFireLabel.InDaysHours(5, 4)) expectNoEvents() } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt index 17a990f..f2ad9d4 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModelTest.kt @@ -258,7 +258,7 @@ class AlarmsViewModelTest { advanceUntilIdle() // Skipped, still on — and still not re-announced: the dismissal is - // the AlarmClock contract's, which the snackbar stays out of. + // the AlarmClock contract's, which the chip stays out of. assertThat(viewModel.alarmSetNotice.first()).isNull() } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 9e7a5c8..2a9f8f1 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -86,7 +86,7 @@ kit module: the kit's roadmap keeps schedulers app-local. | Package | Holds | |---|---| | `domain/` | `Alarm.kt`, `Timer.kt`, `WorldClock.kt`, `Stopwatch.kt`, `ClockDefaults.kt`, `Ringtone.kt` (the silent sentinel), `RepeatSummary.kt`, `AlarmDefaults.kt`. Plain Kotlin, no Android | -| `domain/alarm/` | the alarm engine's pure half: occurrences, the resolver, the ring state, the presentation and scheduling policies, the challenge gate, and `AlarmSetCountdown`, what the "Alarm set for … from now" snackbar says (#19). The *shared* ring vocabulary moved out to `domain/ring/` in M6 | +| `domain/alarm/` | the alarm engine's pure half: occurrences, the resolver, the ring state, the presentation and scheduling policies, the challenge gate, and `AlarmSetCountdown`, what the "Alarm in …" chip says (#19). The *shared* ring vocabulary moved out to `domain/ring/` in M6 | | `domain/ring/` | what both ring paths share: `RingAudio` (the ramp tick, the volume floor), `VolumeRamp`, `AudioSourcePolicy`/`RingtoneSourceKind`, `RingFallbackPolicy`, `VibrationPattern` | | `domain/stopwatch/` | the stopwatch path's pure half: `StopwatchReadings` (the one reading the pill, the shade and the tab share), `LapStats` (best and worst), `StopwatchLaps` (the lap cap), `StopwatchNotificationPolicy` | | `domain/timer/` | the timer path's pure half: `TimerReadings` (the app's one timer precedence), `TimerExpiry` (the sweep and the slot's value), `TimerRingPolicy`, `TimerNotificationPolicy`, `TimerPresets`, `TimerDurationEntry`, `TimerSettings`, `TimerRing` | @@ -133,7 +133,7 @@ kit module: the kit's roadmap keeps schedulers app-local. | `ui/theme/`, `ui/crash/` | the M0/M1 theme and the crash-report surface | | `ui/shell/` | the navigation policy (`ShellNavigation`), the adaptive shell, the live pill and its source/ViewModel, the notification-permission ask. `EmptyTabScreen` is gone: its KDoc said it existed only until each of M5–M8 replaced its own tab, and the world clock was its last caller | | `ui/common/` | `rememberLocalTimeFormatter`, the app's one 12/24-hour formatter, over a `java.time.LocalTime`, with `rememberAlarmTimeFormatter` delegating to it (M8 D17); and, since M6, the ringtone picker (`RingtonePickerState`, `RingtonePickerScreen`), which both editors need | -| `ui/alarms/` | the real Alarms tab (M5): the routes, the list's pure row builder and its source, the two ViewModels, the list, the editor, the M3 time-picker host, the repeat-day selector and the override pickers (the ringtone picker moved to `ui/common/` in M6), and M9's `DismissChooser`/`DismissAlarmDialog`, the "which alarm should I dismiss?" question; and `AlarmSetAnnouncer`/`AlarmSetSnackbar`, the "Alarm set for … from now" snackbar both ViewModels raise after a user action (#19) | +| `ui/alarms/` | the real Alarms tab (M5): the routes, the list's pure row builder and its source, the two ViewModels, the list, the editor, the M3 time-picker host, the repeat-day selector and the override pickers (the ringtone picker moved to `ui/common/` in M6), and M9's `DismissChooser`/`DismissAlarmDialog`, the "which alarm should I dismiss?" question; and `AlarmSetAnnouncer`/`AlarmSetChipBand`, the "Alarm in …" floret `SnackChip` both ViewModels raise after a user action (#19) | | `ui/timers/` | the real Timers tab (M6): the routes, the pure row builder and its source, the two ViewModels, the list, the row, the setup panel and its keypad, the editor | | `ui/stopwatch/` | the real Stopwatch tab (M7): the pure row builder (`StopwatchRows`) and its source, the ViewModel, the readout panel with its controls, the lap table and its column header, `StopwatchDefaults` | | `ui/worldclock/` | the real World clock tab (M8): the pure row builders (`WorldClockRows`, `ZonePickerRows`) and their state, `WorldClockSource`, the ViewModel, the tab, the analog face, the row and the zone picker, `WorldClockDefaults` | @@ -1116,7 +1116,7 @@ its own writes. `reschedule()` is called explicitly: from `ClockulaApp`, from the receivers, and from the alarm editor. `upcoming()` runs the same resolver in preview mode and *discards* the state it returns; a read must not write, and a test asserts the store recorded nothing across a full collection. -The "Alarm set for … from now" snackbar (#19) is built on `upcoming()` too. +The "Alarm in …" chip (#19) is built on `upcoming()` too. `AlarmSetAnnouncer`, an app-wide singleton, records only *which* alarm a user action in the list or the editor just set; while a resumed screen shows it, what it says is derived from `upcoming()`, through `AlarmSetCountdown` (pure, @@ -1125,9 +1125,9 @@ the row's countdown is, never a snooze or a ring. So an alarm switched off or deleted by any path is never announced as set, and the `AlarmClock` intents, which already get the status-bar icon, never reach it. The announcement is *state*, standing until a screen has shown it to the end: pick → OK → back -disposes the editor's snackbar a moment after it appears, and the list +disposes the editor's chip a moment after it appears, and the list underneath picks it up. The editor announces a time pick at once and waits a -second for repeat-day taps to settle, so Mon–Fri is one snackbar; the settle, +second for repeat-day taps to settle, so Mon–Fri is one chip; the settle, and the editor's fire-time-moving writes with their reschedule, run in tap order on the application scope, so leaving the editor at once neither loses the announcement nor leaves a stored time unscheduled. An announcement older