From 697678b2260d4cefc5f0bb939ab8811c3c1d150a Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 07:54:43 +0200 Subject: [PATCH] fix(alarms): drop an Undo offered while no list could show it A delete can finish while no list is on screen, such as the app sent home the instant the editor's Delete was tapped. Its Undo is now dropped unshown once it is 30 seconds old, instead of surfacing hours later. Alarms and timers share one UndoSlot for this. A card whose swipe is re-enabled mid-settle still slides home. --- .../clockula/ui/alarms/AlarmUndo.kt | 28 ++-------- .../clockula/ui/alarms/AlarmsScreen.kt | 1 + .../clockula/ui/alarms/AlarmsViewModel.kt | 3 ++ .../clockula/ui/common/SwipeToDelete.kt | 7 +-- .../clockula/ui/common/UndoSlot.kt | 53 +++++++++++++++++++ .../clockula/ui/common/UndoSnackbar.kt | 9 +++- .../clockula/ui/timers/TimersScreen.kt | 1 + .../clockula/ui/timers/TimersViewModel.kt | 18 ++++--- .../ui/alarms/AlarmEditorViewModelTest.kt | 5 +- .../clockula/ui/alarms/AlarmsViewModelTest.kt | 3 +- .../clockula/ui/common/UndoSlotTest.kt | 40 ++++++++++++++ .../clockula/ui/timers/TimersViewModelTest.kt | 1 + docs/ARCHITECTURE.md | 5 +- 13 files changed, 134 insertions(+), 40 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSlot.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/clockula/ui/common/UndoSlotTest.kt diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmUndo.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmUndo.kt index 561fbfd..ce8b844 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmUndo.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmUndo.kt @@ -1,9 +1,8 @@ package de.jeanlucmakiola.clockula.ui.alarms import de.jeanlucmakiola.clockula.domain.alarm.DeletedAlarm -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asStateFlow +import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock +import de.jeanlucmakiola.clockula.ui.common.UndoSlot import javax.inject.Inject import javax.inject.Singleton @@ -16,27 +15,6 @@ import javax.inject.Singleton * answered; the list underneath shows the snackbar instead, and reads the * deletion from here. Held until the snackbar is answered, times out or is * left unanswered ([de.jeanlucmakiola.clockula.ui.common.UndoSnackbarEffect]). - * - * Memory only, deliberately: an Undo that survived process death would be - * offered with no snackbar anyone remembers. */ @Singleton -class AlarmUndo @Inject constructor() { - - private val slot = MutableStateFlow(null) - - /** Null when there is nothing to undo. */ - val pending: StateFlow = slot.asStateFlow() - - /** A newer deletion replaces an older one: only the latest snackbar is on screen. */ - fun offer(deleted: DeletedAlarm) { - slot.value = deleted - } - - /** - * Claims [deleted] for its one answer. False when it has already been - * answered or replaced, so a second tap cannot restore it twice and a - * stale snackbar cannot drop a newer deletion. - */ - fun take(deleted: DeletedAlarm): Boolean = slot.compareAndSet(deleted, null) -} +class AlarmUndo @Inject constructor(clock: ElapsedRealtimeClock) : UndoSlot(clock) 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 40aad3b..b96a451 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 @@ -83,6 +83,7 @@ fun AlarmsScreen( message = undoMessage.orEmpty(), onUndo = viewModel::onUndoDelete, onExpired = viewModel::onUndoExpired, + isStale = viewModel::isUndoStale, ) CollapsingScaffold( 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 09b14a4..5f77fd3 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 @@ -109,6 +109,9 @@ class AlarmsViewModel @Inject internal constructor( undo.take(deleted) } + /** Offered too long ago to show a snackbar for now ([AlarmUndo.isStale]). */ + fun isUndoStale(deleted: DeletedAlarm): Boolean = undo.isStale(deleted) + /** Dismisses a ring first when switching off (M5 D11), then re-resolves. */ fun onToggleEnabled(alarmId: Long, enabled: Boolean) { viewModelScope.launch { diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/SwipeToDelete.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/SwipeToDelete.kt index 680651a..0d2dda1 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/SwipeToDelete.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/SwipeToDelete.kt @@ -80,10 +80,11 @@ internal fun SwipeToDelete( // Derived, so a drag recomposes this once as the red appears, not per frame. val revealed by remember { derivedStateOf { travel.value > 0f } } - // Disabled mid-drag or mid-settle: the gesture that would have brought the - // card home is gone, so it comes home on its own. + // Disabled (or re-enabled) mid-drag or mid-settle: the gesture that would + // have brought the card home is gone, and so is any settle a quick flip + // back just cancelled, so it comes home on its own. LaunchedEffect(enabled) { - if (!enabled) travel.animateTo(0f, settleSpec) + if (travel.value > 0f) travel.animateTo(0f, settleSpec) } Box( diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSlot.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSlot.kt new file mode 100644 index 0000000..c4a91b0 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSlot.kt @@ -0,0 +1,53 @@ +package de.jeanlucmakiola.clockula.ui.common + +import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlin.time.Duration +import kotlin.time.Duration.Companion.seconds + +/** + * The one deletion an Undo snackbar can still bring back: offered by a delete, + * claimed once by the snackbar's answer. + * + * Deliberately memory only: an Undo that survived process death would be + * offered with no snackbar anyone remembers. + */ +open class UndoSlot(private val clock: ElapsedRealtimeClock) { + + private val slot = MutableStateFlow(null) + + @Volatile + private var offeredAt: Duration = Duration.ZERO + + /** Null when there is nothing to undo. */ + val pending: StateFlow = slot.asStateFlow() + + /** A newer deletion replaces an older one: only the latest snackbar is on screen. */ + fun offer(deleted: T) { + offeredAt = clock.elapsedRealtime() + slot.value = deleted + } + + /** + * Claims [deleted] for its one answer. False when it has already been + * answered or replaced, so a second tap cannot restore it twice and a + * stale snackbar cannot drop a newer deletion. + */ + fun take(deleted: T): Boolean = slot.compareAndSet(deleted, null) + + /** + * Whether [deleted] was offered too long ago to put a snackbar up for now. + * A delete can land while no list is on screen to show it (the app sent + * to the background the moment the editor's Delete was tapped); offered + * hours later, its Undo would restore a snapshot out of any context. + */ + fun isStale(deleted: T): Boolean = + slot.value == deleted && clock.elapsedRealtime() - offeredAt > SHOW_WITHIN + + private companion object { + /** Comfortably longer than a rotation, far shorter than a forgotten app. */ + val SHOW_WITHIN: Duration = 30.seconds + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSnackbar.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSnackbar.kt index a3f3881..c264c75 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSnackbar.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSnackbar.kt @@ -21,7 +21,9 @@ import kotlinx.coroutines.CancellationException * a pushed screen such as Settings, the activity finishing) counts as expired: * an Undo held back and offered again later would restore a snapshot from * minutes or hours ago, out of any context. A rotation is the one exception — - * the activity comes straight back and shows the snackbar again. + * the activity comes straight back and shows the snackbar again. A deletion + * that was offered while no screen was there to show it, and is [isStale] by + * the time one is, expires without a snackbar. */ @Composable internal fun UndoSnackbarEffect( @@ -30,6 +32,7 @@ internal fun UndoSnackbarEffect( message: String, onUndo: (T) -> Unit, onExpired: (T) -> Unit, + isStale: (T) -> Boolean = { false }, ) { val activity = LocalActivity.current val undoLabel = stringResource(R.string.action_undo) @@ -37,6 +40,10 @@ internal fun UndoSnackbarEffect( val currentOnExpired by rememberUpdatedState(onExpired) LaunchedEffect(pending) { val deleted = pending ?: return@LaunchedEffect + if (isStale(deleted)) { + currentOnExpired(deleted) + return@LaunchedEffect + } val result = try { host.showSnackbar(message = message, actionLabel = undoLabel, duration = SnackbarDuration.Long) } catch (cancelled: CancellationException) { diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimersScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimersScreen.kt index 7a9e6a2..c337195 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimersScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimersScreen.kt @@ -64,6 +64,7 @@ fun TimersScreen( message = undoMessage, onUndo = viewModel::onUndoDelete, onExpired = viewModel::onUndoExpired, + isStale = viewModel::isUndoStale, ) val loaded = state as? TimersUiState.Loaded diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimersViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimersViewModel.kt index 3e67c16..4998695 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimersViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimersViewModel.kt @@ -7,14 +7,14 @@ import de.jeanlucmakiola.clockula.data.prefs.SettingsPrefs import de.jeanlucmakiola.clockula.data.timers.TimerRepository import de.jeanlucmakiola.clockula.domain.Timer import de.jeanlucmakiola.clockula.domain.TimerDraft +import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock import de.jeanlucmakiola.clockula.domain.timer.TimerPresets import de.jeanlucmakiola.clockula.timer.TimerEngine +import de.jeanlucmakiola.clockula.ui.common.UndoSlot import de.jeanlucmakiola.floret.di.ApplicationScope import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch @@ -33,6 +33,7 @@ class TimersViewModel @Inject internal constructor( private val engine: TimerEngine, private val settings: SettingsPrefs, source: TimerListSource, + 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, @@ -86,21 +87,21 @@ class TimersViewModel @Inject internal constructor( fun onSubtractMinute(timerId: Long) = launchOnEngine { engine.addTime(timerId, -1.minutes) } - private val undoSlot = MutableStateFlow(null) + private val undo = UndoSlot(elapsed) /** * The deleted timer the snackbar offers to bring back. Null when none. Held * here, not in the screen, so a rotation re-shows the snackbar rather than * losing the Undo; a newer delete replaces it. */ - val pendingUndo: StateFlow = undoSlot.asStateFlow() + val pendingUndo: StateFlow = undo.pending /** * Through the engine, so a *ringing* timer cannot be deleted into a stuck * ring (D2). Still no confirmation; the snackbar's Undo now covers the slip. */ fun onDelete(timerId: Long) = launchOnEngine { - engine.delete(timerId)?.let { undoSlot.value = it } + engine.delete(timerId)?.let(undo::offer) } /** @@ -109,15 +110,18 @@ class TimersViewModel @Inject internal constructor( * chance, so a back press that clears this ViewModel must not drop it. */ fun onUndoDelete(timer: Timer) { - if (!undoSlot.compareAndSet(timer, null)) return + if (!undo.take(timer)) return appScope.launch { engine.restore(timer) } } /** The snackbar timed out or was swiped away: the delete stands. */ fun onUndoExpired(timer: Timer) { - undoSlot.compareAndSet(timer, null) + undo.take(timer) } + /** Offered too long ago to show a snackbar for now ([UndoSlot.isStale]). */ + fun isUndoStale(timer: Timer): Boolean = undo.isStale(timer) + fun onSavePreset(duration: Duration) { viewModelScope.launch { settings.addTimerPreset(duration) } } 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 5f5146b..c010e57 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 @@ -9,6 +9,7 @@ import de.jeanlucmakiola.clockula.domain.Ringtones import de.jeanlucmakiola.clockula.domain.TimeOfDay 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 @@ -51,7 +52,7 @@ 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, undo: AlarmUndo = AlarmUndo()) = + private fun TestScope.editor(harness: AlarmEngineHarness, alarmId: Long = 1L, undo: AlarmUndo = AlarmUndo(FakeElapsedRealtimeClock())) = AlarmEditorViewModel( savedStateHandle = SavedStateHandle(mapOf(AlarmRoutes.ARG_ALARM_ID to alarmId)), alarms = harness.alarms, @@ -662,7 +663,7 @@ class AlarmEditorViewModelTest { val harness = alarmEngineHarness(tempDir, now = fiveToday) harness.givenAlarmWith(hour = 7) harness.engine.reschedule() - val undo = AlarmUndo() + val undo = AlarmUndo(FakeElapsedRealtimeClock()) val viewModel = editor(harness, undo = undo) advanceUntilIdle() val before = harness.alarmDao.stored 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 9ca7e8b..0a712cc 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 @@ -46,7 +47,7 @@ class AlarmsViewModelTest { * `collectAsStateWithLifecycle`; without one there is no subscriber and * nothing upstream would run. */ - private fun TestScope.viewModel(harness: AlarmEngineHarness, undo: AlarmUndo = AlarmUndo()) = AlarmsViewModel( + private fun TestScope.viewModel(harness: AlarmEngineHarness, undo: AlarmUndo = AlarmUndo(FakeElapsedRealtimeClock())) = AlarmsViewModel( alarms = harness.alarms, engine = harness.engine, wall = harness.wallClock, diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/common/UndoSlotTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/common/UndoSlotTest.kt new file mode 100644 index 0000000..4912efa --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/common/UndoSlotTest.kt @@ -0,0 +1,40 @@ +package de.jeanlucmakiola.clockula.ui.common + +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.clockula.testing.FakeElapsedRealtimeClock +import org.junit.jupiter.api.Test +import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.seconds + +/** Claim once, newer replaces older, and an offer nobody saw in time goes stale. */ +class UndoSlotTest { + + private val clock = FakeElapsedRealtimeClock() + private val slot = UndoSlot(clock) + + @Test + fun `an offer is taken exactly once`() { + slot.offer("07:00") + + assertThat(listOf(slot.take("07:00"), slot.take("07:00"), slot.pending.value)) + .isEqualTo(listOf(true, false, null)) + } + + @Test + fun `a stale answer cannot take a newer offer`() { + slot.offer("07:00") + slot.offer("09:00") + + assertThat(slot.take("07:00") to slot.pending.value).isEqualTo(false to "09:00") + } + + @Test + fun `an offer is fresh across a rotation and stale long after`() { + slot.offer("07:00") + clock.advance(5.seconds) + val fresh = slot.isStale("07:00") + clock.advance(10.minutes) + + assertThat(fresh to slot.isStale("07:00")).isEqualTo(false to true) + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/timers/TimersViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/timers/TimersViewModelTest.kt index 0a6c0b6..1ca9b73 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/timers/TimersViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/timers/TimersViewModelTest.kt @@ -50,6 +50,7 @@ class TimersViewModelTest { engine = harness.engine, settings = harness.settings, source = TimerListSource(harness.timers, harness.elapsed, harness.wallClock, harness.ticker), + elapsed = harness.elapsed, appScope = this, ).also { viewModel -> backgroundScope.launch { viewModel.state.collect {} } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 33a736d..51cc86c 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1756,7 +1756,10 @@ replaces it, and when the screen is left with the snackbar unanswered (another tab, Settings, the app closed): an Undo held back and offered later would restore a snapshot from long ago, out of any context, and over a backup that was restored meanwhile. A rotation is the one exception and shows the snackbar -again. It is memory only, so process death drops it too. +again. A deletion offered while no list was on screen (the app sent home the +instant the editor's Delete was tapped) is dropped unshown if it is more than +30 seconds old by the time the list is back. Both lists share this as +`UndoSlot`. It is memory only, so process death drops it too. The swipe is hand-rolled (`ui/common/SwipeToDelete`) rather than M3's `SwipeToDismissBox`, because the tabs are pages of a `HorizontalPager` and the