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 561465d..a44912c 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -145,14 +145,20 @@ class AlarmEngine @Inject constructor( * otherwise outlive the row it belongs to. The ring state goes with the row * (the foreign key cascades). Non-cancellable, like [restore]: a caller * cancelled half-way must not leave the slot registered for a deleted row. + * + * [unlessRinging] is the list's delete: a swipe released just as the alarm + * fired, before the list knew, must not silence it round its dismiss + * challenge, so it writes nothing and returns null. Checked here, under the + * lock, because only here is the answer not already stale. */ - suspend fun delete(alarmId: Long): DeletedAlarm? = lock.withLock { - withContext(NonCancellable) { deleteLocked(alarmId) } + suspend fun delete(alarmId: Long, unlessRinging: Boolean = false): DeletedAlarm? = lock.withLock { + withContext(NonCancellable) { deleteLocked(alarmId, unlessRinging) } } - private suspend fun deleteLocked(alarmId: Long): DeletedAlarm? { + private suspend fun deleteLocked(alarmId: Long, unlessRinging: Boolean): DeletedAlarm? { val alarm = alarms.find(alarmId) ?: return null val state = states.state(alarmId) + if (unlessRinging && state.isRinging) return null silenceLocked(alarmId, state) alarms.delete(alarmId) rescheduleLocked() 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 b96a451..fcee832 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 @@ -84,6 +84,7 @@ fun AlarmsScreen( onUndo = viewModel::onUndoDelete, onExpired = viewModel::onUndoExpired, isStale = viewModel::isUndoStale, + onShown = viewModel::onUndoShown, ) 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 5f77fd3..fe51639 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 @@ -91,7 +91,9 @@ class AlarmsViewModel @Inject internal constructor( * delete the user meant. */ fun onDelete(alarmId: Long) { - viewModelScope.launch { engine.delete(alarmId)?.let(undo::offer) } + // Never a ringing alarm: the card leads to its ring screen then, and the + // engine re-checks under its lock in case the ring began mid-swipe. + viewModelScope.launch { engine.delete(alarmId, unlessRinging = true)?.let(undo::offer) } } /** @@ -112,6 +114,9 @@ class AlarmsViewModel @Inject internal constructor( /** Offered too long ago to show a snackbar for now ([AlarmUndo.isStale]). */ fun isUndoStale(deleted: DeletedAlarm): Boolean = undo.isStale(deleted) + /** The snackbar is up; only its answer settles the deletion now. */ + fun onUndoShown(deleted: DeletedAlarm) = undo.markShown(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 0d2dda1..fc7f79d 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 @@ -138,7 +138,9 @@ internal fun SwipeToDelete( } scope.launch { if (width > 0 && delete) { - travel.animateTo(width.toFloat(), settleSpec) + // Past its own width: the card sits inset from the + // edge, and stopping at its width would leave a sliver. + travel.animateTo(width * ThrowFraction, settleSpec) // Re-read after the animation: the card may have been // disabled meanwhile (its alarm started ringing). if (currentEnabled) currentOnDelete() @@ -179,6 +181,9 @@ internal fun SwipeToDelete( } } +/** How far a deleted card is thrown, as a share of its width, to clear the screen edge. */ +private const val ThrowFraction = 1.25f + /** Past this share of the card's width, letting go deletes. */ private const val DeleteFraction = 0.4f 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 index c4a91b0..b314020 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSlot.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSlot.kt @@ -21,12 +21,17 @@ open class UndoSlot(private val clock: ElapsedRealtimeClock) { @Volatile private var offeredAt: Duration = Duration.ZERO + /** The offer a snackbar has already been put up for, which can no longer go stale. */ + @Volatile + private var shown: T? = 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: T) { offeredAt = clock.elapsedRealtime() + shown = null slot.value = deleted } @@ -41,10 +46,19 @@ open class UndoSlot(private val clock: ElapsedRealtimeClock) { * 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. + * hours later, its Undo would restore a snapshot out of any context. Never + * true once [markShown]. */ fun isStale(deleted: T): Boolean = - slot.value == deleted && clock.elapsedRealtime() - offeredAt > SHOW_WITHIN + slot.value == deleted && shown != deleted && clock.elapsedRealtime() - offeredAt > SHOW_WITHIN + + /** + * A snackbar is up for [deleted]. From now on only its answer settles it, so + * a rotation after a long accessibility timeout still shows it again. + */ + fun markShown(deleted: T) { + if (slot.value == deleted) shown = deleted + } private companion object { /** Comfortably longer than a rotation, far shorter than a forgotten app. */ 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 c264c75..adf26e2 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 @@ -32,7 +32,8 @@ internal fun UndoSnackbarEffect( message: String, onUndo: (T) -> Unit, onExpired: (T) -> Unit, - isStale: (T) -> Boolean = { false }, + isStale: (T) -> Boolean, + onShown: (T) -> Unit, ) { val activity = LocalActivity.current val undoLabel = stringResource(R.string.action_undo) @@ -44,6 +45,7 @@ internal fun UndoSnackbarEffect( currentOnExpired(deleted) return@LaunchedEffect } + onShown(deleted) 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 c337195..f01c9cb 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 @@ -65,6 +65,7 @@ fun TimersScreen( onUndo = viewModel::onUndoDelete, onExpired = viewModel::onUndoExpired, isStale = viewModel::isUndoStale, + onShown = viewModel::onUndoShown, ) 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 4998695..234130a 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 @@ -122,6 +122,9 @@ class TimersViewModel @Inject internal constructor( /** Offered too long ago to show a snackbar for now ([UndoSlot.isStale]). */ fun isUndoStale(timer: Timer): Boolean = undo.isStale(timer) + /** The snackbar is up; only its answer settles the deletion now. */ + fun onUndoShown(timer: Timer) = undo.markShown(timer) + fun onSavePreset(duration: Duration) { viewModelScope.launch { settings.addTimerPreset(duration) } } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineDeleteTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineDeleteTest.kt index d1cca3b..b1756c1 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineDeleteTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineDeleteTest.kt @@ -89,6 +89,18 @@ class AlarmEngineDeleteTest { .isEqualTo(listOf(RingEvent.Stopped(1L), null, null)) } + @Test + fun `the list's delete leaves a ringing alarm ringing`(@TempDir dir: Path) = runTest { + val harness = alarmEngineHarness(dir) + harness.givenAlarm(id = 1L, hour = 7) + harness.givenRinging(alarmId = 1L, at = sevenToday) + + val deleted = harness.engine.delete(1L, unlessRinging = true) + + assertThat(listOf(deleted, harness.ring.ringing.value, harness.alarms.find(1L)?.id)) + .isEqualTo(listOf(null, 1L, 1L)) + } + @Test fun `a ringing one-shot comes back switched off and silent`(@TempDir dir: Path) = runTest { val harness = alarmEngineHarness(dir) 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 0a712cc..fcb246a 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 @@ -424,7 +424,7 @@ class AlarmsViewModelTest { } @Test - fun `swiping away the ringing alarm silences it, and undo does not ring it again`(@TempDir tempDir: Path) = + fun `a ringing alarm cannot be swiped away round its ring screen`(@TempDir tempDir: Path) = runTest(main.dispatcher) { val harness = alarmEngineHarness(tempDir) harness.givenAlarm(id = 1L, hour = 7, repeatDays = RepeatDays.EVERY_DAY) @@ -434,17 +434,9 @@ class AlarmsViewModelTest { viewModel.onDelete(1L) advanceUntilIdle() - viewModel.onUndoDelete(checkNotNull(viewModel.pendingUndo.value)) - advanceUntilIdle() - assertThat( - listOf( - harness.ring.events.last(), - harness.ring.ringing.value, - harness.scheduler.autoSilence, - harness.scheduler.next?.fireAt, - ), - ).isEqualTo(listOf(RingEvent.Stopped(1L), null, null, offsetInstant("2026-06-11T07:00+02:00"))) + assertThat(listOf(harness.ring.ringing.value, harness.alarms.find(1L)?.id, viewModel.pendingUndo.value)) + .isEqualTo(listOf(1L, 1L, null)) } // --- M9: the AlarmClock contract's dismissal, and the chooser dialog's --- 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 index 4912efa..20dc526 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/common/UndoSlotTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/common/UndoSlotTest.kt @@ -37,4 +37,13 @@ class UndoSlotTest { assertThat(fresh to slot.isStale("07:00")).isEqualTo(false to true) } + + @Test + fun `an offer already on screen never goes stale`() { + slot.offer("07:00") + slot.markShown("07:00") + clock.advance(10.minutes) + + assertThat(slot.isStale("07:00")).isFalse() + } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 51cc86c..7b42473 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1720,7 +1720,9 @@ An alarm is deleted by swiping its card towards the end edge, by the card's access, where there is no swipe), or from the editor's top bar. None of them asks first. A *ringing* card offers neither the swipe nor the action: it leads to the ring screen, and a swipe must not become a way round its dismiss -challenge. Each is followed by a snackbar on the list ("07:00 alarm deleted", +challenge. The engine re-checks under its lock (`delete(id, unlessRinging = true)`), +so a swipe released just as the alarm fires, before the list has caught up, +deletes nothing and the card slides back. Each is followed by a snackbar on the list ("07:00 alarm deleted", **Undo**), as in Google Clock. v1 asked before instead, on the argument that a mistaken delete is found at 07:00 rather than now. But a dialog catches no more than the snackbar does: both only catch the slip noticed in the moment, and a