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