From 2feed2ee3537aa9d5bafcfbd01b334c2e245a8d2 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 20:02:00 +0200 Subject: [PATCH 01/11] feat(alarms): swipe to delete, with Undo An alarm card swipes away towards the end edge, or deletes from its "Delete" accessibility action, and the list's snackbar offers Undo. The editor's Delete drops its confirmation for the same Undo, routed through a singleton so the list can answer it after the editor pops. Timer cards get the same Undo for their corner delete. AlarmEngine.delete stops a ringing alarm and clears a snoozed one's notification under the engine lock; restore puts the row back under its own id with its ring state and reschedules. A ringing alarm comes back dismissed, never ringing; a finished timer comes back reset. The swipe is hand-rolled so the end-to-start drag stays with the tab pager. Fixes #20 --- CHANGELOG.md | 4 + .../clockula/ui/AlarmsInstrumentedTest.kt | 25 ++- .../clockula/alarm/AlarmEngine.kt | 43 +++++ .../clockula/data/alarms/AlarmDao.kt | 5 + .../clockula/data/alarms/AlarmRepository.kt | 8 + .../data/alarms/AlarmRepositoryImpl.kt | 3 + .../clockula/data/timers/TimerDao.kt | 5 + .../clockula/data/timers/TimerRepository.kt | 7 + .../data/timers/TimerRepositoryImpl.kt | 3 + .../clockula/domain/alarm/AlarmDeletion.kt | 36 +++++ .../clockula/domain/timer/TimerDeletion.kt | 36 +++++ .../clockula/timer/TimerEngine.kt | 20 ++- .../clockula/ui/alarms/AlarmEditorScreen.kt | 39 +---- .../ui/alarms/AlarmEditorViewModel.kt | 17 +- .../clockula/ui/alarms/AlarmRow.kt | 114 ++++++++------ .../clockula/ui/alarms/AlarmUndo.kt | 42 +++++ .../clockula/ui/alarms/AlarmsScreen.kt | 31 ++++ .../clockula/ui/alarms/AlarmsViewModel.kt | 27 +++- .../clockula/ui/common/SwipeToDelete.kt | 147 ++++++++++++++++++ .../clockula/ui/timers/TimerRow.kt | 7 +- .../clockula/ui/timers/TimersScreen.kt | 34 +++- .../clockula/ui/timers/TimersViewModel.kt | 32 +++- app/src/main/res/values/strings.xml | 5 +- .../clockula/alarm/AlarmEngineDeleteTest.kt | 129 +++++++++++++++ .../domain/alarm/AlarmDeletionTest.kt | 64 ++++++++ .../domain/timer/TimerDeletionTest.kt | 53 +++++++ .../clockula/testing/FakeAlarmSeams.kt | 1 + .../clockula/testing/FakeDaos.kt | 6 + .../clockula/testing/FakeRepositories.kt | 7 + .../ui/alarms/AlarmEditorViewModelTest.kt | 24 ++- .../clockula/ui/alarms/AlarmsViewModelTest.kt | 101 +++++++++++- .../clockula/ui/timers/TimersViewModelTest.kt | 37 +++++ docs/ARCHITECTURE.md | 71 ++++++++- docs/ROADMAP.md | 5 +- 34 files changed, 1074 insertions(+), 114 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmDeletion.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerDeletion.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmUndo.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/ui/common/SwipeToDelete.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineDeleteTest.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmDeletionTest.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/clockula/domain/timer/TimerDeletionTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 76d4d0d..397f0f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ 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. +- Swipe an alarm away to delete it, or use its Delete action with a screen + reader. Deleting an alarm, or a timer from its card, doesn't ask first: + Undo brings it back with all its settings, and a snoozed alarm comes back + snoozed. - 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/androidTest/java/de/jeanlucmakiola/clockula/ui/AlarmsInstrumentedTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/clockula/ui/AlarmsInstrumentedTest.kt index c882f88..0ec1af1 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/clockula/ui/AlarmsInstrumentedTest.kt +++ b/app/src/androidTest/java/de/jeanlucmakiola/clockula/ui/AlarmsInstrumentedTest.kt @@ -3,12 +3,14 @@ package de.jeanlucmakiola.clockula.ui import android.content.Context import android.content.Intent import android.text.format.DateFormat +import androidx.compose.ui.test.ExperimentalTestApi import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.assertIsOff import androidx.compose.ui.test.junit4.v2.createEmptyComposeRule import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performCustomAccessibilityActionWithLabel import androidx.compose.ui.test.performTextReplacement import androidx.test.core.app.ActivityScenario import androidx.test.espresso.Espresso @@ -205,12 +207,31 @@ class AlarmsInstrumentedTest { composeTestRule.onNodeWithText("Work").performClick() composeTestRule.waitForIdle() + // No confirmation: the list underneath offers the Undo instead. composeTestRule.onNodeWithContentDescription(string(R.string.editor_delete)).performClick() composeTestRule.waitForIdle() - composeTestRule.onNodeWithText(string(R.string.action_delete)).performClick() - composeTestRule.waitForIdle() composeTestRule.onNodeWithText("Work").assertDoesNotExist() + composeTestRule.onNodeWithText(string(R.string.action_undo)).assertIsDisplayed() assertThat(stored(id)).isNull() } + + @OptIn(ExperimentalTestApi::class) + @Test + fun theRowsDeleteActionRemovesItAndUndoBringsItBack() { + val id = givenAlarm() + launch() + composeTestRule.waitForIdle() + + composeTestRule.onNodeWithText("Work") + .performCustomAccessibilityActionWithLabel(string(R.string.action_delete)) + composeTestRule.waitForIdle() + assertThat(stored(id)).isNull() + + composeTestRule.onNodeWithText(string(R.string.action_undo)).performClick() + composeTestRule.waitForIdle() + + composeTestRule.onNodeWithText("Work").assertIsDisplayed() + assertThat(stored(id)).isNotNull() + } } 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..82317c3 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -5,10 +5,12 @@ import de.jeanlucmakiola.clockula.data.alarms.AlarmStateRepository import de.jeanlucmakiola.clockula.data.prefs.SettingsPrefs import de.jeanlucmakiola.clockula.domain.Alarm import de.jeanlucmakiola.clockula.domain.AlarmSettings +import de.jeanlucmakiola.clockula.domain.alarm.AlarmDeletion import de.jeanlucmakiola.clockula.domain.alarm.AlarmResolution import de.jeanlucmakiola.clockula.domain.alarm.AlarmResolver import de.jeanlucmakiola.clockula.domain.alarm.AlarmRing import de.jeanlucmakiola.clockula.domain.alarm.AlarmRingState +import de.jeanlucmakiola.clockula.domain.alarm.DeletedAlarm import de.jeanlucmakiola.clockula.domain.alarm.DismissOutcome import de.jeanlucmakiola.clockula.domain.alarm.FireOutcome import de.jeanlucmakiola.clockula.domain.alarm.FireSource @@ -130,6 +132,47 @@ class AlarmEngine @Inject constructor( suspend fun dismiss(alarmId: Long): Unit = lock.withLock { closeCycleLocked(alarmId, states.state(alarmId)) } + /** + * The user's delete, from the list's swipe or the editor. Returns what an + * Undo would put back ([AlarmDeletion.snapshot]), or null when there is + * nothing to put back. + * + * On the engine, under its lock, for the same reason as [dismissUpcoming]: + * a ringing alarm's service and auto-silence backstop are global slots + * only the engine may touch, and a snoozed alarm's notification would + * otherwise outlive the row it belongs to. The ring state goes with the row + * (the foreign key cascades). + */ + suspend fun delete(alarmId: Long): DeletedAlarm? = lock.withLock { + val alarm = alarms.find(alarmId) ?: return@withLock null + val state = states.state(alarmId) + if (state.ringingSince != null) { + ring.stopRinging(alarmId) + scheduler.cancelAutoSilence() + } + notifier.clearSnoozed(alarmId) + alarms.delete(alarmId) + rescheduleLocked() + AlarmDeletion.snapshot(alarm, state) + } + + /** + * The Undo: the row back under its own id, its ring state with it, and the + * slot re-resolved, which is what re-registers it with AlarmManager. False + * when the id has been taken since (a backup restore in between). + * + * A snooze comes back with its notification, but only if the resolver + * still holds it pending once the slot is re-resolved: one that lapsed + * while the alarm was gone is closed there, like any other. + */ + suspend fun restore(deleted: DeletedAlarm): Boolean = lock.withLock { + if (!alarms.restore(deleted.alarm)) return@withLock false + if (deleted.state != AlarmRingState.initial(deleted.alarm.id)) states.save(deleted.state) + rescheduleLocked() + states.state(deleted.alarm.id).snoozedUntil?.let { until -> notifier.showSnoozed(deleted.alarm, until) } + true + } + /** Boot repair (timers + stopwatch), then resume any interrupted ring, then reschedule. */ suspend fun onBootCompleted(): ScheduledFire? = lock.withLock { rebootRepair.repairIfRebooted() diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmDao.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmDao.kt index e638962..d8819b4 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmDao.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmDao.kt @@ -2,6 +2,7 @@ package de.jeanlucmakiola.clockula.data.alarms import androidx.room.Dao import androidx.room.Insert +import androidx.room.OnConflictStrategy import androidx.room.Query import androidx.room.Transaction import androidx.room.Update @@ -24,6 +25,10 @@ abstract class AlarmDao { @Insert abstract suspend fun insert(alarm: AlarmEntity): Long + /** Inserts with the row's own id; -1 when that id is taken, which writes nothing. */ + @Insert(onConflict = OnConflictStrategy.IGNORE) + abstract suspend fun insertIfAbsent(alarm: AlarmEntity): Long + @Update abstract suspend fun update(alarm: AlarmEntity) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmRepository.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmRepository.kt index d1b683c..05b92d7 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmRepository.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmRepository.kt @@ -12,6 +12,14 @@ interface AlarmRepository { suspend fun create(draft: AlarmDraft): Long suspend fun update(alarm: Alarm) suspend fun delete(id: Long) + + /** + * Puts a deleted alarm back under its own id, timestamps and all — the + * Undo after a delete. False when that id is taken (a backup restore + * landed in between), in which case nothing is written. `alarms.id` is + * `AUTOINCREMENT`, so a new alarm can never have claimed it. + */ + suspend fun restore(alarm: Alarm): Boolean suspend fun setEnabled(id: Long, enabled: Boolean) suspend fun setSkipNextOccurrence(id: Long, skip: Boolean) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmRepositoryImpl.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmRepositoryImpl.kt index 25f0656..950009e 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmRepositoryImpl.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmRepositoryImpl.kt @@ -36,6 +36,9 @@ class AlarmRepositoryImpl @Inject constructor( override suspend fun delete(id: Long) = dao.deleteById(id) + override suspend fun restore(alarm: Alarm): Boolean = + dao.insertIfAbsent(AlarmMapper.toEntity(alarm)) != -1L + override suspend fun setEnabled(id: Long, enabled: Boolean) = dao.setEnabled(id, enabled, wallClock.now().toEpochMilliseconds()) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerDao.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerDao.kt index 3e93e70..dadd79e 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerDao.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerDao.kt @@ -2,6 +2,7 @@ package de.jeanlucmakiola.clockula.data.timers import androidx.room.Dao import androidx.room.Insert +import androidx.room.OnConflictStrategy import androidx.room.Query import androidx.room.Transaction import androidx.room.Update @@ -27,6 +28,10 @@ abstract class TimerDao { @Insert abstract suspend fun insert(timer: TimerEntity): Long + /** Inserts with the row's own id; -1 when that id is taken, which writes nothing. */ + @Insert(onConflict = OnConflictStrategy.IGNORE) + abstract suspend fun insertIfAbsent(timer: TimerEntity): Long + @Update abstract suspend fun update(timer: TimerEntity) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepository.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepository.kt index 8107091..515a0e7 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepository.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepository.kt @@ -13,6 +13,13 @@ interface TimerRepository { suspend fun rename(id: Long, label: String) suspend fun delete(id: Long) + /** + * Puts a deleted timer back under its own id, anchors and place in the + * list — the Undo after a delete. False when that id is taken (a backup + * restore landed in between), in which case nothing is written. + */ + suspend fun restore(timer: Timer): Boolean + /** Starts an IDLE/EXPIRED timer from its configured duration, resumes a PAUSED one. No-op when RUNNING. */ suspend fun start(id: Long) suspend fun pause(id: Long) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepositoryImpl.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepositoryImpl.kt index 8d5d4e5..eaeaaec 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepositoryImpl.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepositoryImpl.kt @@ -43,6 +43,9 @@ class TimerRepositoryImpl @Inject constructor( override suspend fun delete(id: Long) = dao.deleteById(id) + override suspend fun restore(timer: Timer): Boolean = + dao.insertIfAbsent(TimerMapper.toEntity(timer)) != -1L + override suspend fun start(id: Long) { val now = elapsedRealtimeClock.elapsedRealtime() val wall = wallClock.now() diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmDeletion.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmDeletion.kt new file mode 100644 index 0000000..2df40b9 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmDeletion.kt @@ -0,0 +1,36 @@ +package de.jeanlucmakiola.clockula.domain.alarm + +import de.jeanlucmakiola.clockula.domain.Alarm + +/** What an Undo puts back: the alarm's row and its ring state, as of the delete. */ +data class DeletedAlarm(val alarm: Alarm, val state: AlarmRingState) + +/** + * What a deleted alarm comes back as. Pure, so the "no stale ring" rule is + * decided once, here, and the engine only carries it out. + */ +object AlarmDeletion { + + /** + * The snapshot an Undo restores, or null when there is nothing to restore. + * + * Everything the user set comes back as it was (label, sound, challenge, + * skip-next), and so does the ring state: a snoozed alarm is snoozed again, + * and the handled-occurrence watermark still stops the occurrence just + * closed from ringing twice. + * + * A *ringing* alarm is the exception. Deleting it stopped the ring, which is + * a dismissal in all but name, so it comes back as a dismissal would have + * left it: cycle closed, a one-shot switched off, never ringing again. A + * transient one-shot (M9 D10) would have been deleted by that dismissal + * anyway, so there is nothing to undo. + */ + fun snapshot(alarm: Alarm, state: AlarmRingState): DeletedAlarm? { + if (!state.isRinging) return DeletedAlarm(alarm, state) + + val closed = state.copy(ringingSince = null, snoozedUntil = null, snoozeCount = 0) + if (alarm.repeatDays.isRepeating) return DeletedAlarm(alarm, closed) + if (alarm.deleteAfterUse) return null + return DeletedAlarm(alarm.copy(enabled = false), closed) + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerDeletion.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerDeletion.kt new file mode 100644 index 0000000..3348515 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerDeletion.kt @@ -0,0 +1,36 @@ +package de.jeanlucmakiola.clockula.domain.timer + +import de.jeanlucmakiola.clockula.domain.Timer +import de.jeanlucmakiola.clockula.domain.TimerState +import de.jeanlucmakiola.clockula.domain.snapshotAt +import kotlin.time.Duration +import kotlin.time.Instant + +/** What a deleted timer comes back as when the user taps Undo. Pure. */ +object TimerDeletion { + + /** + * The row an Undo restores, or null when there is nothing to restore. + * + * A running or paused timer comes back exactly as it was. Its anchors are + * on the monotonic clock, so a running one has kept counting down while it + * was gone, and one whose end passed in the meantime rings on restore, as + * it would have done had it never been deleted. + * + * A timer that had already *finished* is the exception. Deleting it + * silenced it, which is a dismissal, so it comes back reset rather than + * ringing a second time. A transient one (M9 D10) would have been deleted + * by that dismissal anyway, so there is nothing to undo. + */ + fun snapshot(timer: Timer, elapsedRealtime: Duration, wallClock: Instant): Timer? { + if (!timer.snapshotAt(elapsedRealtime, wallClock).hasExpired) return timer + if (timer.deleteAfterUse) return null + return timer.copy( + state = TimerState.IDLE, + remaining = timer.duration, + startedAtElapsedRealtime = null, + endsAtElapsedRealtime = null, + endsAtWallClock = null, + ) + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/timer/TimerEngine.kt b/app/src/main/java/de/jeanlucmakiola/clockula/timer/TimerEngine.kt index 04b27cc..71f8155 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/timer/TimerEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/timer/TimerEngine.kt @@ -3,9 +3,11 @@ package de.jeanlucmakiola.clockula.timer import de.jeanlucmakiola.clockula.data.prefs.SettingsPrefs import de.jeanlucmakiola.clockula.data.timers.TimerRepository import de.jeanlucmakiola.clockula.data.timers.TimerRingStateStore +import de.jeanlucmakiola.clockula.domain.Timer import de.jeanlucmakiola.clockula.domain.TimerState import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock import de.jeanlucmakiola.clockula.domain.time.WallClock +import de.jeanlucmakiola.clockula.domain.timer.TimerDeletion import de.jeanlucmakiola.clockula.domain.timer.TimerExpiry import de.jeanlucmakiola.clockula.domain.timer.TimerNotificationPolicy import de.jeanlucmakiola.clockula.domain.timer.TimerNotificationState @@ -181,10 +183,26 @@ class TimerEngine @Inject constructor( * Deletes, then resyncs — so a *ringing* timer cannot be deleted into a * stuck ring (D2). It is on the engine and not on a ViewModel-plus-resync * path deliberately: the wrong call should be impossible to write. + * + * Returns what an Undo would put back ([TimerDeletion.snapshot]), or null + * when there is nothing to put back. */ - suspend fun delete(timerId: Long): Unit = lock.withLock { + suspend fun delete(timerId: Long): Timer? = lock.withLock { + val timer = timers.find(timerId) timers.delete(timerId) resyncLocked() + timer?.let { TimerDeletion.snapshot(it, elapsed.elapsedRealtime(), wall.now()) } + } + + /** + * The Undo: the row back under its own id, then the same resync every verb + * ends with, which hands the expiry slot and the service the restored timer. + * False when the id has been taken since (a backup restore in between). + */ + suspend fun restore(timer: Timer): Boolean = lock.withLock { + if (!timers.restore(timer)) return@withLock false + resyncLocked() + true } /** 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..1b3fa7a 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 @@ -7,13 +7,11 @@ import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.material.icons.Icons import androidx.compose.material.icons.rounded.Delete -import androidx.compose.material3.AlertDialog import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Switch import androidx.compose.material3.Text -import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue @@ -57,8 +55,6 @@ fun AlarmEditorScreen( val state by viewModel.state.collectAsStateWithLifecycle() val picker by viewModel.ringtonePicker.collectAsStateWithLifecycle() val previewPlaying by viewModel.previewPlaying.collectAsStateWithLifecycle() - var confirmingDelete by rememberSaveable { mutableStateOf(false) } - val formatTime = rememberAlarmTimeFormatter() // Registered unconditionally, and deliberately **not** inside the // `picker?.let` below: the picker's open flag is ViewModel state that does @@ -86,7 +82,8 @@ fun AlarmEditorScreen( predictiveBack = true, actions = { if (state is AlarmEditorUiState.Editing) { - IconButton(onClick = { confirmingDelete = true }) { + // No confirmation: the list underneath offers an Undo instead. + IconButton(onClick = viewModel::onDelete) { Icon( Icons.Rounded.Delete, contentDescription = stringResource(R.string.editor_delete), @@ -103,18 +100,6 @@ fun AlarmEditorScreen( ) } - val editing = state as? AlarmEditorUiState.Editing - if (confirmingDelete && editing != null) { - DeleteConfirmation( - time = formatTime(editing.time), - onConfirm = { - confirmingDelete = false - viewModel.onDelete() - }, - onDismiss = { confirmingDelete = false }, - ) - } - picker?.let { pickerState -> val editing = state as? AlarmEditorUiState.Editing RingtonePickerScreen( @@ -325,23 +310,3 @@ private fun SoundRow( onClick = onClick, ) } - -/** - * A confirmation that names the alarm, deliberately instead of an undo chip: - * an accidentally deleted alarm is discovered at 07:00, not now, so the cheap - * moment to ask is before (M5 D29). - */ -@Composable -private fun DeleteConfirmation(time: String, onConfirm: () -> Unit, onDismiss: () -> Unit) { - AlertDialog( - onDismissRequest = onDismiss, - title = { Text(stringResource(R.string.editor_delete_title)) }, - text = { Text(stringResource(R.string.editor_delete_message, time)) }, - confirmButton = { - TextButton(onClick = onConfirm) { Text(stringResource(R.string.action_delete)) } - }, - dismissButton = { - TextButton(onClick = onDismiss) { Text(stringResource(R.string.action_cancel)) } - }, - ) -} 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..7605bf1 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 @@ -41,6 +41,7 @@ class AlarmEditorViewModel @Inject internal constructor( private val engine: AlarmEngine, private val ringtones: RingtoneCatalog, private val preview: RingtonePreviewer, + private val undo: AlarmUndo, // `@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, @@ -206,12 +207,18 @@ class AlarmEditorViewModel @Inject internal constructor( viewModelScope.launch { preview.play(sound) } } - /** Dismisses a ring first (M5 D11), deletes, and the state becomes [AlarmEditorUiState.Gone]. */ + /** + * Deletes through the engine, which silences a ring first (M5 D11), and the + * state becomes [AlarmEditorUiState.Gone]. No confirmation: the editor pops + * and the list underneath offers the Undo, through [AlarmUndo] because this + * ViewModel is about to be cleared. + */ fun onDelete() { - viewModelScope.launch { - if (engine.ringSession()?.alarm?.id == alarmId) engine.dismiss(alarmId) - alarms.delete(alarmId) - engine.reschedule() + // On the application scope: a back press right after the tap clears + // this ViewModel, and a delete cut off half-way would leave the slot + // registered for an alarm that is no longer there. + appScope.launch { + engine.delete(alarmId)?.let(undo::offer) _state.value = AlarmEditorUiState.Gone } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt index c47eb5a..7e3752b 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt @@ -5,6 +5,7 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Switch import androidx.compose.material3.Text @@ -15,7 +16,9 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalConfiguration import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.CustomAccessibilityAction import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.customActions import androidx.compose.ui.semantics.semantics import androidx.compose.ui.text.AnnotatedString import androidx.compose.ui.text.SpanStyle @@ -29,6 +32,7 @@ import de.jeanlucmakiola.clockula.domain.RepeatDays import de.jeanlucmakiola.clockula.domain.RepeatDaysSummary import de.jeanlucmakiola.clockula.domain.RepeatSummary import de.jeanlucmakiola.clockula.domain.format.NextFireLabel +import de.jeanlucmakiola.clockula.ui.common.SwipeToDelete import de.jeanlucmakiola.clockula.ui.common.rememberAlarmTimeFormatter import de.jeanlucmakiola.floret.components.GroupedListInset import de.jeanlucmakiola.floret.components.GroupedSurface @@ -41,17 +45,23 @@ import java.util.Locale /** * A standalone alarm card. The time and switch share the first line, leaving * room below for the label and schedule without squeezing either beside the time. + * + * Swiping it towards the end edge deletes it, and so does the card's "Delete" + * custom action, which is the same verb for a screen reader or switch access, + * where a swipe is not to be had. The list's snackbar offers the Undo. */ @Composable internal fun AlarmRow( row: AlarmRowState, onClick: () -> Unit, onToggle: (Boolean) -> Unit, + onDelete: () -> Unit, modifier: Modifier = Modifier, ) { val formatTime = rememberAlarmTimeFormatter() val time = formatTime(row.time) val toggleDescription = stringResource(R.string.alarm_row_toggle, time) + val deleteAction = stringResource(R.string.action_delete) val textColor = if (row.enabled) { MaterialTheme.colorScheme.onSurface } else { @@ -62,61 +72,77 @@ internal fun AlarmRow( } else { MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.38f) } - GroupedSurface( - position = Position.Alone, + SwipeToDelete( + onDelete = onDelete, + shape = RoundedCornerShape(SwipeBackgroundCorner), modifier = modifier.padding(horizontal = GroupedListInset), - onClick = onClick, - color = if (row.enabled) { - MaterialTheme.colorScheme.surfaceContainerHigh - } else { - MaterialTheme.colorScheme.surfaceContainer - }, ) { - Column( - modifier = Modifier.fillMaxWidth().padding(horizontal = 20.dp, vertical = 20.dp), - verticalArrangement = Arrangement.spacedBy(8.dp), + GroupedSurface( + position = Position.Alone, + modifier = Modifier.semantics { + customActions = listOf( + CustomAccessibilityAction(deleteAction) { + onDelete() + true + }, + ) + }, + onClick = onClick, + color = if (row.enabled) { + MaterialTheme.colorScheme.surfaceContainerHigh + } else { + MaterialTheme.colorScheme.surfaceContainer + }, ) { - Row( - modifier = Modifier.fillMaxWidth(), - horizontalArrangement = Arrangement.spacedBy(16.dp), - verticalAlignment = Alignment.CenterVertically, + Column( + modifier = Modifier.fillMaxWidth().padding(horizontal = 20.dp, vertical = 20.dp), + verticalArrangement = Arrangement.spacedBy(8.dp), ) { + Row( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.spacedBy(16.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + text = time, + style = MaterialTheme.typography.displaySmall, + fontWeight = FontWeight.Bold, + color = textColor, + modifier = Modifier.weight(1f), + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + // Keep the switch live and fully visible even when the alarm is off. + Switch( + checked = row.enabled, + onCheckedChange = onToggle, + // "Alarm at 07:00" — a bare switch announces neither which row + // it belongs to nor what it does. + modifier = Modifier.semantics { contentDescription = toggleDescription }, + ) + } + if (row.label.isNotBlank()) { + Text( + text = row.label, + style = MaterialTheme.typography.bodyLarge, + color = textColor, + maxLines = 2, + overflow = TextOverflow.Ellipsis, + ) + } Text( - text = time, - style = MaterialTheme.typography.displaySmall, - fontWeight = FontWeight.Bold, - color = textColor, - modifier = Modifier.weight(1f), - maxLines = 1, - overflow = TextOverflow.Ellipsis, - ) - // Keep the switch live and fully visible even when the alarm is off. - Switch( - checked = row.enabled, - onCheckedChange = onToggle, - // "Alarm at 07:00" — a bare switch announces neither which row - // it belongs to nor what it does. - modifier = Modifier.semantics { contentDescription = toggleDescription }, + text = rowSummary(row), + style = MaterialTheme.typography.bodyMedium, + color = supportingColor, ) } - if (row.label.isNotBlank()) { - Text( - text = row.label, - style = MaterialTheme.typography.bodyLarge, - color = textColor, - maxLines = 2, - overflow = TextOverflow.Ellipsis, - ) - } - Text( - text = rowSummary(row), - style = MaterialTheme.typography.bodyMedium, - color = supportingColor, - ) } } } +/** The card's resting corner, so the red behind a swipe has the card's outline. */ +private val SwipeBackgroundCorner = 22.dp + @Composable private fun rowSummary(row: AlarmRowState): AnnotatedString { val primary = MaterialTheme.colorScheme.primary 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 new file mode 100644 index 0000000..63e04e8 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmUndo.kt @@ -0,0 +1,42 @@ +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 javax.inject.Inject +import javax.inject.Singleton + +/** + * The one deleted alarm an Undo can still bring back. + * + * A singleton rather than ViewModel state, because the two places that delete + * are not the place that offers the Undo. The editor is popped the moment its + * alarm is gone, so its ViewModel is cleared before any snackbar could be + * answered; the list underneath shows the snackbar instead, and reads the + * deletion from here. Held until the snackbar is answered or times out, so a + * rotation or a trip to another tab re-shows it rather than losing it. + * + * 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) +} 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..d630a79 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,15 @@ import androidx.compose.material3.FloatingActionButton import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.SnackbarDuration +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState +import androidx.compose.material3.SnackbarResult 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 @@ -26,6 +31,7 @@ import androidx.compose.ui.unit.dp import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.clockula.R +import de.jeanlucmakiola.clockula.ui.common.rememberAlarmTimeFormatter import de.jeanlucmakiola.clockula.ui.ring.AlarmRingActivity import de.jeanlucmakiola.clockula.ui.shell.LocalLivePillInset import de.jeanlucmakiola.floret.components.CollapsingScaffold @@ -59,6 +65,11 @@ fun AlarmsScreen( val context = LocalContext.current val pillInset = LocalLivePillInset.current val addAlarm = stringResource(R.string.alarms_add) + val pendingUndo by viewModel.pendingUndo.collectAsStateWithLifecycle() + val snackbar = remember { SnackbarHostState() } + val formatTime = rememberAlarmTimeFormatter() + val undoMessage = pendingUndo?.let { stringResource(R.string.alarm_deleted, formatTime(it.alarm.time)) } + val undoLabel = stringResource(R.string.action_undo) LaunchedEffect(viewModel) { // The FAB creates the alarm and the editor edits a persisted row, so @@ -66,10 +77,28 @@ fun AlarmsScreen( viewModel.openEditor.collectLatest(onCreateAlarm) } + // Keyed on the deletion, so a newer one replaces the snackbar on screen + // (and the older delete stands). The deletion lives in a singleton, not + // here: one made in the editor arrives after the editor has gone, and one + // whose snackbar a rotation or another tab interrupted is shown again. + LaunchedEffect(pendingUndo) { + val deleted = pendingUndo ?: return@LaunchedEffect + val result = snackbar.showSnackbar( + message = undoMessage.orEmpty(), + actionLabel = undoLabel, + duration = SnackbarDuration.Long, + ) + when (result) { + SnackbarResult.ActionPerformed -> viewModel.onUndoDelete(deleted) + SnackbarResult.Dismissed -> viewModel.onUndoExpired(deleted) + } + } + CollapsingScaffold( title = stringResource(R.string.tab_alarms), modifier = modifier, scrollable = false, + snackbarHost = { SnackbarHost(snackbar) }, actions = { IconButton(onClick = onOpenSettings) { Icon( @@ -116,6 +145,8 @@ fun AlarmsScreen( } }, onToggle = { enabled -> viewModel.onToggleEnabled(row.id, enabled) }, + // No confirmation: the snackbar's Undo is the safety net. + onDelete = { viewModel.onDelete(row.id) }, // An alarm whose time changed slides to its new place // instead of teleporting. modifier = animateItemMotion(), 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..00f33c4 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 @@ -7,6 +7,7 @@ import de.jeanlucmakiola.clockula.alarm.AlarmEngine import de.jeanlucmakiola.clockula.data.alarms.AlarmRepository import de.jeanlucmakiola.clockula.domain.AlarmDefaults import de.jeanlucmakiola.clockula.domain.AlarmDraft +import de.jeanlucmakiola.clockula.domain.alarm.DeletedAlarm import de.jeanlucmakiola.clockula.domain.time.WallClock import de.jeanlucmakiola.clockula.domain.time.ZoneProvider import kotlinx.coroutines.channels.Channel @@ -20,7 +21,7 @@ import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import javax.inject.Inject -/** The alarm list: what it shows, the "+" that creates a row, and the row's switch. */ +/** The alarm list: what it shows, the "+" that creates a row, the row's switch and swipe, and the Undo. */ @HiltViewModel class AlarmsViewModel @Inject internal constructor( private val alarms: AlarmRepository, @@ -28,6 +29,7 @@ class AlarmsViewModel @Inject internal constructor( private val wall: WallClock, private val zones: ZoneProvider, private val source: AlarmListSource, + private val undo: AlarmUndo, ) : ViewModel() { val state: StateFlow = source.rows() @@ -76,6 +78,29 @@ class AlarmsViewModel @Inject internal constructor( viewModelScope.launch { engine.dismissUpcoming(alarmId) } } + /** The deletion the list's snackbar offers to undo, from the swipe or the editor. Null when none. */ + val pendingUndo: StateFlow = undo.pending + + /** + * The swipe's delete. No confirmation: the snackbar's Undo is the safety + * net, and it catches the same slip a dialog would without taxing every + * delete the user meant. + */ + fun onDelete(alarmId: Long) { + viewModelScope.launch { engine.delete(alarmId)?.let(undo::offer) } + } + + /** The snackbar's Undo. Claimed first, so a double tap restores once. */ + fun onUndoDelete(deleted: DeletedAlarm) { + if (!undo.take(deleted)) return + viewModelScope.launch { engine.restore(deleted) } + } + + /** The snackbar timed out or was swiped away: the delete stands. */ + fun onUndoExpired(deleted: DeletedAlarm) { + undo.take(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 new file mode 100644 index 0000000..ec4f8db --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/SwipeToDelete.kt @@ -0,0 +1,147 @@ +package de.jeanlucmakiola.clockula.ui.common + +import androidx.compose.animation.core.Animatable +import androidx.compose.foundation.background +import androidx.compose.foundation.gestures.awaitEachGesture +import androidx.compose.foundation.gestures.awaitFirstDown +import androidx.compose.foundation.gestures.awaitHorizontalTouchSlopOrCancellation +import androidx.compose.foundation.gestures.horizontalDrag +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.padding +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.rounded.Delete +import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableIntStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.rememberUpdatedState +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.graphics.Shape +import androidx.compose.ui.graphics.graphicsLayer +import androidx.compose.ui.hapticfeedback.HapticFeedbackType +import androidx.compose.ui.input.pointer.pointerInput +import androidx.compose.ui.input.pointer.positionChange +import androidx.compose.ui.input.pointer.util.VelocityTracker +import androidx.compose.ui.input.pointer.util.addPointerInputChange +import androidx.compose.ui.layout.onSizeChanged +import androidx.compose.ui.platform.LocalDensity +import androidx.compose.ui.platform.LocalHapticFeedback +import androidx.compose.ui.platform.LocalLayoutDirection +import androidx.compose.ui.unit.LayoutDirection +import androidx.compose.ui.unit.dp +import kotlinx.coroutines.launch + +/** + * A card the user can swipe away towards the **end** edge to delete. + * + * Hand-rolled rather than M3's `SwipeToDismissBox`, because the tabs are pages + * of a `HorizontalPager` and the box claims a horizontal drag in *both* + * directions, even one it has no anchor for. Here only the start-to-end drag + * is claimed. The end-to-start one is left unconsumed for the pager, so a + * swipe across a card still changes tab. Start-to-end is also the pager's + * "previous tab" direction, which on the first tab goes nowhere, so the two + * gestures never compete for the same drag. + * + * The gesture is never the only way: the caller still owes a screen reader a + * "Delete" custom action on the card itself. + */ +@OptIn(ExperimentalMaterial3ExpressiveApi::class) +@Composable +internal fun SwipeToDelete( + onDelete: () -> Unit, + shape: Shape, + modifier: Modifier = Modifier, + content: @Composable () -> Unit, +) { + val currentOnDelete by rememberUpdatedState(onDelete) + val scope = rememberCoroutineScope() + val haptics = LocalHapticFeedback.current + val settleSpec = MaterialTheme.motionScheme.defaultSpatialSpec() + // +1 when "towards the end" is rightwards, -1 when it is leftwards. + val direction = if (LocalLayoutDirection.current == LayoutDirection.Ltr) 1f else -1f + val flingVelocity = with(LocalDensity.current) { FlingVelocity.toPx() } + + // How far the card has travelled towards the end edge: 0 at rest, never negative. + val travel = remember { Animatable(0f) } + var width by remember { mutableIntStateOf(0) } + + Box( + modifier = modifier + .onSizeChanged { width = it.width } + .pointerInput(direction, flingVelocity) { + awaitEachGesture { + val down = awaitFirstDown(requireUnconsumed = false) + val velocity = VelocityTracker() + velocity.addPointerInputChange(down) + val start = awaitHorizontalTouchSlopOrCancellation(down.id) { change, overSlop -> + // Unconsumed, the detection keeps waiting and the pager + // gets its chance at the same drag. + if (overSlop * direction > 0f) change.consume() + } ?: return@awaitEachGesture + + // Accumulated here, not read back from [travel]: a snap is + // launched, so the next delta can arrive before it lands. + var position = travel.value + var armed = position >= width * DeleteFraction + horizontalDrag(start.id) { change -> + velocity.addPointerInputChange(change) + position = (position + change.positionChange().x * direction) + .coerceIn(0f, width.toFloat()) + val nowArmed = position >= width * DeleteFraction + // One tick as the release would start to delete, so the + // threshold is felt rather than guessed at. + if (nowArmed && !armed) haptics.performHapticFeedback(HapticFeedbackType.GestureThresholdActivate) + armed = nowArmed + val target = position + scope.launch { travel.snapTo(target) } + change.consume() + } + + val flung = velocity.calculateVelocity().x * direction >= flingVelocity + scope.launch { + if (width > 0 && (armed || (flung && position > 0f))) { + travel.animateTo(width.toFloat(), settleSpec) + currentOnDelete() + } else { + travel.animateTo(0f, settleSpec) + } + } + } + }, + ) { + if (travel.value > 0f) { + Box( + contentAlignment = Alignment.CenterStart, + modifier = Modifier + .matchParentSize() + .clip(shape) + .background(MaterialTheme.colorScheme.errorContainer), + ) { + Icon( + imageVector = Icons.Rounded.Delete, + // The card's own custom action is what a screen reader + // hears; this glyph only names what the swipe is doing. + contentDescription = null, + tint = MaterialTheme.colorScheme.onErrorContainer, + modifier = Modifier.padding(start = 24.dp), + ) + } + } + Box(Modifier.graphicsLayer { translationX = travel.value * direction }) { + content() + } + } +} + +/** Past this share of the card's width, letting go deletes. */ +private const val DeleteFraction = 0.4f + +/** A fling at least this fast towards the end deletes, however short the drag. */ +private val FlingVelocity = 800.dp diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimerRow.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimerRow.kt index d99eb4e..93d02bf 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimerRow.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimerRow.kt @@ -67,10 +67,9 @@ import kotlin.time.Duration.Companion.minutes * never from enlarging text", and timer progress is one of the three places §8 * sanctions a showpiece (D29). * - * The top-right delete button asks nothing first: re-creating a timer costs - * one tap on the keypad and a Start, which is faster than reading a - * confirmation dialog would be, so there is nothing a dialog would protect - * that undoing the mistake doesn't already cover. + * The top-right delete button asks nothing first: the list's snackbar offers + * an Undo that puts the timer back still counting, which covers the slip a + * dialog would without slowing every delete the user meant. */ @OptIn(ExperimentalMaterial3ExpressiveApi::class) @Composable 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 0741321..36b1db0 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 @@ -12,8 +12,14 @@ import androidx.compose.material.icons.rounded.Settings import androidx.compose.material3.FloatingActionButton import androidx.compose.material3.Icon import androidx.compose.material3.IconButton +import androidx.compose.material3.SnackbarDuration +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState +import androidx.compose.material3.SnackbarResult import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue @@ -50,6 +56,25 @@ fun TimersScreen( val state by viewModel.state.collectAsStateWithLifecycle() val pillInset = LocalLivePillInset.current val addTimer = stringResource(R.string.timers_set_title) + val pendingUndo by viewModel.pendingUndo.collectAsStateWithLifecycle() + val snackbar = remember { SnackbarHostState() } + val undoMessage = stringResource(R.string.timer_deleted) + val undoLabel = stringResource(R.string.action_undo) + + // Keyed on the deletion, so a newer one replaces the snackbar on screen and + // the older delete stands. + LaunchedEffect(pendingUndo) { + val deleted = pendingUndo ?: return@LaunchedEffect + val result = snackbar.showSnackbar( + message = undoMessage, + actionLabel = undoLabel, + duration = SnackbarDuration.Long, + ) + when (result) { + SnackbarResult.ActionPerformed -> viewModel.onUndoDelete(deleted) + SnackbarResult.Dismissed -> viewModel.onUndoExpired(deleted) + } + } val loaded = state as? TimersUiState.Loaded val rows = loaded?.rows @@ -58,6 +83,7 @@ fun TimersScreen( title = stringResource(R.string.tab_timers), modifier = modifier, scrollable = false, + snackbarHost = { SnackbarHost(snackbar) }, actions = { IconButton(onClick = onOpenSettings) { Icon( @@ -110,10 +136,10 @@ fun TimersScreen( onReset = { viewModel.onReset(row.id) }, onAddMinute = { viewModel.onAddMinute(row.id) }, onSubtractMinute = { viewModel.onSubtractMinute(row.id) }, - // No confirmation: re-creating a timer costs one tap and a - // Start, faster than reading a dialog would be (D2's delete - // still routes through the engine, so a ringing timer is - // stopped cleanly rather than deleted into a stuck ring). + // No confirmation: the snackbar's Undo puts the timer back, + // still counting (D2's delete still routes through the + // engine, so a ringing timer is stopped cleanly rather than + // deleted into a stuck ring). onDelete = { viewModel.onDelete(row.id) }, // A timer that finishes stays where it is (D22); the motion // is for a row arriving or leaving. 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 513164e..3d07d9f 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 @@ -5,11 +5,14 @@ import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel 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.timer.TimerPresets import de.jeanlucmakiola.clockula.timer.TimerEngine +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 @@ -78,8 +81,33 @@ class TimersViewModel @Inject internal constructor( fun onSubtractMinute(timerId: Long) = launchOnEngine { engine.addTime(timerId, -1.minutes) } - /** Through the engine, so a *ringing* timer cannot be deleted into a stuck ring (D2). */ - fun onDelete(timerId: Long) = launchOnEngine { engine.delete(timerId) } + private val undoSlot = MutableStateFlow(null) + + /** + * The deleted timer the snackbar offers to bring back. Null when none. Held + * here, not in the screen, so a rotation or a trip to another tab re-shows + * the snackbar rather than losing the Undo; a newer delete replaces it. + */ + val pendingUndo: StateFlow = undoSlot.asStateFlow() + + /** + * 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 } + } + + /** The snackbar's Undo. Claimed first, so a double tap restores once. */ + fun onUndoDelete(timer: Timer) { + if (!undoSlot.compareAndSet(timer, null)) return + launchOnEngine { engine.restore(timer) } + } + + /** The snackbar timed out or was swiped away: the delete stands. */ + fun onUndoExpired(timer: Timer) { + undoSlot.compareAndSet(timer, null) + } fun onSavePreset(duration: Duration) { viewModelScope.launch { settings.addTimerPreset(duration) } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index ad0a66c..7904c6f 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -65,6 +65,7 @@ Sound Delete timer Delete this timer? + Timer deleted Timers @@ -135,9 +136,9 @@ Snooze limit To dismiss Delete alarm - Delete this alarm? - The %1$s alarm will be removed. + %1$s alarm deleted Delete + Undo Cancel OK diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineDeleteTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineDeleteTest.kt new file mode 100644 index 0000000..d1cca3b --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineDeleteTest.kt @@ -0,0 +1,129 @@ +package de.jeanlucmakiola.clockula.alarm + +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.clockula.data.alarms.AlarmMapper +import de.jeanlucmakiola.clockula.domain.RepeatDays +import de.jeanlucmakiola.clockula.testing.RingEvent +import de.jeanlucmakiola.clockula.testing.alarmAt +import de.jeanlucmakiola.clockula.testing.alarmEngineHarness +import de.jeanlucmakiola.clockula.testing.offsetInstant +import de.jeanlucmakiola.clockula.testing.ringState +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.nio.file.Path +import kotlin.time.Duration.Companion.minutes + +/** + * The engine's delete and its Undo: the ring and the snooze notification go + * with the row, and what comes back is re-registered and never rings stale. + */ +class AlarmEngineDeleteTest { + + private val sevenToday = offsetInstant("2026-06-10T07:00+02:00") + private val sevenTomorrow = offsetInstant("2026-06-11T07:00+02:00") + + @Test + fun `deleting an alarm that is not there returns nothing to undo`(@TempDir dir: Path) = runTest { + val harness = alarmEngineHarness(dir) + + assertThat(harness.engine.delete(1L)).isNull() + } + + @Test + fun `deleting a snoozed alarm clears its notification and frees the slot`(@TempDir dir: Path) = runTest { + val harness = alarmEngineHarness(dir) + harness.givenAlarm(id = 1L, hour = 7) + harness.givenRinging(alarmId = 1L, at = sevenToday) + harness.engine.snooze(1L) + + harness.engine.delete(1L) + + assertThat(listOf(harness.notifier.cleared.last(), harness.scheduler.next, harness.stateDao.stored)) + .isEqualTo(listOf(1L, null, emptyList())) + } + + @Test + fun `undoing the delete of a snoozed alarm brings the snooze and its notification back`(@TempDir dir: Path) = + runTest { + val harness = alarmEngineHarness(dir) + harness.givenAlarm(id = 1L, hour = 7) + harness.givenRinging(alarmId = 1L, at = sevenToday) + harness.engine.snooze(1L) + val until = sevenToday + 10.minutes + val deleted = checkNotNull(harness.engine.delete(1L)) + harness.notifier.shown.clear() + + val restored = harness.engine.restore(deleted) + + assertThat(listOf(restored, harness.scheduler.next?.fireAt, harness.notifier.shown)) + .isEqualTo(listOf(true, until, listOf(1L to until))) + } + + @Test + fun `a snooze that lapsed while the alarm was gone comes back closed, with no notification`(@TempDir dir: Path) = + runTest { + val harness = alarmEngineHarness(dir) + harness.givenAlarm(id = 1L, hour = 7, repeatDays = RepeatDays.EVERY_DAY) + harness.givenRinging(alarmId = 1L, at = sevenToday) + harness.engine.snooze(1L) + val deleted = checkNotNull(harness.engine.delete(1L)) + harness.notifier.shown.clear() + harness.wallClock.instant = sevenToday + 3.minutes * 60 + + harness.engine.restore(deleted) + + assertThat(harness.notifier.shown to harness.scheduler.next?.fireAt) + .isEqualTo(emptyList() to sevenTomorrow) + } + + @Test + fun `deleting the ringing alarm stops the ring and its backstop`(@TempDir dir: Path) = runTest { + val harness = alarmEngineHarness(dir) + harness.givenAlarm(id = 1L, hour = 7) + harness.givenRinging(alarmId = 1L, at = sevenToday) + + harness.engine.delete(1L) + + assertThat(listOf(harness.ring.events.last(), harness.ring.ringing.value, harness.scheduler.autoSilence)) + .isEqualTo(listOf(RingEvent.Stopped(1L), null, null)) + } + + @Test + fun `a ringing one-shot comes back switched off and silent`(@TempDir dir: Path) = runTest { + val harness = alarmEngineHarness(dir) + harness.givenAlarm(id = 1L, hour = 7) + harness.givenRinging(alarmId = 1L, at = sevenToday) + val deleted = checkNotNull(harness.engine.delete(1L)) + + harness.engine.restore(deleted) + + val alarm = checkNotNull(harness.alarms.find(1L)) + assertThat(listOf(alarm.enabled, harness.ring.ringing.value, harness.scheduler.next)) + .isEqualTo(listOf(false, null, null)) + } + + @Test + fun `a ringing transient alarm leaves nothing to undo`(@TempDir dir: Path) = runTest { + val harness = alarmEngineHarness(dir) + harness.givenAlarm(id = 1L, hour = 7, deleteAfterUse = true) + harness.givenRinging(alarmId = 1L, at = sevenToday) + + assertThat(harness.engine.delete(1L) to harness.alarmDao.stored).isEqualTo(null to emptyList()) + } + + @Test + fun `undo writes nothing when the id has been taken in the meantime`(@TempDir dir: Path) = runTest { + val harness = alarmEngineHarness(dir) + harness.givenAlarm(id = 1L, hour = 7) + val deleted = checkNotNull(harness.engine.delete(1L)) + val imported = alarmAt(id = 1L, hour = 9, label = "From a backup") + harness.alarmDao.insert(AlarmMapper.toEntity(imported)) + harness.states.save(ringState(alarmId = 1L, handledOccurrence = sevenToday)) + + val restored = harness.engine.restore(deleted) + + assertThat(listOf(restored, harness.alarms.find(1L), harness.states.state(1L).handledOccurrence)) + .isEqualTo(listOf(false, imported, sevenToday)) + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmDeletionTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmDeletionTest.kt new file mode 100644 index 0000000..b7a973c --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmDeletionTest.kt @@ -0,0 +1,64 @@ +package de.jeanlucmakiola.clockula.domain.alarm + +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.clockula.domain.RepeatDays +import de.jeanlucmakiola.clockula.testing.alarmAt +import de.jeanlucmakiola.clockula.testing.offsetInstant +import de.jeanlucmakiola.clockula.testing.ringState +import org.junit.jupiter.api.Test + +/** What an Undo puts back, and the one rule that matters: never a stale ring. */ +class AlarmDeletionTest { + + private val seven = offsetInstant("2026-06-10T07:00+02:00") + private val tenPast = offsetInstant("2026-06-10T07:10+02:00") + + @Test + fun `an idle alarm comes back exactly as it was`() { + val alarm = alarmAt(hour = 7, label = "Gym", repeatDays = RepeatDays.WEEKDAYS, skipNextOccurrence = true) + val state = ringState(handledOccurrence = seven) + + assertThat(AlarmDeletion.snapshot(alarm, state)).isEqualTo(DeletedAlarm(alarm, state)) + } + + @Test + fun `a snoozed alarm comes back snoozed`() { + val alarm = alarmAt(hour = 7) + val state = ringState(snoozedUntil = tenPast, snoozeCount = 1, handledOccurrence = seven) + + assertThat(AlarmDeletion.snapshot(alarm, state)).isEqualTo(DeletedAlarm(alarm, state)) + } + + @Test + fun `a ringing repeating alarm comes back with its cycle closed and its watermark kept`() { + val alarm = alarmAt(hour = 7, repeatDays = RepeatDays.EVERY_DAY) + val state = ringState(ringingSince = seven, snoozeCount = 2, handledOccurrence = seven) + + assertThat(AlarmDeletion.snapshot(alarm, state)) + .isEqualTo(DeletedAlarm(alarm, ringState(handledOccurrence = seven))) + } + + @Test + fun `a ringing one-shot comes back switched off, as a dismissal would leave it`() { + val alarm = alarmAt(hour = 7) + val state = ringState(ringingSince = seven, handledOccurrence = seven) + + assertThat(AlarmDeletion.snapshot(alarm, state)) + .isEqualTo(DeletedAlarm(alarm.copy(enabled = false), ringState(handledOccurrence = seven))) + } + + @Test + fun `a ringing transient one-shot has nothing to come back as`() { + val alarm = alarmAt(hour = 7, deleteAfterUse = true) + + assertThat(AlarmDeletion.snapshot(alarm, ringState(ringingSince = seven))).isNull() + } + + @Test + fun `a transient alarm that is not ringing can still be undone`() { + val alarm = alarmAt(hour = 7, deleteAfterUse = true) + val state = ringState() + + assertThat(AlarmDeletion.snapshot(alarm, state)).isEqualTo(DeletedAlarm(alarm, state)) + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/timer/TimerDeletionTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/timer/TimerDeletionTest.kt new file mode 100644 index 0000000..53fd87f --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/clockula/domain/timer/TimerDeletionTest.kt @@ -0,0 +1,53 @@ +package de.jeanlucmakiola.clockula.domain.timer + +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.clockula.testing.T0 +import de.jeanlucmakiola.clockula.testing.expiredTimer +import de.jeanlucmakiola.clockula.testing.idleTimer +import de.jeanlucmakiola.clockula.testing.pausedTimer +import de.jeanlucmakiola.clockula.testing.runningTimerWith +import org.junit.jupiter.api.Test +import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.seconds + +/** What an Undo puts back: everything as it was, except a finished timer's ring. */ +class TimerDeletionTest { + + private val now = 1_000.seconds + + @Test + fun `a running timer comes back with its anchors, still counting`() { + val timer = runningTimerWith(left = 2.minutes, elapsedNow = now, wallNow = T0, label = "Tea", sortOrder = 3) + + assertThat(TimerDeletion.snapshot(timer, now, T0)).isEqualTo(timer) + } + + @Test + fun `paused and idle timers come back as they were`() { + val paused = pausedTimer(remaining = 90.seconds) + val idle = idleTimer(duration = 10.minutes) + + assertThat(listOf(TimerDeletion.snapshot(paused, now, T0), TimerDeletion.snapshot(idle, now, T0))) + .containsExactly(paused, idle) + .inOrder() + } + + @Test + fun `a finished timer comes back reset, not ringing`() { + val timer = expiredTimer(duration = 5.minutes, label = "Eggs") + + assertThat(TimerDeletion.snapshot(timer, now, T0)).isEqualTo(idleTimer(duration = 5.minutes, label = "Eggs")) + } + + @Test + fun `a running timer already past its end counts as finished`() { + val timer = runningTimerWith(left = 1.minutes, elapsedNow = now - 2.minutes, wallNow = T0) + + assertThat(TimerDeletion.snapshot(timer, now, T0)).isEqualTo(idleTimer()) + } + + @Test + fun `a finished transient timer has nothing to come back as`() { + assertThat(TimerDeletion.snapshot(expiredTimer(deleteAfterUse = true), now, T0)).isNull() + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeAlarmSeams.kt b/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeAlarmSeams.kt index 591ec13..8143c25 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeAlarmSeams.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeAlarmSeams.kt @@ -204,6 +204,7 @@ class RecordingTimerRepository : TimerRepository { override suspend fun create(draft: TimerDraft): Long = TODO("not used by the reboot gate") override suspend fun rename(id: Long, label: String): Unit = TODO("not used by the reboot gate") override suspend fun delete(id: Long): Unit = TODO("not used by the reboot gate") + override suspend fun restore(timer: Timer): Boolean = TODO("not used by the reboot gate") override suspend fun start(id: Long): Unit = TODO("not used by the reboot gate") override suspend fun pause(id: Long): Unit = TODO("not used by the reboot gate") override suspend fun reset(id: Long): Unit = TODO("not used by the reboot gate") diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeDaos.kt b/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeDaos.kt index b56b28e..2a168ea 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeDaos.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeDaos.kt @@ -60,6 +60,9 @@ open class FakeAlarmDao( return id } + override suspend fun insertIfAbsent(alarm: AlarmEntity): Long = + if (rows.value.any { it.id == alarm.id }) -1L else insert(alarm) + override suspend fun update(alarm: AlarmEntity) { rows.update { rows -> rows.map { if (it.id == alarm.id) alarm else it } } } @@ -109,6 +112,9 @@ open class FakeTimerDao(initial: List = emptyList()) : TimerDao() { return id } + override suspend fun insertIfAbsent(timer: TimerEntity): Long = + if (rows.value.any { it.id == timer.id }) -1L else insert(timer) + override suspend fun update(timer: TimerEntity) { rows.update { rows -> rows.map { if (it.id == timer.id) timer else it } } } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeRepositories.kt b/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeRepositories.kt index bc59532..c3adf5d 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeRepositories.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeRepositories.kt @@ -79,6 +79,13 @@ class FakeTimerRepository( rows.update { all -> all.filterNot { it.id == id } } } + override suspend fun restore(timer: Timer): Boolean { + log += "restore(${timer.id})" + if (rows.value.any { it.id == timer.id }) return false + rows.update { it + timer } + return true + } + override suspend fun start(id: Long) { log += "start($id)" val now = elapsed.elapsedRealtime() 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..5f5146b 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 @@ -51,7 +51,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) = + private fun TestScope.editor(harness: AlarmEngineHarness, alarmId: Long = 1L, undo: AlarmUndo = AlarmUndo()) = AlarmEditorViewModel( savedStateHandle = SavedStateHandle(mapOf(AlarmRoutes.ARG_ALARM_ID to alarmId)), alarms = harness.alarms, @@ -59,6 +59,7 @@ class AlarmEditorViewModelTest { engine = harness.engine, ringtones = catalog, preview = previewer, + undo = undo, appScope = this, ) @@ -655,6 +656,27 @@ class AlarmEditorViewModelTest { .isEqualTo(RingEvent.Stopped(1L) to emptyList()) } + @Test + fun `deleting hands the alarm to the list's undo, and the undo puts it back`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + harness.engine.reschedule() + val undo = AlarmUndo() + val viewModel = editor(harness, undo = undo) + advanceUntilIdle() + val before = harness.alarmDao.stored + + viewModel.onDelete() + advanceUntilIdle() + val deleted = checkNotNull(undo.pending.value) + // The editor's ViewModel is gone by now; the list's restores. + harness.engine.restore(deleted) + + assertThat(harness.alarmDao.stored to harness.scheduler.next?.alarmId) + .isEqualTo(before to 1L) + } + @Test fun `deleting the last alarm clears the slot`(@TempDir tempDir: Path) = runTest(main.dispatcher) { val harness = alarmEngineHarness(tempDir, now = fiveToday) 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..9abed55 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 @@ -46,12 +46,13 @@ 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, undo: AlarmUndo = AlarmUndo()) = AlarmsViewModel( alarms = harness.alarms, engine = harness.engine, wall = harness.wallClock, zones = harness.zones, source = AlarmListSource(harness.engine, harness.wallClock, harness.ticker), + undo = undo, ).also { viewModel -> backgroundScope.launch { viewModel.state.collect {} } } private fun AlarmsUiState.rows(): List = (this as AlarmsUiState.Loaded).rows @@ -346,6 +347,104 @@ class AlarmsViewModelTest { assertThat(viewModel.state.value.rows().map { it.id }).containsExactly(2L) } + // --- swipe to delete, and the Undo --- + + @Test + fun `a swiped alarm is deleted at once and offered back`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 7) + harness.givenAlarm(id = 2L, hour = 9) + val viewModel = viewModel(harness) + advanceUntilIdle() + + viewModel.onDelete(1L) + advanceUntilIdle() + + assertThat(viewModel.state.value.rows().map { it.id } to viewModel.pendingUndo.value?.alarm?.id) + .isEqualTo(listOf(2L) to 1L) + } + + @Test + fun `undo restores the alarm with its id and settings and re-registers it`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 7, label = "Gym", repeatDays = RepeatDays.WEEKDAYS, skipNextOccurrence = true) + harness.engine.reschedule() + val before = harness.alarmDao.stored + val viewModel = viewModel(harness) + advanceUntilIdle() + viewModel.onDelete(1L) + advanceUntilIdle() + check(harness.scheduler.next == null) + + viewModel.onUndoDelete(checkNotNull(viewModel.pendingUndo.value)) + advanceUntilIdle() + + assertThat(Triple(harness.alarmDao.stored, harness.scheduler.next?.alarmId, viewModel.pendingUndo.value)) + .isEqualTo(Triple(before, 1L, null)) + } + + @Test + fun `a second tap on undo restores once`(@TempDir tempDir: Path) = runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 7) + val viewModel = viewModel(harness) + advanceUntilIdle() + viewModel.onDelete(1L) + advanceUntilIdle() + val deleted = checkNotNull(viewModel.pendingUndo.value) + + viewModel.onUndoDelete(deleted) + advanceUntilIdle() + harness.alarms.delete(1L) + viewModel.onUndoDelete(deleted) + advanceUntilIdle() + + assertThat(harness.alarmDao.stored).isEmpty() + } + + @Test + fun `a timed-out snackbar lets the delete stand`(@TempDir tempDir: Path) = runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 7) + val viewModel = viewModel(harness) + advanceUntilIdle() + viewModel.onDelete(1L) + advanceUntilIdle() + val deleted = checkNotNull(viewModel.pendingUndo.value) + + viewModel.onUndoExpired(deleted) + viewModel.onUndoDelete(deleted) + advanceUntilIdle() + + assertThat(harness.alarmDao.stored to viewModel.pendingUndo.value).isEqualTo(emptyList() to null) + } + + @Test + fun `swiping away the ringing alarm silences it, and undo does not ring it again`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 7, repeatDays = RepeatDays.EVERY_DAY) + harness.givenRinging(alarmId = 1L, at = sevenToday) + val viewModel = viewModel(harness) + advanceUntilIdle() + + 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"))) + } + // --- M9: the AlarmClock contract's dismissal, and the chooser dialog's --- @Test 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 3dc0fe1..bc90f75 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 @@ -158,6 +158,43 @@ class TimersViewModelTest { assertThat(harness.ringState.current()).isNull() } + @Test + fun `undo puts a deleted running timer back, still counting and registered`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = timerEngineHarness(tempDir, timerDao = dao) + val timer = harness.given(runningTimerWith(id = 1L, left = 4.minutes, elapsedNow = uptime, wallNow = T0)) + harness.engine.resync() + val viewModel = viewModel(harness) + advanceUntilIdle() + viewModel.onDelete(1L) + advanceUntilIdle() + check(harness.scheduler.expiryAt == null) + + viewModel.onUndoDelete(checkNotNull(viewModel.pendingUndo.value)) + advanceUntilIdle() + + assertThat(listOf(harness.stored(1L), harness.scheduler.expiryAt, harness.service.active, viewModel.pendingUndo.value)) + .isEqualTo(listOf(timer, uptime + 4.minutes, true, null)) + } + + @Test + fun `undo brings a finished timer back reset, not ringing`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = timerEngineHarness(tempDir, timerDao = dao) + harness.given(expiredTimer(id = 1L)) + harness.ringState.set(T0 - 30.seconds) + val viewModel = viewModel(harness) + advanceUntilIdle() + viewModel.onDelete(1L) + advanceUntilIdle() + + viewModel.onUndoDelete(checkNotNull(viewModel.pendingUndo.value)) + advanceUntilIdle() + + assertThat(listOf(harness.stored(1L)?.state, harness.ringState.current())) + .isEqualTo(listOf(TimerState.IDLE, null)) + } + /** §5.12 #8 */ @Test fun `the stored presets are part of the loaded state`(@TempDir tempDir: Path) = diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 97ef4e8..8eec71d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1060,7 +1060,7 @@ M10 adds no ``. | A user-configurable auto-silence duration, an unlimited-snooze option, per-alarm auto-silence | not planned for v1 | | Reordering timers by hand. `sort_order` and `TimerRepository.reorder` exist and stay caller-less: the list is in storage order, M5 gave the same answer for alarms, and a drag surface would also mean abandoning the `LazyColumn` | not planned for v1 | | Per-timer vibrate, volume-ramp or dismiss-challenge overrides. `timers` has exactly one nullable settings column, its ringtone; four more would be a schema change for settings the roadmap does not ask for | not planned for v1 | -| A "stop all" for several expired timers, and undo after a timer delete. Each expired timer is acknowledged on its own, and the delete confirmation asks before rather than after | not planned for v1 | +| A "stop all" for several expired timers. Each expired timer is acknowledged on its own | not planned for v1 | | A presets management screen (add/remove the quick-start durations themselves). M10's settings screen edits `default_timer_ringtone_uri`, but the preset list is still only editable from the two-gesture surface on the setup panel | not planned for v1 | | A timer that ramps its volume, a dismiss challenge or a snooze for a timer | not planned for v1 | | Surfacing `TimerSnapshot.anchorIsStale` in the UI. The boot repair makes it vanishingly rare and the value is already honest; a row does not say "estimated" in v1 | not planned for v1 | @@ -1688,7 +1688,8 @@ the backstop disarmed by its own guard**. Both ViewModels therefore call if (engine.ringSession()?.alarm?.id == alarmId) engine.dismiss(alarmId) ``` -before `setEnabled(id, false)` and before `delete(id)`. It reads storage under +before `setEnabled(id, false)`. Deleting goes further and is an engine verb of +its own (`AlarmEngine.delete`, below), which stops the ring itself. The check reads storage under the engine's own mutex, so it cannot race the fire it is asking about, and it is deliberately **conditional**: dismissing unconditionally would also drop a merely *snoozed* alarm's pending snooze, which is wrong for a label edit. @@ -1712,10 +1713,52 @@ a **repeating** alarm. The resolver answers a one-shot-with-skip by disabling th alarm, and a switch labelled "skip" that silently switched the alarm off would be a lie; the user who wants that has the enable switch. -Delete lives in the editor behind a confirmation that names the alarm, and there -is deliberately no undo chip: the delete happens on the editor, so an undo chip -would have to live on the list, and an accidentally deleted alarm is discovered -at 07:00 rather than now; the cheap moment to ask is before. +### Delete, and the Undo + +An alarm is deleted by swiping its card towards the end edge, by the card's +**Delete** custom accessibility action (the same verb for TalkBack and switch +access, where there is no swipe), or from the editor's top bar. None of them +asks first. 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 +confirmation the user taps through out of habit catches less. The Undo does it +without taxing every delete the user meant, so the editor's dialog went too +rather than leaving two delete paths with different safety nets. + +`AlarmEngine.delete(id)` does the work under the engine's lock: it stops a +ringing alarm's service and auto-silence backstop (the global slots of §11), +clears a snoozed alarm's notification, deletes the row (the ring state +cascades) and re-resolves the slot. It returns a `DeletedAlarm`, the row and +its ring state as `AlarmDeletion.snapshot` decided, and `AlarmEngine.restore` +puts that back **under the same id** (`alarms.id` is `AUTOINCREMENT`, so no new +alarm can have taken it) and re-resolves, which is what re-registers it with +AlarmManager. The snapshot keeps every setting and the ring state as it was, so a +snoozed alarm comes back snoozed, notification included, and the handled-occurrence +watermark still stops a just-closed occurrence ringing twice. A **ringing** +alarm is the exception: deleting it was a dismissal in all but name, so it comes +back as a dismissal would have left it (cycle closed, a one-shot switched off) +and never rings again, and a ringing transient alarm, which that dismissal would +have deleted anyway, offers no Undo at all. A snooze that lapsed while the alarm +was gone is closed by the resolver on restore, and gets no notification. An id +that a backup restore claimed in the meantime makes the restore write nothing. + +The pending deletion lives in `AlarmUndo`, a singleton, not in either +ViewModel. The editor is popped the moment its alarm is gone, so its ViewModel +is cleared before any snackbar could be answered; the list underneath reads the +deletion from the singleton and shows the snackbar. It is cleared only when the +snackbar is answered or times out, or a newer deletion replaces it, so a +rotation or a trip to another tab re-shows the snackbar rather than losing the +Undo. It is memory only: an Undo that outlived process death would be offered +with no snackbar anyone remembers. + +The swipe is hand-rolled (`ui/common/SwipeToDelete`) rather than M3's +`SwipeToDismissBox`, because the tabs are pages of a `HorizontalPager` and the +box claims a horizontal drag in both directions, even one it has no anchor for. +Only the start-to-end drag is claimed; the end-to-start one is left unconsumed, +so a swipe across a card still changes tab. Start-to-end is also the pager's +"previous tab" direction, which from Alarms, the first tab, goes nowhere, so the +two gestures never want the same drag. --- @@ -1751,6 +1794,17 @@ already met once, and the cheapest way not to have it again is to make the wrong call impossible to write. **Configuration** edits (`rename`, `setDuration`, `setRingtoneUri`) stay on `TimerRepository` and the editor calls them directly. +A card's corner delete asks nothing first, and the list's snackbar offers an +**Undo**. `TimerEngine.delete` returns the row as `TimerDeletion.snapshot` +decided, and `TimerEngine.restore` puts it back under its own id and place and +resyncs. A running timer comes back with its anchors, so it has kept counting +while it was gone, and one whose end passed in the meantime rings, as it would +have had it never been deleted. A timer that had already finished comes back +reset: deleting it silenced it, which is a dismissal, so it does not ring a +second time (a finished transient timer offers no Undo). The pending row is +`TimersViewModel` state, since both the delete and the snackbar are on the +list; the editor's delete keeps its confirmation. + `TimerRepository` deliberately has **no generic `edit(id, transform)`** the way `AlarmRepository` does: a timer row carries three derived anchors whose consistency is the whole of §5, and a caller handed a `(Timer) -> Timer` can @@ -2462,8 +2516,9 @@ an id the list does not hold, and always a permutation of its input; a move that changes nothing writes nothing. Remove is a long-press plus a confirmation that names the city, with -deliberately **no undo chip**. That is the same trade M5 and M6 made: the cheap -moment to ask is before. The pending id is `rememberSaveable`, so a rotation +deliberately **no undo chip**. That was the trade M5 and M6 made too, and the +alarm and timer lists have since moved to an Undo (§13, §14); removing a city +still asks first. The pending id is `rememberSaveable`, so a rotation mid-confirmation does not lose the question. A row whose zone the device's tzdata no longer knows shows **no time, no offset diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index bb7731b..d53556f 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -247,7 +247,8 @@ line ("in 9h 12m") on each row. Built on `GroupedSurface`/`GroupedRow`. Knowingly open: no hand reordering of alarms (the time order answers it, and `alarms` has no `sort_order` column), no multi-select or bulk delete, no undo - chip after a delete (traded for a confirmation that names the alarm), no + chip after a delete (traded for a confirmation that names the alarm; since + replaced by swipe to delete with Undo, #20), no repeat presets, and no per-alarm auto-silence (still M10 at the earliest). Editing the app-wide defaults every override row offers to follow is M10's settings screen. The seven new instrumentation tests compile in the gate but @@ -316,7 +317,7 @@ Elapsed-realtime anchored. per-timer vibrate, volume-ramp or dismiss-challenge overrides (`timers` has one nullable settings column and four more would be a schema change), no "stop all" when several timers are expired, no undo after a delete (traded for - a confirmation, as with alarms), no ramp on a timer's ring, no presets + a confirmation, as with alarms; added since, #20), no ramp on a timer's ring, no presets management screen and no editing of `default_timer_duration_millis` or `default_timer_ringtone_uri` (M10's settings screen). `TimerSnapshot.anchorIsStale` is still not surfaced in the UI. Of `AlarmClock`'s intent contract only the From ef26d433e175b972cb92e35cb538627261dd6f00 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 20:13:09 +0200 Subject: [PATCH 02/11] fix(alarms): let an unanswered Undo lapse, and make delete and restore atomic A snackbar the screen leaves unanswered (another tab, Settings, the app closing) now counts as expired, except across a rotation. Held back, it could restore a snapshot from hours ago, ring a long-gone timer, or land on top of a backup restored in the meantime. One shared UndoSnackbarEffect does this for alarms and timers. The engines' delete and restore run non-cancellable, so a caller cancelled half-way cannot leave a restored alarm unscheduled. The editor writes the label still in flight before it deletes, so Undo brings back what was typed. The swipe counts the touch-slop overshoot, keeps a card flung back towards the start, slides a card home if its row is still there a second after the delete, and no longer recomposes on every drag frame. --- .../clockula/alarm/AlarmEngine.kt | 26 +++++++--- .../clockula/data/alarms/AlarmRepository.kt | 6 +-- .../clockula/data/timers/TimerRepository.kt | 4 +- .../clockula/timer/TimerEngine.kt | 25 +++++---- .../ui/alarms/AlarmEditorViewModel.kt | 3 ++ .../clockula/ui/alarms/AlarmUndo.kt | 4 +- .../clockula/ui/alarms/AlarmsScreen.kt | 29 ++++------- .../clockula/ui/common/SwipeToDelete.kt | 38 +++++++++++--- .../clockula/ui/common/UndoSnackbar.kt | 51 +++++++++++++++++++ .../clockula/ui/timers/TimersScreen.kt | 26 +++------- .../clockula/ui/timers/TimersViewModel.kt | 4 +- docs/ARCHITECTURE.md | 15 +++--- 12 files changed, 157 insertions(+), 74 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSnackbar.kt 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 82317c3..446975b 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -23,6 +23,7 @@ import de.jeanlucmakiola.clockula.domain.resolveSettings import de.jeanlucmakiola.clockula.domain.time.WallClock import de.jeanlucmakiola.clockula.domain.time.ZoneProvider import de.jeanlucmakiola.clockula.system.RebootRepair +import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged @@ -31,6 +32,7 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext import kotlin.time.Duration.Companion.minutes import kotlin.time.Instant import javax.inject.Inject @@ -141,10 +143,15 @@ class AlarmEngine @Inject constructor( * a ringing alarm's service and auto-silence backstop are global slots * only the engine may touch, and a snoozed alarm's notification would * otherwise outlive the row it belongs to. The ring state goes with the row - * (the foreign key cascades). + * (the foreign key cascades). Non-cancellable, like [restore]: a caller + * cancelled half-way must not leave the slot registered for a deleted row. */ suspend fun delete(alarmId: Long): DeletedAlarm? = lock.withLock { - val alarm = alarms.find(alarmId) ?: return@withLock null + withContext(NonCancellable) { deleteLocked(alarmId) } + } + + private suspend fun deleteLocked(alarmId: Long): DeletedAlarm? { + val alarm = alarms.find(alarmId) ?: return null val state = states.state(alarmId) if (state.ringingSince != null) { ring.stopRinging(alarmId) @@ -153,24 +160,31 @@ class AlarmEngine @Inject constructor( notifier.clearSnoozed(alarmId) alarms.delete(alarmId) rescheduleLocked() - AlarmDeletion.snapshot(alarm, state) + return AlarmDeletion.snapshot(alarm, state) } /** * The Undo: the row back under its own id, its ring state with it, and the * slot re-resolved, which is what re-registers it with AlarmManager. False - * when the id has been taken since (a backup restore in between). + * when the id is taken, which writes nothing. * * A snooze comes back with its notification, but only if the resolver * still holds it pending once the slot is re-resolved: one that lapsed * while the alarm was gone is closed there, like any other. + * + * Non-cancellable: a row restored but never rescheduled would be an alarm + * on the list that AlarmManager does not know about. */ suspend fun restore(deleted: DeletedAlarm): Boolean = lock.withLock { - if (!alarms.restore(deleted.alarm)) return@withLock false + withContext(NonCancellable) { restoreLocked(deleted) } + } + + private suspend fun restoreLocked(deleted: DeletedAlarm): Boolean { + if (!alarms.restore(deleted.alarm)) return false if (deleted.state != AlarmRingState.initial(deleted.alarm.id)) states.save(deleted.state) rescheduleLocked() states.state(deleted.alarm.id).snoozedUntil?.let { until -> notifier.showSnoozed(deleted.alarm, until) } - true + return true } /** Boot repair (timers + stopwatch), then resume any interrupted ring, then reschedule. */ diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmRepository.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmRepository.kt index 05b92d7..143e920 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmRepository.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/alarms/AlarmRepository.kt @@ -15,9 +15,9 @@ interface AlarmRepository { /** * Puts a deleted alarm back under its own id, timestamps and all — the - * Undo after a delete. False when that id is taken (a backup restore - * landed in between), in which case nothing is written. `alarms.id` is - * `AUTOINCREMENT`, so a new alarm can never have claimed it. + * Undo after a delete. `alarms.id` is `AUTOINCREMENT`, so no new alarm can + * have claimed it; should the id be taken all the same, nothing is written + * and the answer is false, rather than a crash or an overwrite. */ suspend fun restore(alarm: Alarm): Boolean suspend fun setEnabled(id: Long, enabled: Boolean) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepository.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepository.kt index 515a0e7..7199a8d 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepository.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/timers/TimerRepository.kt @@ -15,8 +15,8 @@ interface TimerRepository { /** * Puts a deleted timer back under its own id, anchors and place in the - * list — the Undo after a delete. False when that id is taken (a backup - * restore landed in between), in which case nothing is written. + * list — the Undo after a delete. False, writing nothing, should that id + * be taken (`timers.id` is `AUTOINCREMENT`, so no new timer can have it). */ suspend fun restore(timer: Timer): Boolean diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/timer/TimerEngine.kt b/app/src/main/java/de/jeanlucmakiola/clockula/timer/TimerEngine.kt index 71f8155..feafcf3 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/timer/TimerEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/timer/TimerEngine.kt @@ -16,9 +16,11 @@ import de.jeanlucmakiola.clockula.domain.timer.TimerRing import de.jeanlucmakiola.clockula.domain.timer.TimerRingDecision import de.jeanlucmakiola.clockula.domain.timer.TimerRingPolicy import de.jeanlucmakiola.clockula.system.RebootRepair +import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.flow.first import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext import kotlin.time.Duration import kotlin.time.Instant import javax.inject.Inject @@ -185,24 +187,29 @@ class TimerEngine @Inject constructor( * path deliberately: the wrong call should be impossible to write. * * Returns what an Undo would put back ([TimerDeletion.snapshot]), or null - * when there is nothing to put back. + * when there is nothing to put back. Non-cancellable, like [restore]: a + * caller cancelled half-way must not leave the slot or the service behind. */ suspend fun delete(timerId: Long): Timer? = lock.withLock { - val timer = timers.find(timerId) - timers.delete(timerId) - resyncLocked() - timer?.let { TimerDeletion.snapshot(it, elapsed.elapsedRealtime(), wall.now()) } + withContext(NonCancellable) { + val timer = timers.find(timerId) + timers.delete(timerId) + resyncLocked() + timer?.let { TimerDeletion.snapshot(it, elapsed.elapsedRealtime(), wall.now()) } + } } /** * The Undo: the row back under its own id, then the same resync every verb * ends with, which hands the expiry slot and the service the restored timer. - * False when the id has been taken since (a backup restore in between). + * False when the id is taken, which writes nothing. */ suspend fun restore(timer: Timer): Boolean = lock.withLock { - if (!timers.restore(timer)) return@withLock false - resyncLocked() - true + withContext(NonCancellable) { + if (!timers.restore(timer)) return@withContext false + resyncLocked() + true + } } /** 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 7605bf1..c7b9d64 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 @@ -218,6 +218,9 @@ class AlarmEditorViewModel @Inject internal constructor( // this ViewModel, and a delete cut off half-way would leave the slot // registered for an alarm that is no longer there. appScope.launch { + // The label still in flight goes in first, so the Undo brings back + // what the user typed rather than what had been written so far. + labelWrites.value?.let { text -> alarms.edit(alarmId) { it.copy(label = text) } } engine.delete(alarmId)?.let(undo::offer) _state.value = AlarmEditorUiState.Gone } 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 63e04e8..561fbfd 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 @@ -14,8 +14,8 @@ import javax.inject.Singleton * are not the place that offers the Undo. The editor is popped the moment its * alarm is gone, so its ViewModel is cleared before any snackbar could be * answered; the list underneath shows the snackbar instead, and reads the - * deletion from here. Held until the snackbar is answered or times out, so a - * rotation or a trip to another tab re-shows it rather than losing it. + * 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. 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 d630a79..40aad3b 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,8 @@ import androidx.compose.material3.FloatingActionButton import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.SnackbarDuration import androidx.compose.material3.SnackbarHost import androidx.compose.material3.SnackbarHostState -import androidx.compose.material3.SnackbarResult import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect @@ -31,6 +29,7 @@ import androidx.compose.ui.unit.dp import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.clockula.R +import de.jeanlucmakiola.clockula.ui.common.UndoSnackbarEffect import de.jeanlucmakiola.clockula.ui.common.rememberAlarmTimeFormatter import de.jeanlucmakiola.clockula.ui.ring.AlarmRingActivity import de.jeanlucmakiola.clockula.ui.shell.LocalLivePillInset @@ -69,7 +68,6 @@ fun AlarmsScreen( val snackbar = remember { SnackbarHostState() } val formatTime = rememberAlarmTimeFormatter() val undoMessage = pendingUndo?.let { stringResource(R.string.alarm_deleted, formatTime(it.alarm.time)) } - val undoLabel = stringResource(R.string.action_undo) LaunchedEffect(viewModel) { // The FAB creates the alarm and the editor edits a persisted row, so @@ -77,22 +75,15 @@ fun AlarmsScreen( viewModel.openEditor.collectLatest(onCreateAlarm) } - // Keyed on the deletion, so a newer one replaces the snackbar on screen - // (and the older delete stands). The deletion lives in a singleton, not - // here: one made in the editor arrives after the editor has gone, and one - // whose snackbar a rotation or another tab interrupted is shown again. - LaunchedEffect(pendingUndo) { - val deleted = pendingUndo ?: return@LaunchedEffect - val result = snackbar.showSnackbar( - message = undoMessage.orEmpty(), - actionLabel = undoLabel, - duration = SnackbarDuration.Long, - ) - when (result) { - SnackbarResult.ActionPerformed -> viewModel.onUndoDelete(deleted) - SnackbarResult.Dismissed -> viewModel.onUndoExpired(deleted) - } - } + // The deletion lives in a singleton, not here: one made in the editor + // arrives after the editor has gone. + UndoSnackbarEffect( + pending = pendingUndo, + host = snackbar, + message = undoMessage.orEmpty(), + onUndo = viewModel::onUndoDelete, + onExpired = viewModel::onUndoExpired, + ) CollapsingScaffold( title = stringResource(R.string.tab_alarms), 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 ec4f8db..9a858d5 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 @@ -14,6 +14,7 @@ import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable +import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.remember @@ -36,7 +37,9 @@ import androidx.compose.ui.platform.LocalHapticFeedback import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.unit.LayoutDirection import androidx.compose.ui.unit.dp +import kotlinx.coroutines.delay import kotlinx.coroutines.launch +import kotlin.time.Duration.Companion.seconds /** * A card the user can swipe away towards the **end** edge to delete. @@ -71,6 +74,8 @@ internal fun SwipeToDelete( // How far the card has travelled towards the end edge: 0 at rest, never negative. val travel = remember { Animatable(0f) } var width by remember { mutableIntStateOf(0) } + // Derived, so a drag recomposes this once as the red appears, not per frame. + val revealed by remember { derivedStateOf { travel.value > 0f } } Box( modifier = modifier @@ -80,15 +85,20 @@ internal fun SwipeToDelete( val down = awaitFirstDown(requireUnconsumed = false) val velocity = VelocityTracker() velocity.addPointerInputChange(down) + var slopOvershoot = 0f val start = awaitHorizontalTouchSlopOrCancellation(down.id) { change, overSlop -> // Unconsumed, the detection keeps waiting and the pager // gets its chance at the same drag. - if (overSlop * direction > 0f) change.consume() + if (overSlop * direction > 0f) { + change.consume() + slopOvershoot = overSlop * direction + } } ?: return@awaitEachGesture + velocity.addPointerInputChange(start) // Accumulated here, not read back from [travel]: a snap is // launched, so the next delta can arrive before it lands. - var position = travel.value + var position = (travel.value + slopOvershoot).coerceIn(0f, width.toFloat()) var armed = position >= width * DeleteFraction horizontalDrag(start.id) { change -> velocity.addPointerInputChange(change) @@ -104,11 +114,24 @@ internal fun SwipeToDelete( change.consume() } - val flung = velocity.calculateVelocity().x * direction >= flingVelocity + // A fling decides on its own, either way: a card thrown back + // towards the start is kept however far it had gone. + val towardsEnd = velocity.calculateVelocity().x * direction + val delete = when { + towardsEnd >= flingVelocity -> position > 0f + towardsEnd <= -flingVelocity -> false + else -> armed + } scope.launch { - if (width > 0 && (armed || (flung && position > 0f))) { + if (width > 0 && delete) { travel.animateTo(width.toFloat(), settleSpec) currentOnDelete() + // Normally the row has left the list long before this. + // If it is still here (the delete found nothing, or an + // Undo brought the same item back), it slides home + // rather than staying off-screen over the red. + delay(RestoreAfter) + travel.animateTo(0f, settleSpec) } else { travel.animateTo(0f, settleSpec) } @@ -116,7 +139,7 @@ internal fun SwipeToDelete( } }, ) { - if (travel.value > 0f) { + if (revealed) { Box( contentAlignment = Alignment.CenterStart, modifier = Modifier @@ -143,5 +166,8 @@ internal fun SwipeToDelete( /** Past this share of the card's width, letting go deletes. */ private const val DeleteFraction = 0.4f -/** A fling at least this fast towards the end deletes, however short the drag. */ +/** A fling at least this fast deletes (towards the end) or keeps (back), however far the drag. */ private val FlingVelocity = 800.dp + +/** How long a deleted card waits to have left the list before it slides back. */ +private val RestoreAfter = 1.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 new file mode 100644 index 0000000..a3f3881 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSnackbar.kt @@ -0,0 +1,51 @@ +package de.jeanlucmakiola.clockula.ui.common + +import androidx.activity.compose.LocalActivity +import androidx.compose.material3.SnackbarDuration +import androidx.compose.material3.SnackbarHostState +import androidx.compose.material3.SnackbarResult +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.rememberUpdatedState +import androidx.compose.ui.res.stringResource +import de.jeanlucmakiola.clockula.R +import kotlinx.coroutines.CancellationException + +/** + * Shows [pending]'s "deleted · Undo" snackbar on [host] and reports its one + * answer: [onUndo] for the action, [onExpired] for anything else. + * + * Keyed on the deletion, so a newer one replaces the snackbar on screen and the + * older delete stands. A snackbar the screen *leaves* unanswered (another tab, + * 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. + */ +@Composable +internal fun UndoSnackbarEffect( + pending: T?, + host: SnackbarHostState, + message: String, + onUndo: (T) -> Unit, + onExpired: (T) -> Unit, +) { + val activity = LocalActivity.current + val undoLabel = stringResource(R.string.action_undo) + val currentOnUndo by rememberUpdatedState(onUndo) + val currentOnExpired by rememberUpdatedState(onExpired) + LaunchedEffect(pending) { + val deleted = pending ?: return@LaunchedEffect + val result = try { + host.showSnackbar(message = message, actionLabel = undoLabel, duration = SnackbarDuration.Long) + } catch (cancelled: CancellationException) { + if (activity?.isChangingConfigurations != true) currentOnExpired(deleted) + throw cancelled + } + when (result) { + SnackbarResult.ActionPerformed -> currentOnUndo(deleted) + SnackbarResult.Dismissed -> currentOnExpired(deleted) + } + } +} 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 36b1db0..7a9e6a2 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 @@ -12,12 +12,9 @@ import androidx.compose.material.icons.rounded.Settings import androidx.compose.material3.FloatingActionButton import androidx.compose.material3.Icon import androidx.compose.material3.IconButton -import androidx.compose.material3.SnackbarDuration import androidx.compose.material3.SnackbarHost import androidx.compose.material3.SnackbarHostState -import androidx.compose.material3.SnackbarResult import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.remember import androidx.compose.runtime.mutableStateOf @@ -30,6 +27,7 @@ import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.clockula.R import de.jeanlucmakiola.clockula.domain.timer.TimerDurationEntry +import de.jeanlucmakiola.clockula.ui.common.UndoSnackbarEffect import de.jeanlucmakiola.clockula.ui.shell.LocalLivePillInset import de.jeanlucmakiola.floret.components.CollapsingScaffold import de.jeanlucmakiola.floret.identity.animateItemMotion @@ -59,22 +57,14 @@ fun TimersScreen( val pendingUndo by viewModel.pendingUndo.collectAsStateWithLifecycle() val snackbar = remember { SnackbarHostState() } val undoMessage = stringResource(R.string.timer_deleted) - val undoLabel = stringResource(R.string.action_undo) - // Keyed on the deletion, so a newer one replaces the snackbar on screen and - // the older delete stands. - LaunchedEffect(pendingUndo) { - val deleted = pendingUndo ?: return@LaunchedEffect - val result = snackbar.showSnackbar( - message = undoMessage, - actionLabel = undoLabel, - duration = SnackbarDuration.Long, - ) - when (result) { - SnackbarResult.ActionPerformed -> viewModel.onUndoDelete(deleted) - SnackbarResult.Dismissed -> viewModel.onUndoExpired(deleted) - } - } + UndoSnackbarEffect( + pending = pendingUndo, + host = snackbar, + message = undoMessage, + onUndo = viewModel::onUndoDelete, + onExpired = viewModel::onUndoExpired, + ) val loaded = state as? TimersUiState.Loaded val rows = loaded?.rows 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 3d07d9f..3a5e142 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 @@ -85,8 +85,8 @@ class TimersViewModel @Inject internal constructor( /** * The deleted timer the snackbar offers to bring back. Null when none. Held - * here, not in the screen, so a rotation or a trip to another tab re-shows - * the snackbar rather than losing the Undo; a newer delete replaces it. + * 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() diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8eec71d..fd381a6 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1740,17 +1740,18 @@ alarm is the exception: deleting it was a dismissal in all but name, so it comes back as a dismissal would have left it (cycle closed, a one-shot switched off) and never rings again, and a ringing transient alarm, which that dismissal would have deleted anyway, offers no Undo at all. A snooze that lapsed while the alarm -was gone is closed by the resolver on restore, and gets no notification. An id -that a backup restore claimed in the meantime makes the restore write nothing. +was gone is closed by the resolver on restore, and gets no notification. The pending deletion lives in `AlarmUndo`, a singleton, not in either ViewModel. The editor is popped the moment its alarm is gone, so its ViewModel is cleared before any snackbar could be answered; the list underneath reads the -deletion from the singleton and shows the snackbar. It is cleared only when the -snackbar is answered or times out, or a newer deletion replaces it, so a -rotation or a trip to another tab re-shows the snackbar rather than losing the -Undo. It is memory only: an Undo that outlived process death would be offered -with no snackbar anyone remembers. +deletion from the singleton and shows the snackbar (`UndoSnackbarEffect`). It +is cleared when the snackbar is answered or times out, when a newer deletion +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. The swipe is hand-rolled (`ui/common/SwipeToDelete`) rather than M3's `SwipeToDismissBox`, because the tabs are pages of a `HorizontalPager` and the From af12b7bb34798df3286bb1dd0392a63e78aa47a1 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 20:22:25 +0200 Subject: [PATCH 03/11] fix(alarms): keep a ringing card out of the swipe, and Undo off the ViewModel scope A ringing alarm's card no longer swipes or offers Delete: it leads to the ring screen, and a swipe must not get round the dismiss challenge. A drag cancelled before release (a ring screen opening on top) never deletes. Undo restores on the application scope, since a claimed Undo has no second chance. Closing a cycle and deleting share one teardown. --- .../clockula/alarm/AlarmEngine.kt | 25 +++++++++++-------- .../clockula/ui/alarms/AlarmRow.kt | 19 ++++++++------ .../clockula/ui/alarms/AlarmsViewModel.kt | 12 +++++++-- .../clockula/ui/common/SwipeToDelete.kt | 9 +++++-- .../clockula/ui/timers/TimersViewModel.kt | 13 ++++++++-- .../clockula/ui/alarms/AlarmsViewModelTest.kt | 1 + .../clockula/ui/timers/TimersViewModelTest.kt | 1 + docs/ARCHITECTURE.md | 4 ++- 8 files changed, 60 insertions(+), 24 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 446975b..561465d 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -153,11 +153,7 @@ class AlarmEngine @Inject constructor( private suspend fun deleteLocked(alarmId: Long): DeletedAlarm? { val alarm = alarms.find(alarmId) ?: return null val state = states.state(alarmId) - if (state.ringingSince != null) { - ring.stopRinging(alarmId) - scheduler.cancelAutoSilence() - } - notifier.clearSnoozed(alarmId) + silenceLocked(alarmId, state) alarms.delete(alarmId) rescheduleLocked() return AlarmDeletion.snapshot(alarm, state) @@ -406,11 +402,7 @@ class AlarmEngine @Inject constructor( * with its backstop intact. */ private suspend fun closeCycleLocked(alarmId: Long, state: AlarmRingState) { - if (state.ringingSince != null) { - ring.stopRinging(alarmId) - scheduler.cancelAutoSilence() - } - notifier.clearSnoozed(alarmId) + silenceLocked(alarmId, state) val closed = state.copy(ringingSince = null, snoozedUntil = null, snoozeCount = 0) if (closed != state) states.save(closed) @@ -419,4 +411,17 @@ class AlarmEngine @Inject constructor( if (alarm != null && !alarm.repeatDays.isRepeating) disableOrDeleteLocked(alarm) rescheduleLocked() } + + /** + * Everything audible or visible an alarm's cycle holds: the ring service and + * its backstop if it is the one ringing, and its snooze notification. The + * one teardown, for closing a cycle and for deleting the alarm. + */ + private fun silenceLocked(alarmId: Long, state: AlarmRingState) { + if (state.ringingSince != null) { + ring.stopRinging(alarmId) + scheduler.cancelAutoSilence() + } + notifier.clearSnoozed(alarmId) + } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt index 7e3752b..4b300d7 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt @@ -48,7 +48,9 @@ import java.util.Locale * * Swiping it towards the end edge deletes it, and so does the card's "Delete" * custom action, which is the same verb for a screen reader or switch access, - * where a swipe is not to be had. The list's snackbar offers the Undo. + * where a swipe is not to be had. The list's snackbar offers the Undo. Neither + * is offered while the alarm rings: the card leads to the ring screen then, and + * a swipe must not become a way round its dismiss challenge. */ @Composable internal fun AlarmRow( @@ -76,16 +78,19 @@ internal fun AlarmRow( onDelete = onDelete, shape = RoundedCornerShape(SwipeBackgroundCorner), modifier = modifier.padding(horizontal = GroupedListInset), + enabled = !row.ringing, ) { GroupedSurface( position = Position.Alone, modifier = Modifier.semantics { - customActions = listOf( - CustomAccessibilityAction(deleteAction) { - onDelete() - true - }, - ) + if (!row.ringing) { + customActions = listOf( + CustomAccessibilityAction(deleteAction) { + onDelete() + true + }, + ) + } }, onClick = onClick, color = if (row.enabled) { 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 00f33c4..09b14a4 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 @@ -10,6 +10,8 @@ import de.jeanlucmakiola.clockula.domain.AlarmDraft import de.jeanlucmakiola.clockula.domain.alarm.DeletedAlarm import de.jeanlucmakiola.clockula.domain.time.WallClock import de.jeanlucmakiola.clockula.domain.time.ZoneProvider +import de.jeanlucmakiola.floret.di.ApplicationScope +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted @@ -30,6 +32,8 @@ class AlarmsViewModel @Inject internal constructor( private val zones: ZoneProvider, private val source: AlarmListSource, private val undo: AlarmUndo, + // `@param:` — see AlarmEditorViewModel. + @param:ApplicationScope private val appScope: CoroutineScope, ) : ViewModel() { val state: StateFlow = source.rows() @@ -90,10 +94,14 @@ class AlarmsViewModel @Inject internal constructor( viewModelScope.launch { engine.delete(alarmId)?.let(undo::offer) } } - /** The snackbar's Undo. Claimed first, so a double tap restores once. */ + /** + * The snackbar's Undo. Claimed first, so a double tap restores once, and + * restored on the application scope: once claimed there is no second + * chance, so a back press that clears this ViewModel must not drop it. + */ fun onUndoDelete(deleted: DeletedAlarm) { if (!undo.take(deleted)) return - viewModelScope.launch { engine.restore(deleted) } + appScope.launch { engine.restore(deleted) } } /** The snackbar timed out or was swiped away: the delete stands. */ 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 9a858d5..9ef68e2 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 @@ -61,6 +61,7 @@ internal fun SwipeToDelete( onDelete: () -> Unit, shape: Shape, modifier: Modifier = Modifier, + enabled: Boolean = true, content: @Composable () -> Unit, ) { val currentOnDelete by rememberUpdatedState(onDelete) @@ -80,7 +81,8 @@ internal fun SwipeToDelete( Box( modifier = modifier .onSizeChanged { width = it.width } - .pointerInput(direction, flingVelocity) { + .pointerInput(direction, flingVelocity, enabled) { + if (!enabled) return@pointerInput awaitEachGesture { val down = awaitFirstDown(requireUnconsumed = false) val velocity = VelocityTracker() @@ -100,7 +102,7 @@ internal fun SwipeToDelete( // launched, so the next delta can arrive before it lands. var position = (travel.value + slopOvershoot).coerceIn(0f, width.toFloat()) var armed = position >= width * DeleteFraction - horizontalDrag(start.id) { change -> + val released = horizontalDrag(start.id) { change -> velocity.addPointerInputChange(change) position = (position + change.positionChange().x * direction) .coerceIn(0f, width.toFloat()) @@ -118,6 +120,9 @@ internal fun SwipeToDelete( // towards the start is kept however far it had gone. val towardsEnd = velocity.calculateVelocity().x * direction val delete = when { + // Cancelled rather than let go (a ring screen opening + // on top, the shade pulled): nothing was decided. + !released -> false towardsEnd >= flingVelocity -> position > 0f towardsEnd <= -flingVelocity -> false else -> armed 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 3a5e142..3e67c16 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 @@ -9,6 +9,8 @@ import de.jeanlucmakiola.clockula.domain.Timer import de.jeanlucmakiola.clockula.domain.TimerDraft import de.jeanlucmakiola.clockula.domain.timer.TimerPresets import de.jeanlucmakiola.clockula.timer.TimerEngine +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 @@ -31,6 +33,9 @@ class TimersViewModel @Inject internal constructor( private val engine: TimerEngine, private val settings: SettingsPrefs, source: TimerListSource, + // `@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, ) : ViewModel() { /** `WhileSubscribed(5_000)`, seeded [TimersUiState.Loading]. */ @@ -98,10 +103,14 @@ class TimersViewModel @Inject internal constructor( engine.delete(timerId)?.let { undoSlot.value = it } } - /** The snackbar's Undo. Claimed first, so a double tap restores once. */ + /** + * The snackbar's Undo. Claimed first, so a double tap restores once, and + * restored on the application scope: once claimed there is no second + * chance, so a back press that clears this ViewModel must not drop it. + */ fun onUndoDelete(timer: Timer) { if (!undoSlot.compareAndSet(timer, null)) return - launchOnEngine { engine.restore(timer) } + appScope.launch { engine.restore(timer) } } /** The snackbar timed out or was swiped away: the delete stands. */ 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 9abed55..9ca7e8b 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 @@ -53,6 +53,7 @@ class AlarmsViewModelTest { zones = harness.zones, source = AlarmListSource(harness.engine, harness.wallClock, harness.ticker), undo = undo, + appScope = this, ).also { viewModel -> backgroundScope.launch { viewModel.state.collect {} } } private fun AlarmsUiState.rows(): List = (this as AlarmsUiState.Loaded).rows 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 bc90f75..0a6c0b6 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), + appScope = this, ).also { viewModel -> backgroundScope.launch { viewModel.state.collect {} } } private fun TimersUiState.loaded(): TimersUiState.Loaded = this as TimersUiState.Loaded diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index fd381a6..fd46477 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1718,7 +1718,9 @@ be a lie; the user who wants that has the enable switch. An alarm is deleted by swiping its card towards the end edge, by the card's **Delete** custom accessibility action (the same verb for TalkBack and switch access, where there is no swipe), or from the editor's top bar. None of them -asks first. Each is followed by a snackbar on the list ("07:00 alarm deleted", +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", **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 From 62da327771e9929a2110c3ed71b130061e0808ab Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 07:44:28 +0200 Subject: [PATCH 04/11] fix(alarms): bring a card home when its swipe is disabled mid-gesture A card whose alarm starts ringing during a drag or the throw-away animation slides back instead of staying displaced, and the delete is re-checked after the animation, so it cannot land on a ringing alarm. --- .../clockula/ui/common/SwipeToDelete.kt | 12 +++++++++++- docs/ARCHITECTURE.md | 5 ++++- 2 files changed, 15 insertions(+), 2 deletions(-) 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 9ef68e2..680651a 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 @@ -14,6 +14,7 @@ import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf @@ -65,6 +66,7 @@ internal fun SwipeToDelete( content: @Composable () -> Unit, ) { val currentOnDelete by rememberUpdatedState(onDelete) + val currentEnabled by rememberUpdatedState(enabled) val scope = rememberCoroutineScope() val haptics = LocalHapticFeedback.current val settleSpec = MaterialTheme.motionScheme.defaultSpatialSpec() @@ -78,6 +80,12 @@ 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. + LaunchedEffect(enabled) { + if (!enabled) travel.animateTo(0f, settleSpec) + } + Box( modifier = modifier .onSizeChanged { width = it.width } @@ -130,7 +138,9 @@ internal fun SwipeToDelete( scope.launch { if (width > 0 && delete) { travel.animateTo(width.toFloat(), settleSpec) - currentOnDelete() + // Re-read after the animation: the card may have been + // disabled meanwhile (its alarm started ringing). + if (currentEnabled) currentOnDelete() // Normally the row has left the list long before this. // If it is still here (the delete found nothing, or an // Undo brought the same item back), it slides home diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index fd46477..33a736d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1742,7 +1742,10 @@ alarm is the exception: deleting it was a dismissal in all but name, so it comes back as a dismissal would have left it (cycle closed, a one-shot switched off) and never rings again, and a ringing transient alarm, which that dismissal would have deleted anyway, offers no Undo at all. A snooze that lapsed while the alarm -was gone is closed by the resolver on restore, and gets no notification. +was gone is closed by the resolver on restore, and gets no notification. An +alarm deleted just before its time and restored just after it rings at once, +since the occurrence is still inside the fire grace window: Undo means "as if it +had not been deleted", and that alarm would have rung. The pending deletion lives in `AlarmUndo`, a singleton, not in either ViewModel. The editor is popped the moment its alarm is gone, so its ViewModel From 697678b2260d4cefc5f0bb939ab8811c3c1d150a Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 07:54:43 +0200 Subject: [PATCH 05/11] 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 From 5f3b97b32a3918f69ad54c5606edbfa704cbd865 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 08:05:25 +0200 Subject: [PATCH 06/11] fix(alarms): refuse a list delete of a ringing alarm in the engine The list's delete re-checks for a ring under the engine lock, so a swipe released as the alarm fires cannot silence it round its dismiss challenge. An Undo whose snackbar is already up no longer goes stale on a rotation after a long accessibility timeout, and a deleted card is thrown clear of the screen edge. --- .../clockula/alarm/AlarmEngine.kt | 12 +++++++++--- .../clockula/ui/alarms/AlarmsScreen.kt | 1 + .../clockula/ui/alarms/AlarmsViewModel.kt | 7 ++++++- .../clockula/ui/common/SwipeToDelete.kt | 7 ++++++- .../clockula/ui/common/UndoSlot.kt | 18 ++++++++++++++++-- .../clockula/ui/common/UndoSnackbar.kt | 4 +++- .../clockula/ui/timers/TimersScreen.kt | 1 + .../clockula/ui/timers/TimersViewModel.kt | 3 +++ .../clockula/alarm/AlarmEngineDeleteTest.kt | 12 ++++++++++++ .../clockula/ui/alarms/AlarmsViewModelTest.kt | 14 +++----------- .../clockula/ui/common/UndoSlotTest.kt | 9 +++++++++ docs/ARCHITECTURE.md | 4 +++- 12 files changed, 72 insertions(+), 20 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 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 From 7adda73a79894683bd29db9468446039edc2ae41 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 08:15:52 +0200 Subject: [PATCH 07/11] fix(alarms): keep the editor's Delete off a ringing alarm too With its confirmation gone, one tap on the editor's Delete would have silenced a ringing alarm round its dismiss challenge, which the list's swipe is already kept from doing; the ring screen is the way out. The list's delete now runs on the application scope like the editor's, a thrown-away card ignores a new drag, and the timers snackbar clears the live pill on an empty tab. --- .../ui/alarms/AlarmEditorViewModel.kt | 13 +++++++--- .../clockula/ui/alarms/AlarmsViewModel.kt | 4 ++- .../clockula/ui/common/SwipeToDelete.kt | 2 ++ .../clockula/ui/timers/TimersScreen.kt | 9 ++++++- .../ui/alarms/AlarmEditorViewModelTest.kt | 25 ++++++++++--------- docs/ARCHITECTURE.md | 11 +++++--- 6 files changed, 42 insertions(+), 22 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 c7b9d64..b88ca71 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 @@ -208,8 +208,8 @@ class AlarmEditorViewModel @Inject internal constructor( } /** - * Deletes through the engine, which silences a ring first (M5 D11), and the - * state becomes [AlarmEditorUiState.Gone]. No confirmation: the editor pops + * Deletes through the engine, and the state becomes + * [AlarmEditorUiState.Gone]. A ringing alarm is left alone. No confirmation: the editor pops * and the list underneath offers the Undo, through [AlarmUndo] because this * ViewModel is about to be cleared. */ @@ -221,8 +221,13 @@ class AlarmEditorViewModel @Inject internal constructor( // The label still in flight goes in first, so the Undo brings back // what the user typed rather than what had been written so far. labelWrites.value?.let { text -> alarms.edit(alarmId) { it.copy(label = text) } } - engine.delete(alarmId)?.let(undo::offer) - _state.value = AlarmEditorUiState.Gone + // A ringing alarm is not deleted from here either: with no + // confirmation in the way, one tap would silence it round its + // dismiss challenge. Its ring screen is the way out. + engine.delete(alarmId, unlessRinging = true)?.let { deleted -> + undo.offer(deleted) + _state.value = AlarmEditorUiState.Gone + } } } 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 fe51639..0998fd4 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 @@ -93,7 +93,9 @@ class AlarmsViewModel @Inject internal constructor( fun onDelete(alarmId: Long) { // 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) } + // On the application scope: the card has already been thrown away, so + // a back press while the engine lock is busy must not drop the delete. + appScope.launch { engine.delete(alarmId, unlessRinging = true)?.let(undo::offer) } } /** 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 fc7f79d..b369e0e 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 @@ -94,6 +94,8 @@ internal fun SwipeToDelete( if (!enabled) return@pointerInput awaitEachGesture { val down = awaitFirstDown(requireUnconsumed = false) + // Thrown away and waiting to leave the list: not a card to swipe. + if (travel.value >= width) return@awaitEachGesture val velocity = VelocityTracker() velocity.addPointerInputChange(down) var slopOvershoot = 0f 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 f01c9cb..8a255f5 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 @@ -75,7 +75,14 @@ fun TimersScreen( title = stringResource(R.string.tab_timers), modifier = modifier, scrollable = false, - snackbarHost = { SnackbarHost(snackbar) }, + // Above the pill: over a list the FAB (padded by the same inset) lifts + // the snackbar, but an empty tab has no FAB to stand on. + snackbarHost = { + SnackbarHost( + snackbar, + modifier = Modifier.padding(bottom = if (rows.isNullOrEmpty()) pillInset else 0.dp), + ) + }, actions = { IconButton(onClick = onOpenSettings) { Icon( 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 c010e57..850b677 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 @@ -15,7 +15,6 @@ import de.jeanlucmakiola.clockula.testing.FakeRingtonePreviewer import de.jeanlucmakiola.clockula.testing.MainDispatcherExtension import de.jeanlucmakiola.clockula.testing.PreviewEvent import de.jeanlucmakiola.clockula.ui.common.RingtoneChoice -import de.jeanlucmakiola.clockula.testing.RingEvent import de.jeanlucmakiola.clockula.testing.alarmAt import de.jeanlucmakiola.clockula.testing.alarmEngineHarness import de.jeanlucmakiola.clockula.testing.instantAt @@ -643,19 +642,21 @@ class AlarmEditorViewModelTest { } @Test - fun `deleting the ringing alarm silences it first`(@TempDir tempDir: Path) = runTest(main.dispatcher) { - val harness = alarmEngineHarness(tempDir, now = fiveToday) - harness.givenAlarmWith(hour = 7) - harness.givenRinging(alarmId = 1L, at = sevenToday) - val viewModel = editor(harness) - advanceUntilIdle() + fun `the ringing alarm is not deleted round its ring screen`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val harness = alarmEngineHarness(tempDir, now = fiveToday) + harness.givenAlarmWith(hour = 7) + harness.givenRinging(alarmId = 1L, at = sevenToday) + val viewModel = editor(harness) + advanceUntilIdle() - viewModel.onDelete() - advanceUntilIdle() + viewModel.onDelete() + advanceUntilIdle() - assertThat(harness.ring.events.last() to harness.alarmDao.stored) - .isEqualTo(RingEvent.Stopped(1L) to emptyList()) - } + assertThat(listOf(harness.ring.ringing.value, harness.alarmDao.stored.map { it.id })) + .isEqualTo(listOf(1L, listOf(1L))) + assertThat(viewModel.state.value).isInstanceOf(AlarmEditorUiState.Editing::class.java) + } @Test fun `deleting hands the alarm to the list's undo, and the undo puts it back`(@TempDir tempDir: Path) = diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 7b42473..9d1a60d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1682,14 +1682,15 @@ auto-silence backstop are single *global* slots (§11), touched only by `closeCycleLocked`, and `onAutoSilence` returns early once `ringingSince` is null. So switching off or deleting a ringing alarm from this screen would clear its state and leave it **sounding until the 15-minute wake-lock timeout, with -the backstop disarmed by its own guard**. Both ViewModels therefore call +the backstop disarmed by its own guard**. The list therefore calls ```kotlin if (engine.ringSession()?.alarm?.id == alarmId) engine.dismiss(alarmId) ``` -before `setEnabled(id, false)`. Deleting goes further and is an engine verb of -its own (`AlarmEngine.delete`, below), which stops the ring itself. The check reads storage under +before `setEnabled(id, false)`. Deleting is an engine verb of its own +(`AlarmEngine.delete`, below), which stops a ring itself but, from the list and +the editor, refuses a ringing alarm outright. The check reads storage under the engine's own mutex, so it cannot race the fire it is asking about, and it is deliberately **conditional**: dismissing unconditionally would also drop a merely *snoozed* alarm's pending snooze, which is wrong for a label edit. @@ -1722,7 +1723,9 @@ 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. 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", +deletes nothing and the card slides back. The editor's Delete passes the same +flag: with its confirmation gone, one tap there would otherwise do what the +swipe is kept from doing, so a ringing alarm is dealt with on its ring screen. 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 From ddec2d8ed1809415dd35e6d1b58fb83658261da7 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 08:25:12 +0200 Subject: [PATCH 08/11] fix(alarms): open the ring screen when the editor's Delete is refused Deleting a ringing alarm from the editor is refused, and a refused tap did nothing visible. It now opens the ring screen, the same way out the list's card offers. --- .../clockula/ui/alarms/AlarmEditorScreen.kt | 14 ++++++++++++++ .../clockula/ui/alarms/AlarmEditorViewModel.kt | 15 ++++++++++++++- .../ui/alarms/AlarmEditorViewModelTest.kt | 2 ++ docs/ARCHITECTURE.md | 2 +- 4 files changed, 31 insertions(+), 2 deletions(-) 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 1b3fa7a..82e143c 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 @@ -1,5 +1,6 @@ package de.jeanlucmakiola.clockula.ui.alarms +import android.content.Intent import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.foundation.layout.Spacer @@ -19,6 +20,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.dp @@ -27,6 +29,7 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.clockula.R import de.jeanlucmakiola.clockula.domain.TimeOfDay import de.jeanlucmakiola.clockula.ui.common.RingtonePickerScreen +import de.jeanlucmakiola.clockula.ui.ring.AlarmRingActivity import de.jeanlucmakiola.clockula.ui.common.rememberAlarmTimeFormatter import de.jeanlucmakiola.clockula.ui.shell.LocalLivePillInset import de.jeanlucmakiola.floret.components.CollapsingScaffold @@ -75,6 +78,17 @@ fun AlarmEditorScreen( if (state is AlarmEditorUiState.Gone) onBack() } + val context = LocalContext.current + LaunchedEffect(viewModel) { + // Delete is refused while the alarm rings; its ring screen, with the + // dismiss challenge, is the way out, as it is from the list's card. + viewModel.openRingScreen.collect { + context.startActivity( + Intent(context, AlarmRingActivity::class.java).addFlags(Intent.FLAG_ACTIVITY_NEW_TASK), + ) + } + } + CollapsingScaffold( title = stringResource(R.string.editor_title), onBack = onBack, 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 b88ca71..0b2ba8b 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,11 +18,14 @@ 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.channels.Channel +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.receiveAsFlow import kotlinx.coroutines.launch import java.time.DayOfWeek import javax.inject.Inject @@ -62,6 +65,12 @@ class AlarmEditorViewModel @Inject internal constructor( private val _state = MutableStateFlow(AlarmEditorUiState.Loading) + // Conflated: two refused taps open the ring screen once. + private val ringScreenRequests = Channel(Channel.CONFLATED) + + /** One-shot: a Delete refused because the alarm is ringing; the screen opens the ring screen. */ + val openRingScreen: Flow = ringScreenRequests.receiveAsFlow() + val state: StateFlow = _state.asStateFlow() private val _ringtonePicker = MutableStateFlow(null) @@ -224,9 +233,13 @@ class AlarmEditorViewModel @Inject internal constructor( // A ringing alarm is not deleted from here either: with no // confirmation in the way, one tap would silence it round its // dismiss challenge. Its ring screen is the way out. - engine.delete(alarmId, unlessRinging = true)?.let { deleted -> + val deleted = engine.delete(alarmId, unlessRinging = true) + if (deleted != null) { undo.offer(deleted) _state.value = AlarmEditorUiState.Gone + } else if (alarms.find(alarmId) != null) { + // Refused because it rings: take the user to where it stops. + ringScreenRequests.trySend(Unit) } } } 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 850b677..219793a 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 app.cash.turbine.test import com.google.common.truth.Truth.assertThat import de.jeanlucmakiola.clockula.data.alarms.AlarmMapper import de.jeanlucmakiola.clockula.domain.DismissChallenge @@ -656,6 +657,7 @@ class AlarmEditorViewModelTest { assertThat(listOf(harness.ring.ringing.value, harness.alarmDao.stored.map { it.id })) .isEqualTo(listOf(1L, listOf(1L))) assertThat(viewModel.state.value).isInstanceOf(AlarmEditorUiState.Editing::class.java) + viewModel.openRingScreen.test { assertThat(awaitItem()).isEqualTo(Unit) } } @Test diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 9d1a60d..5f7b50b 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1725,7 +1725,7 @@ 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. The editor's Delete passes the same flag: with its confirmation gone, one tap there would otherwise do what the -swipe is kept from doing, so a ringing alarm is dealt with on its ring screen. Each is followed by a snackbar on the list ("07:00 alarm deleted", +swipe is kept from doing, so a refused tap opens the ring screen instead, where the alarm is dealt with. 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 From 40c71a49f7334cc566930579ca024dd2c3cce014 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 08:35:14 +0200 Subject: [PATCH 09/11] fix(timers): delete on the application scope, and wait for storage in the delete tests The timer list's delete now outlives a back press the same way the alarm list's does. The instrumented delete and Undo tests wait for the row to change in storage, because that work runs outside Compose's idling. --- .../clockula/ui/AlarmsInstrumentedTest.kt | 13 ++++++++++--- .../clockula/ui/timers/TimersViewModel.kt | 6 ++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/app/src/androidTest/java/de/jeanlucmakiola/clockula/ui/AlarmsInstrumentedTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/clockula/ui/AlarmsInstrumentedTest.kt index 0ec1af1..ace87ca 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/clockula/ui/AlarmsInstrumentedTest.kt +++ b/app/src/androidTest/java/de/jeanlucmakiola/clockula/ui/AlarmsInstrumentedTest.kt @@ -209,11 +209,13 @@ class AlarmsInstrumentedTest { // No confirmation: the list underneath offers the Undo instead. composeTestRule.onNodeWithContentDescription(string(R.string.editor_delete)).performClick() + // The delete runs on the application scope, which Compose's idling + // does not track, so the test waits for storage rather than for idle. + composeTestRule.waitUntil(STORAGE_TIMEOUT_MS) { stored(id) == null } composeTestRule.waitForIdle() composeTestRule.onNodeWithText("Work").assertDoesNotExist() composeTestRule.onNodeWithText(string(R.string.action_undo)).assertIsDisplayed() - assertThat(stored(id)).isNull() } @OptIn(ExperimentalTestApi::class) @@ -225,13 +227,18 @@ class AlarmsInstrumentedTest { composeTestRule.onNodeWithText("Work") .performCustomAccessibilityActionWithLabel(string(R.string.action_delete)) + composeTestRule.waitUntil(STORAGE_TIMEOUT_MS) { stored(id) == null } composeTestRule.waitForIdle() - assertThat(stored(id)).isNull() composeTestRule.onNodeWithText(string(R.string.action_undo)).performClick() + composeTestRule.waitUntil(STORAGE_TIMEOUT_MS) { stored(id) != null } composeTestRule.waitForIdle() composeTestRule.onNodeWithText("Work").assertIsDisplayed() - assertThat(stored(id)).isNotNull() + } + + private companion object { + /** Delete and restore run on the application scope, outside Compose's idling. */ + const val STORAGE_TIMEOUT_MS = 5_000L } } 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 234130a..b672fde 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 @@ -100,8 +100,10 @@ class TimersViewModel @Inject internal constructor( * 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(undo::offer) + fun onDelete(timerId: Long) { + // On the application scope, as the alarm list's is: a back press while + // the engine lock is busy must not drop a delete the user saw happen. + appScope.launch { engine.delete(timerId)?.let(undo::offer) } } /** From ba1172b82292c9f5bdfee4332f7c64d647d97e01 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 08:43:26 +0200 Subject: [PATCH 10/11] fix(alarms): keep a settling card from overshooting past its rest --- .../java/de/jeanlucmakiola/clockula/ui/common/SwipeToDelete.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 b369e0e..548ee40 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 @@ -75,7 +75,8 @@ internal fun SwipeToDelete( val flingVelocity = with(LocalDensity.current) { FlingVelocity.toPx() } // How far the card has travelled towards the end edge: 0 at rest, never negative. - val travel = remember { Animatable(0f) } + // Bounded below, so the settle spring cannot overshoot the card past its rest. + val travel = remember { Animatable(0f).apply { updateBounds(lowerBound = 0f) } } var width by remember { mutableIntStateOf(0) } // Derived, so a drag recomposes this once as the red appears, not per frame. val revealed by remember { derivedStateOf { travel.value > 0f } } From c2a583fda728d16e767a3be9a50e063de0d33364 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 26 Oct 2026 21:51:38 +0100 Subject: [PATCH 11/11] fix(alarms): offer the delete Undo as the family's SnackChip The alarm and timer lists show the Undo in floret-kit's SnackChip beside the FAB instead of the stock M3 snackbar, as Agendula and Calendula do. --- .../clockula/ui/alarms/AlarmRow.kt | 2 +- .../clockula/ui/alarms/AlarmUndo.kt | 8 +- .../clockula/ui/alarms/AlarmsScreen.kt | 154 ++++++++------- .../clockula/ui/alarms/AlarmsViewModel.kt | 12 +- .../clockula/ui/common/UndoChip.kt | 116 +++++++++++ .../clockula/ui/common/UndoSlot.kt | 16 +- .../clockula/ui/common/UndoSnackbar.kt | 60 ------ .../clockula/ui/timers/TimerRow.kt | 2 +- .../clockula/ui/timers/TimersScreen.kt | 186 ++++++++++-------- .../clockula/ui/timers/TimersViewModel.kt | 14 +- .../clockula/ui/alarms/AlarmsViewModelTest.kt | 2 +- docs/ARCHITECTURE.md | 18 +- 12 files changed, 333 insertions(+), 257 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoChip.kt delete mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSnackbar.kt diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt index 4b300d7..f51ce83 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt @@ -48,7 +48,7 @@ import java.util.Locale * * Swiping it towards the end edge deletes it, and so does the card's "Delete" * custom action, which is the same verb for a screen reader or switch access, - * where a swipe is not to be had. The list's snackbar offers the Undo. Neither + * where a swipe is not to be had. The list's chip offers the Undo. Neither * is offered while the alarm rings: the card leads to the ring screen then, and * a swipe must not become a way round its dismiss challenge. */ 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 ce8b844..51a70f8 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 @@ -11,10 +11,10 @@ import javax.inject.Singleton * * A singleton rather than ViewModel state, because the two places that delete * are not the place that offers the Undo. The editor is popped the moment its - * alarm is gone, so its ViewModel is cleared before any snackbar could be - * 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]). + * alarm is gone, so its ViewModel is cleared before any chip could be + * answered; the list underneath shows the chip instead, and reads the + * deletion from here. Held until the chip is answered, times out or is + * left unanswered ([de.jeanlucmakiola.clockula.ui.common.UndoChipBand]). */ @Singleton 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 fcee832..7643e26 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 @@ -3,9 +3,15 @@ package de.jeanlucmakiola.clockula.ui.alarms import android.content.Intent import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box 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 @@ -29,7 +33,7 @@ import androidx.compose.ui.unit.dp import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.clockula.R -import de.jeanlucmakiola.clockula.ui.common.UndoSnackbarEffect +import de.jeanlucmakiola.clockula.ui.common.UndoChipBand import de.jeanlucmakiola.clockula.ui.common.rememberAlarmTimeFormatter import de.jeanlucmakiola.clockula.ui.ring.AlarmRingActivity import de.jeanlucmakiola.clockula.ui.shell.LocalLivePillInset @@ -65,7 +69,6 @@ fun AlarmsScreen( val pillInset = LocalLivePillInset.current val addAlarm = stringResource(R.string.alarms_add) val pendingUndo by viewModel.pendingUndo.collectAsStateWithLifecycle() - val snackbar = remember { SnackbarHostState() } val formatTime = rememberAlarmTimeFormatter() val undoMessage = pendingUndo?.let { stringResource(R.string.alarm_deleted, formatTime(it.alarm.time)) } @@ -75,77 +78,82 @@ fun AlarmsScreen( viewModel.openEditor.collectLatest(onCreateAlarm) } - // The deletion lives in a singleton, not here: one made in the editor - // arrives after the editor has gone. - UndoSnackbarEffect( - pending = pendingUndo, - host = snackbar, - message = undoMessage.orEmpty(), - onUndo = viewModel::onUndoDelete, - onExpired = viewModel::onUndoExpired, - isStale = viewModel::isUndoStale, - onShown = viewModel::onUndoShown, - ) - - CollapsingScaffold( - title = stringResource(R.string.tab_alarms), - modifier = modifier, - scrollable = false, - snackbarHost = { SnackbarHost(snackbar) }, - 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) - } - }, - ) { - 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) }, - // No confirmation: the snackbar's Undo is the safety net. - onDelete = { viewModel.onDelete(row.id) }, - // 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) }, + // No confirmation: the chip's Undo is the safety net. + onDelete = { viewModel.onDelete(row.id) }, + // An alarm whose time changed slides to its new place + // instead of teleporting. + modifier = animateItemMotion(), + ) + } } } + // The deletion lives in a singleton, not here: one made in the editor + // arrives after the editor has gone. 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. + UndoChipBand( + pending = pendingUndo, + message = undoMessage.orEmpty(), + onUndo = viewModel::onUndoDelete, + onExpired = viewModel::onUndoExpired, + isStale = viewModel::isUndoStale, + onShown = viewModel::onUndoShown, + 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 0998fd4..b9eebdd 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 @@ -82,11 +82,11 @@ class AlarmsViewModel @Inject internal constructor( viewModelScope.launch { engine.dismissUpcoming(alarmId) } } - /** The deletion the list's snackbar offers to undo, from the swipe or the editor. Null when none. */ + /** The deletion the list's chip offers to undo, from the swipe or the editor. Null when none. */ val pendingUndo: StateFlow = undo.pending /** - * The swipe's delete. No confirmation: the snackbar's Undo is the safety + * The swipe's delete. No confirmation: the chip's Undo is the safety * net, and it catches the same slip a dialog would without taxing every * delete the user meant. */ @@ -99,7 +99,7 @@ class AlarmsViewModel @Inject internal constructor( } /** - * The snackbar's Undo. Claimed first, so a double tap restores once, and + * The chip's Undo. Claimed first, so a double tap restores once, and * restored on the application scope: once claimed there is no second * chance, so a back press that clears this ViewModel must not drop it. */ @@ -108,15 +108,15 @@ class AlarmsViewModel @Inject internal constructor( appScope.launch { engine.restore(deleted) } } - /** The snackbar timed out or was swiped away: the delete stands. */ + /** The chip timed out: the delete stands. */ fun onUndoExpired(deleted: DeletedAlarm) { undo.take(deleted) } - /** Offered too long ago to show a snackbar for now ([AlarmUndo.isStale]). */ + /** Offered too long ago to show a chip for now ([AlarmUndo.isStale]). */ fun isUndoStale(deleted: DeletedAlarm): Boolean = undo.isStale(deleted) - /** The snackbar is up; only its answer settles the deletion now. */ + /** The chip 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. */ diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoChip.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoChip.kt new file mode 100644 index 0000000..bbff826 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoChip.kt @@ -0,0 +1,116 @@ +package de.jeanlucmakiola.clockula.ui.common + +import androidx.activity.compose.LocalActivity +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.res.stringResource +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.dp +import de.jeanlucmakiola.clockula.R +import de.jeanlucmakiola.floret.components.SnackChip +import de.jeanlucmakiola.floret.components.SnackChipHeight +import de.jeanlucmakiola.floret.components.SnackChipMargin +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.withTimeoutOrNull + +/** + * Shows [pending]'s "deleted · Undo" as the family's [SnackChip], in a + * FAB-height band along the bottom edge, and reports its one answer: [onUndo] + * for the action, [onExpired] for anything else. + * + * Keyed on the deletion, so a newer one replaces the chip on screen and the + * older delete stands. A chip the screen *leaves* unanswered (another tab, 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 chip 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 chip. + * + * The chip carries no timer of its own, so the window is a long snackbar's, + * stretched the way the accessibility settings ask. [endClearance] is what the + * band keeps free at its end for a FAB sharing it. + */ +@Composable +internal fun UndoChipBand( + pending: T?, + message: String, + onUndo: (T) -> Unit, + onExpired: (T) -> Unit, + isStale: (T) -> Boolean, + onShown: (T) -> Unit, + modifier: Modifier = Modifier, + endClearance: Dp = 0.dp, +) { + val activity = LocalActivity.current + val accessibility = LocalAccessibilityManager.current + val currentMessage by rememberUpdatedState(message) + val currentOnUndo by rememberUpdatedState(onUndo) + val currentOnExpired by rememberUpdatedState(onExpired) + // Kept after the chip hides, so its exit animation still has words in it: + // the message is built from the deletion, which an Undo clears at once. + var shownMessage by remember { mutableStateOf("") } + // Non-null exactly while an Undo is on offer; the action completes it. + var offer by remember { mutableStateOf?>(null) } + LaunchedEffect(pending) { + val deleted = pending ?: return@LaunchedEffect + if (isStale(deleted)) { + currentOnExpired(deleted) + return@LaunchedEffect + } + onShown(deleted) + val tapped = CompletableDeferred() + shownMessage = currentMessage + offer = tapped + val window = accessibility?.calculateRecommendedTimeoutMillis( + UNDO_WINDOW_MILLIS, + containsIcons = false, + containsText = true, + containsControls = true, + ) ?: UNDO_WINDOW_MILLIS + val undone = try { + withTimeoutOrNull(window) { tapped.await() } != null + } catch (cancelled: CancellationException) { + if (activity?.isChangingConfigurations != true) currentOnExpired(deleted) + throw cancelled + } finally { + if (offer === tapped) offer = null + } + if (undone) currentOnUndo(deleted) else currentOnExpired(deleted) + } + BoxWithConstraints( + modifier = modifier + .fillMaxWidth() + .padding(start = SnackChipMargin, end = SnackChipMargin, bottom = SnackChipMargin) + .height(SnackChipHeight), + contentAlignment = Alignment.CenterStart, + ) { + SnackChip( + visible = offer != null, + message = shownMessage, + modifier = Modifier.fillMaxHeight(), + actionLabel = stringResource(R.string.action_undo), + onAction = { offer?.complete(Unit) }, + maxWidth = maxWidth - endClearance, + color = MaterialTheme.colorScheme.secondaryContainer, + ) + } +} + +/** A long snackbar's window. */ +private const val UNDO_WINDOW_MILLIS = 10_000L 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 b314020..9e0c183 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 @@ -8,11 +8,11 @@ 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. + * The one deletion an Undo chip can still bring back: offered by a delete, + * claimed once by the chip's answer. * * Deliberately memory only: an Undo that survived process death would be - * offered with no snackbar anyone remembers. + * offered with no chip anyone remembers. */ open class UndoSlot(private val clock: ElapsedRealtimeClock) { @@ -21,14 +21,14 @@ 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. */ + /** The offer a chip 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. */ + /** A newer deletion replaces an older one: only the latest chip is on screen. */ fun offer(deleted: T) { offeredAt = clock.elapsedRealtime() shown = null @@ -38,12 +38,12 @@ open class UndoSlot(private val clock: ElapsedRealtimeClock) { /** * 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. + * stale chip 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. + * Whether [deleted] was offered too long ago to put a chip 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. Never @@ -53,7 +53,7 @@ open class UndoSlot(private val clock: ElapsedRealtimeClock) { 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 chip 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) { 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 deleted file mode 100644 index adf26e2..0000000 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSnackbar.kt +++ /dev/null @@ -1,60 +0,0 @@ -package de.jeanlucmakiola.clockula.ui.common - -import androidx.activity.compose.LocalActivity -import androidx.compose.material3.SnackbarDuration -import androidx.compose.material3.SnackbarHostState -import androidx.compose.material3.SnackbarResult -import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.getValue -import androidx.compose.runtime.rememberUpdatedState -import androidx.compose.ui.res.stringResource -import de.jeanlucmakiola.clockula.R -import kotlinx.coroutines.CancellationException - -/** - * Shows [pending]'s "deleted · Undo" snackbar on [host] and reports its one - * answer: [onUndo] for the action, [onExpired] for anything else. - * - * Keyed on the deletion, so a newer one replaces the snackbar on screen and the - * older delete stands. A snackbar the screen *leaves* unanswered (another tab, - * 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. 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( - pending: T?, - host: SnackbarHostState, - message: String, - onUndo: (T) -> Unit, - onExpired: (T) -> Unit, - isStale: (T) -> Boolean, - onShown: (T) -> Unit, -) { - val activity = LocalActivity.current - val undoLabel = stringResource(R.string.action_undo) - val currentOnUndo by rememberUpdatedState(onUndo) - val currentOnExpired by rememberUpdatedState(onExpired) - LaunchedEffect(pending) { - val deleted = pending ?: return@LaunchedEffect - if (isStale(deleted)) { - currentOnExpired(deleted) - return@LaunchedEffect - } - onShown(deleted) - val result = try { - host.showSnackbar(message = message, actionLabel = undoLabel, duration = SnackbarDuration.Long) - } catch (cancelled: CancellationException) { - if (activity?.isChangingConfigurations != true) currentOnExpired(deleted) - throw cancelled - } - when (result) { - SnackbarResult.ActionPerformed -> currentOnUndo(deleted) - SnackbarResult.Dismissed -> currentOnExpired(deleted) - } - } -} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimerRow.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimerRow.kt index 93d02bf..12a03a2 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimerRow.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimerRow.kt @@ -67,7 +67,7 @@ import kotlin.time.Duration.Companion.minutes * never from enlarging text", and timer progress is one of the three places §8 * sanctions a showpiece (D29). * - * The top-right delete button asks nothing first: the list's snackbar offers + * The top-right delete button asks nothing first: the list's chip offers * an Undo that puts the timer back still counting, which covers the slip a * dialog would without slowing every delete the user meant. */ 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 8a255f5..2351077 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 @@ -1,9 +1,16 @@ package de.jeanlucmakiola.clockula.ui.timers +import androidx.compose.animation.core.animateDpAsState import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box 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 @@ -12,14 +19,12 @@ import androidx.compose.material.icons.rounded.Settings import androidx.compose.material3.FloatingActionButton import androidx.compose.material3.Icon import androidx.compose.material3.IconButton -import androidx.compose.material3.SnackbarHost -import androidx.compose.material3.SnackbarHostState import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue -import androidx.compose.runtime.remember import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp @@ -27,9 +32,11 @@ import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.clockula.R import de.jeanlucmakiola.clockula.domain.timer.TimerDurationEntry -import de.jeanlucmakiola.clockula.ui.common.UndoSnackbarEffect +import de.jeanlucmakiola.clockula.ui.common.UndoChipBand import de.jeanlucmakiola.clockula.ui.shell.LocalLivePillInset import de.jeanlucmakiola.floret.components.CollapsingScaffold +import de.jeanlucmakiola.floret.components.SnackChipHeight +import de.jeanlucmakiola.floret.components.SnackChipMargin import de.jeanlucmakiola.floret.identity.animateItemMotion /** @@ -55,98 +62,103 @@ fun TimersScreen( val pillInset = LocalLivePillInset.current val addTimer = stringResource(R.string.timers_set_title) val pendingUndo by viewModel.pendingUndo.collectAsStateWithLifecycle() - val snackbar = remember { SnackbarHostState() } val undoMessage = stringResource(R.string.timer_deleted) - UndoSnackbarEffect( - pending = pendingUndo, - host = snackbar, - message = undoMessage, - onUndo = viewModel::onUndoDelete, - onExpired = viewModel::onUndoExpired, - isStale = viewModel::isUndoStale, - onShown = viewModel::onUndoShown, - ) - val loaded = state as? TimersUiState.Loaded val rows = loaded?.rows + val hasFab = rows != null && rows.isNotEmpty() + // Deleting the last timer leaves the keypad on screen, and the Undo chip + // would sit on its bottom row: the keypad, which shrinks to fit, gives up + // the chip's band while an Undo is on offer. + val undoBand by animateDpAsState( + targetValue = if (!hasFab && pendingUndo != null) SnackChipHeight + SnackChipMargin else 0.dp, + label = "undo-band", + ) - CollapsingScaffold( - title = stringResource(R.string.tab_timers), - modifier = modifier, - scrollable = false, - // Above the pill: over a list the FAB (padded by the same inset) lifts - // the snackbar, but an empty tab has no FAB to stand on. - snackbarHost = { - SnackbarHost( - snackbar, - modifier = Modifier.padding(bottom = if (rows.isNullOrEmpty()) pillInset else 0.dp), - ) - }, - actions = { - IconButton(onClick = onOpenSettings) { - Icon( - imageVector = Icons.Rounded.Settings, - contentDescription = stringResource(R.string.settings_title), - ) - } - }, - floatingActionButton = { - // Only over a list: on an empty tab the keypad is already on - // screen, and a FAB that opens what is in front of you is noise. - if (rows != null && rows.isNotEmpty()) { - FloatingActionButton( - onClick = onAddTimer, - // 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 = addTimer) - } - } - }, - ) { - if (rows != null && rows.isEmpty()) { - // Outside the list, not an item of it: a lazy item has unbounded - // height, and the keypad needs the real height to fit itself into. - InlineTimerSetup( - onStartNew = viewModel::onStartNew, - modifier = Modifier - .fillMaxSize() - .padding(bottom = pillInset), - ) - } else { - LazyColumn( - modifier = Modifier.fillMaxSize(), - // Disconnected cards, exactly as the Alarms tab's own list is: each - // timer is its own object, not a row of a table. - verticalArrangement = Arrangement.spacedBy(12.dp), - contentPadding = PaddingValues( - top = TimersDefaults.ListBottomPadding, - bottom = TimersDefaults.ListBottomPadding + TimersDefaults.FabBand + pillInset, - ), - ) { - items(rows.orEmpty(), key = { row -> row.id }) { row -> - TimerRow( - row = row, - onOpen = { onEditTimer(row.id) }, - onStart = { viewModel.onStart(row.id) }, - onPause = { viewModel.onPause(row.id) }, - onReset = { viewModel.onReset(row.id) }, - onAddMinute = { viewModel.onAddMinute(row.id) }, - onSubtractMinute = { viewModel.onSubtractMinute(row.id) }, - // No confirmation: the snackbar's Undo puts the timer back, - // still counting (D2's delete still routes through the - // engine, so a ringing timer is stopped cleanly rather than - // deleted into a stuck ring). - onDelete = { viewModel.onDelete(row.id) }, - // A timer that finishes stays where it is (D22); the motion - // is for a row arriving or leaving. - modifier = animateItemMotion(), + Box(modifier = modifier.fillMaxSize()) { + CollapsingScaffold( + title = stringResource(R.string.tab_timers), + scrollable = false, + actions = { + IconButton(onClick = onOpenSettings) { + Icon( + imageVector = Icons.Rounded.Settings, + contentDescription = stringResource(R.string.settings_title), ) } + }, + floatingActionButton = { + // Only over a list: on an empty tab the keypad is already on + // screen, and a FAB that opens what is in front of you is noise. + if (hasFab) { + FloatingActionButton( + onClick = onAddTimer, + // 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 = addTimer) + } + } + }, + ) { + if (rows != null && rows.isEmpty()) { + // Outside the list, not an item of it: a lazy item has unbounded + // height, and the keypad needs the real height to fit itself into. + InlineTimerSetup( + onStartNew = viewModel::onStartNew, + modifier = Modifier + .fillMaxSize() + .padding(bottom = pillInset + undoBand), + ) + } else { + LazyColumn( + modifier = Modifier.fillMaxSize(), + // Disconnected cards, exactly as the Alarms tab's own list is: each + // timer is its own object, not a row of a table. + verticalArrangement = Arrangement.spacedBy(12.dp), + contentPadding = PaddingValues( + top = TimersDefaults.ListBottomPadding, + bottom = TimersDefaults.ListBottomPadding + TimersDefaults.FabBand + pillInset, + ), + ) { + items(rows.orEmpty(), key = { row -> row.id }) { row -> + TimerRow( + row = row, + onOpen = { onEditTimer(row.id) }, + onStart = { viewModel.onStart(row.id) }, + onPause = { viewModel.onPause(row.id) }, + onReset = { viewModel.onReset(row.id) }, + onAddMinute = { viewModel.onAddMinute(row.id) }, + onSubtractMinute = { viewModel.onSubtractMinute(row.id) }, + // No confirmation: the chip's Undo puts the timer back, + // still counting (D2's delete still routes through the + // engine, so a ringing timer is stopped cleanly rather than + // deleted into a stuck ring). + onDelete = { viewModel.onDelete(row.id) }, + // A timer that finishes stays where it is (D22); the motion + // is for a row arriving or leaving. + modifier = animateItemMotion(), + ) + } + } } } + // Beside the FAB over a list, measured from the same insets the scaffold + // gives it; above the pill either way. + UndoChipBand( + pending = pendingUndo, + message = undoMessage, + onUndo = viewModel::onUndoDelete, + onExpired = viewModel::onUndoExpired, + isStale = viewModel::isUndoStale, + onShown = viewModel::onUndoShown, + endClearance = if (hasFab) TimersDefaults.FabBand else 0.dp, + modifier = Modifier + .align(Alignment.BottomStart) + .windowInsetsPadding(WindowInsets.systemBars.only(WindowInsetsSides.Bottom)) + .padding(bottom = pillInset), + ) } } 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 b672fde..8fcd972 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 @@ -90,15 +90,15 @@ class TimersViewModel @Inject internal constructor( 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 + * The deleted timer the chip offers to bring back. Null when none. Held + * here, not in the screen, so a rotation re-shows the chip rather than * losing the Undo; a newer delete replaces it. */ 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. + * ring (D2). Still no confirmation; the chip's Undo now covers the slip. */ fun onDelete(timerId: Long) { // On the application scope, as the alarm list's is: a back press while @@ -107,7 +107,7 @@ class TimersViewModel @Inject internal constructor( } /** - * The snackbar's Undo. Claimed first, so a double tap restores once, and + * The chip's Undo. Claimed first, so a double tap restores once, and * restored on the application scope: once claimed there is no second * chance, so a back press that clears this ViewModel must not drop it. */ @@ -116,15 +116,15 @@ class TimersViewModel @Inject internal constructor( appScope.launch { engine.restore(timer) } } - /** The snackbar timed out or was swiped away: the delete stands. */ + /** The chip timed out: the delete stands. */ fun onUndoExpired(timer: Timer) { undo.take(timer) } - /** Offered too long ago to show a snackbar for now ([UndoSlot.isStale]). */ + /** Offered too long ago to show a chip for now ([UndoSlot.isStale]). */ fun isUndoStale(timer: Timer): Boolean = undo.isStale(timer) - /** The snackbar is up; only its answer settles the deletion now. */ + /** The chip is up; only its answer settles the deletion now. */ fun onUndoShown(timer: Timer) = undo.markShown(timer) fun onSavePreset(duration: Duration) { 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 fcb246a..c09c6cb 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 @@ -407,7 +407,7 @@ class AlarmsViewModelTest { } @Test - fun `a timed-out snackbar lets the delete stand`(@TempDir tempDir: Path) = runTest(main.dispatcher) { + fun `a timed-out Undo chip lets the delete stand`(@TempDir tempDir: Path) = runTest(main.dispatcher) { val harness = alarmEngineHarness(tempDir) harness.givenAlarm(id = 1L, hour = 7) val viewModel = viewModel(harness) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 5f7b50b..476b3a9 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1725,10 +1725,10 @@ 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. The editor's Delete passes the same flag: with its confirmation gone, one tap there would otherwise do what the -swipe is kept from doing, so a refused tap opens the ring screen instead, where the alarm is dealt with. Each is followed by a snackbar on the list ("07:00 alarm deleted", +swipe is kept from doing, so a refused tap opens the ring screen instead, where the alarm is dealt with. Each is followed by a chip 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 +than the chip does: both only catch the slip noticed in the moment, and a confirmation the user taps through out of habit catches less. The Undo does it without taxing every delete the user meant, so the editor's dialog went too rather than leaving two delete paths with different safety nets. @@ -1754,13 +1754,13 @@ had not been deleted", and that alarm would have rung. The pending deletion lives in `AlarmUndo`, a singleton, not in either ViewModel. The editor is popped the moment its alarm is gone, so its ViewModel -is cleared before any snackbar could be answered; the list underneath reads the -deletion from the singleton and shows the snackbar (`UndoSnackbarEffect`). It -is cleared when the snackbar is answered or times out, when a newer deletion -replaces it, and when the screen is left with the snackbar unanswered (another +is cleared before any chip could be answered; the list underneath reads the +deletion from the singleton and shows the chip (`UndoChipBand`). It +is cleared when the chip is answered or times out, when a newer deletion +replaces it, and when the screen is left with the chip 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 +was restored meanwhile. A rotation is the one exception and shows the chip 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 @@ -1808,7 +1808,7 @@ already met once, and the cheapest way not to have it again is to make the wrong call impossible to write. **Configuration** edits (`rename`, `setDuration`, `setRingtoneUri`) stay on `TimerRepository` and the editor calls them directly. -A card's corner delete asks nothing first, and the list's snackbar offers an +A card's corner delete asks nothing first, and the list's chip offers an **Undo**. `TimerEngine.delete` returns the row as `TimerDeletion.snapshot` decided, and `TimerEngine.restore` puts it back under its own id and place and resyncs. A running timer comes back with its anchors, so it has kept counting @@ -1816,7 +1816,7 @@ while it was gone, and one whose end passed in the meantime rings, as it would have had it never been deleted. A timer that had already finished comes back reset: deleting it silenced it, which is a dismissal, so it does not ring a second time (a finished transient timer offers no Undo). The pending row is -`TimersViewModel` state, since both the delete and the snackbar are on the +`TimersViewModel` state, since both the delete and the chip are on the list; the editor's delete keeps its confirmation. `TimerRepository` deliberately has **no generic `edit(id, transform)`** the way