diff --git a/CHANGELOG.md b/CHANGELOG.md index 02ae0ba..a7580d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,10 @@ The first release of Clockula: alarms, timers, a stopwatch and a world clock. - Setting an alarm, switching it on or changing its time or repeat days says how long until it rings, such as "Alarm set for 7 hours and 12 minutes from now", so an AM/PM or wrong-day slip shows at once. +- 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. - An "Upcoming alarm" notification two hours before an alarm rings, with 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..ace87ca 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,38 @@ 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() + // 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() - assertThat(stored(id)).isNull() + composeTestRule.onNodeWithText(string(R.string.action_undo)).assertIsDisplayed() + } + + @OptIn(ExperimentalTestApi::class) + @Test + fun theRowsDeleteActionRemovesItAndUndoBringsItBack() { + val id = givenAlarm() + launch() + composeTestRule.waitForIdle() + + composeTestRule.onNodeWithText("Work") + .performCustomAccessibilityActionWithLabel(string(R.string.action_delete)) + composeTestRule.waitUntil(STORAGE_TIMEOUT_MS) { stored(id) == null } + composeTestRule.waitForIdle() + + composeTestRule.onNodeWithText(string(R.string.action_undo)).performClick() + composeTestRule.waitUntil(STORAGE_TIMEOUT_MS) { stored(id) != null } + composeTestRule.waitForIdle() + + composeTestRule.onNodeWithText("Work").assertIsDisplayed() + } + + 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/alarm/AlarmEngine.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt index be5722f..72847a2 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -5,11 +5,13 @@ 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.AlarmSchedule +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.RingSession @@ -212,6 +214,61 @@ 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). 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, unlessRinging: Boolean = false): DeletedAlarm? = lock.withLock { + withContext(NonCancellable) { deleteLocked(alarmId, unlessRinging) } + } + + 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() + 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 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 { + 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) } + return true + } + /** Boot repair (timers + stopwatch), then resume any interrupted ring, then reschedule. */ suspend fun onBootCompleted(): ScheduledFire? = lock.withLock { rebootRepair.repairIfRebooted() @@ -575,11 +632,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) @@ -602,6 +655,19 @@ class AlarmEngine @Inject constructor( } 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) + } } /** What one reschedule pass hands the upcoming-alarm sync (#16). */ 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 8af9f12..ab28293 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 06cfd4c..077a9ca 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 @@ -14,6 +14,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. `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) 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 c90a23f..c9e0c1c 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 @@ -38,6 +38,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..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 @@ -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, writing nothing, should that id + * be taken (`timers.id` is `AUTOINCREMENT`, so no new timer can have it). + */ + 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 26dbbcb..bf8464f 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 @@ -200,10 +202,31 @@ 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. Non-cancellable, like [restore]: a + * caller cancelled half-way must not leave the slot or the service behind. */ - suspend fun delete(timerId: Long): Unit = lock.withLock { - timers.delete(timerId) - resyncLocked() + suspend fun delete(timerId: Long): Timer? = lock.withLock { + 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 is taken, which writes nothing. + */ + suspend fun restore(timer: Timer): Boolean = lock.withLock { + withContext(NonCancellable) { + if (!timers.restore(timer)) return@withContext 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 6869a63..d7f4539 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 @@ -7,13 +8,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 @@ -21,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 @@ -29,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 @@ -58,8 +59,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 @@ -80,6 +79,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, @@ -96,7 +106,8 @@ fun AlarmEditorScreen( }, 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), @@ -113,18 +124,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( @@ -403,23 +402,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 21da7bd..3424624 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 @@ -23,6 +23,7 @@ import de.jeanlucmakiola.clockula.ui.common.RingtonePickerUiState import de.jeanlucmakiola.floret.di.ApplicationScope import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -30,6 +31,7 @@ import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock @@ -56,6 +58,7 @@ class AlarmEditorViewModel @Inject internal constructor( private val announcer: AlarmSetAnnouncer, private val wallClock: WallClock, private val zones: ZoneProvider, + 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, @@ -79,6 +82,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) @@ -325,13 +334,32 @@ 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, 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. + */ fun onDelete() { + // On the application scope, in tap order with the other schedule + // writes: 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. scheduleWrite { - if (engine.ringSession()?.alarm?.id == alarmId) engine.dismiss(alarmId) - alarms.delete(alarmId) - engine.reschedule() - _state.value = AlarmEditorUiState.Gone + // 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) } } + // 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. + 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/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt index a738832..0616540 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 @@ -11,6 +11,7 @@ import androidx.compose.material.icons.rounded.AlarmOff import androidx.compose.material3.AssistChip import androidx.compose.material3.AssistChipDefaults import androidx.compose.material3.Icon +import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Switch import androidx.compose.material3.Text @@ -21,7 +22,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 @@ -36,6 +39,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 @@ -48,18 +52,26 @@ 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 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. */ @Composable internal fun AlarmRow( row: AlarmRowState, onClick: () -> Unit, onToggle: (Boolean) -> Unit, + onDelete: () -> Unit, modifier: Modifier = Modifier, onDismissComing: () -> Unit = {}, ) { 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 { @@ -70,80 +82,99 @@ 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 - }, + enabled = !row.ringing, ) { - Column( - modifier = Modifier.fillMaxWidth().padding(horizontal = 20.dp, vertical = 20.dp), - verticalArrangement = Arrangement.spacedBy(8.dp), + GroupedSurface( + position = Position.Alone, + modifier = Modifier.semantics { + if (!row.ringing) { + 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 }, - ) - } - 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, - ) - // Inside the upcoming notification's window, its Dismiss is here - // too, for whoever opens the app instead (#16). - if (row.enabled && row.dismissibleFireAt != null) { - val dismissDescription = stringResource(R.string.alarm_row_dismiss_upcoming, time) - AssistChip( - onClick = onDismissComing, - label = { Text(stringResource(R.string.alarm_action_dismiss)) }, - leadingIcon = { - Icon( - imageVector = Icons.Rounded.AlarmOff, - contentDescription = null, - modifier = Modifier.size(AssistChipDefaults.IconSize), - ) - }, - // "Dismiss the alarm at 07:00": the bare word names neither - // the row nor that it is the coming ring, not this one. - modifier = Modifier.semantics { contentDescription = dismissDescription }, + text = rowSummary(row), + style = MaterialTheme.typography.bodyMedium, + color = supportingColor, ) + // Inside the upcoming notification's window, its Dismiss is here + // too, for whoever opens the app instead (#16). + if (row.enabled && row.dismissibleFireAt != null) { + val dismissDescription = stringResource(R.string.alarm_row_dismiss_upcoming, time) + AssistChip( + onClick = onDismissComing, + label = { Text(stringResource(R.string.alarm_action_dismiss)) }, + leadingIcon = { + Icon( + imageVector = Icons.Rounded.AlarmOff, + contentDescription = null, + modifier = Modifier.size(AssistChipDefaults.IconSize), + ) + }, + // "Dismiss the alarm at 07:00": the bare word names neither + // the row nor that it is the coming ring, not this one. + modifier = Modifier.semantics { contentDescription = dismissDescription }, + ) + } } } } } +/** 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..51a70f8 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmUndo.kt @@ -0,0 +1,20 @@ +package de.jeanlucmakiola.clockula.ui.alarms + +import de.jeanlucmakiola.clockula.domain.alarm.DeletedAlarm +import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock +import de.jeanlucmakiola.clockula.ui.common.UndoSlot +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 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 ca2df9d..c89a6ea 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 @@ -4,6 +4,7 @@ import android.content.Intent import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.WindowInsetsSides @@ -31,6 +32,8 @@ 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.UndoChipBand +import de.jeanlucmakiola.clockula.ui.common.rememberAlarmTimeFormatter import de.jeanlucmakiola.clockula.ui.ring.AlarmRingActivity import de.jeanlucmakiola.clockula.ui.shell.LocalLivePillInset import de.jeanlucmakiola.clockula.ui.shell.ShellOverflowMenu @@ -65,6 +68,9 @@ fun AlarmsScreen( val context = LocalContext.current val pillInset = LocalLivePillInset.current val addAlarm = stringResource(R.string.alarms_add) + val pendingUndo by viewModel.pendingUndo.collectAsStateWithLifecycle() + val formatTime = rememberAlarmTimeFormatter() + val undoMessage = pendingUndo?.let { stringResource(R.string.alarm_deleted, formatTime(it.alarm.time)) } LaunchedEffect(viewModel) { // The FAB creates the alarm and the editor edits a persisted row, so @@ -120,6 +126,8 @@ fun AlarmsScreen( onDismissComing = { row.dismissibleFireAt?.let { viewModel.onDismissComing(row.id, it) } }, + // 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(), @@ -129,14 +137,31 @@ fun AlarmsScreen( } // Beside the FAB, not stacked above it as a snackbar would be: the band is // the FAB's own, measured from the same insets the scaffold gives it. - AlarmSetChipBand( - notice = viewModel.alarmSetNotice, - onShown = viewModel::onAlarmSetNoticeShown, + val chipBand = Modifier + .align(Alignment.BottomStart) + .windowInsetsPadding(WindowInsets.systemBars.only(WindowInsetsSides.Bottom)) + .padding(bottom = pillInset) + // One chip at a time in the band: an Undo still open has it, and an + // "Alarm in …" waits for it, kept until a screen has shown it. + if (pendingUndo == null) { + AlarmSetChipBand( + notice = viewModel.alarmSetNotice, + onShown = viewModel::onAlarmSetNoticeShown, + endClearance = AlarmsDefaults.FabBand, + modifier = chipBand, + ) + } + // The deletion lives in a singleton, not here: one made in the editor + // arrives after the editor has gone. + 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), + modifier = chipBand, ) } 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 0005bef..f219225 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,8 +7,11 @@ 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 de.jeanlucmakiola.floret.di.ApplicationScope +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted @@ -21,7 +24,7 @@ import kotlinx.coroutines.launch import kotlin.time.Instant 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, @@ -30,6 +33,9 @@ class AlarmsViewModel @Inject internal constructor( private val zones: ZoneProvider, private val source: AlarmListSource, private val announcer: AlarmSetAnnouncer, + private val undo: AlarmUndo, + // `@param:` — see AlarmEditorViewModel. + @param:ApplicationScope private val appScope: CoroutineScope, ) : ViewModel() { val state: StateFlow = source.rows() @@ -100,6 +106,43 @@ class AlarmsViewModel @Inject internal constructor( viewModelScope.launch { engine.dismissUpcomingOccurrence(alarmId, fireAt) } } + /** 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 chip'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) { + // 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. + // 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) } + } + + /** + * 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. + */ + fun onUndoDelete(deleted: DeletedAlarm) { + if (!undo.take(deleted)) return + appScope.launch { engine.restore(deleted) } + } + + /** The chip timed out: the delete stands. */ + fun onUndoExpired(deleted: DeletedAlarm) { + undo.take(deleted) + } + + /** Offered too long ago to show a chip for now ([AlarmUndo.isStale]). */ + fun isUndoStale(deleted: DeletedAlarm): Boolean = undo.isStale(deleted) + + /** 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. */ 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..548ee40 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/SwipeToDelete.kt @@ -0,0 +1,197 @@ +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.LaunchedEffect +import androidx.compose.runtime.derivedStateOf +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.delay +import kotlinx.coroutines.launch +import kotlin.time.Duration.Companion.seconds + +/** + * 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, + enabled: Boolean = true, + 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() + // +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. + // 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 } } + + // 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 (travel.value > 0f) travel.animateTo(0f, settleSpec) + } + + Box( + modifier = modifier + .onSizeChanged { width = it.width } + .pointerInput(direction, flingVelocity, enabled) { + 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 + 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() + 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 + slopOvershoot).coerceIn(0f, width.toFloat()) + var armed = position >= width * DeleteFraction + val released = 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() + } + + // 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 { + // 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 + } + scope.launch { + if (width > 0 && delete) { + // 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() + // 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) + } + } + } + }, + ) { + if (revealed) { + 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() + } + } +} + +/** 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 + +/** 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/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 new file mode 100644 index 0000000..9e0c183 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/common/UndoSlot.kt @@ -0,0 +1,67 @@ +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 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 chip anyone remembers. + */ +open class UndoSlot(private val clock: ElapsedRealtimeClock) { + + private val slot = MutableStateFlow(null) + + @Volatile + private var offeredAt: Duration = Duration.ZERO + + /** 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 chip is on screen. */ + fun offer(deleted: T) { + offeredAt = clock.elapsedRealtime() + shown = null + 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 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 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 + * true once [markShown]. + */ + fun isStale(deleted: T): Boolean = + slot.value == deleted && shown != deleted && clock.elapsedRealtime() - offeredAt > SHOW_WITHIN + + /** + * 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) { + if (slot.value == deleted) shown = deleted + } + + 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/timers/TimerRow.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/timers/TimerRow.kt index d99eb4e..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,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 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. */ @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 4681572..44f4ded 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 @@ -15,6 +22,7 @@ import androidx.compose.runtime.getValue 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 @@ -22,9 +30,12 @@ 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.UndoChipBand import de.jeanlucmakiola.clockula.ui.shell.LocalLivePillInset import de.jeanlucmakiola.clockula.ui.shell.ShellOverflowMenu import de.jeanlucmakiola.floret.components.CollapsingScaffold +import de.jeanlucmakiola.floret.components.SnackChipHeight +import de.jeanlucmakiola.floret.components.SnackChipMargin import de.jeanlucmakiola.floret.identity.animateItemMotion /** @@ -49,73 +60,99 @@ 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 undoMessage = stringResource(R.string.timer_deleted) 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, - actions = { - ShellOverflowMenu(onOpenSettings = onOpenSettings) - }, - 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) + Box(modifier = modifier.fillMaxSize()) { + CollapsingScaffold( + title = stringResource(R.string.tab_timers), + scrollable = false, + actions = { + ShellOverflowMenu(onOpenSettings = onOpenSettings) + }, + 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), - ) - } 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: 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). - 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(), - ) + }, + ) { + 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 513164e..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 @@ -5,9 +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.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.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine @@ -28,6 +33,10 @@ 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, ) : ViewModel() { /** `WhileSubscribed(5_000)`, seeded [TimersUiState.Loading]. */ @@ -78,8 +87,45 @@ 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 undo = UndoSlot(elapsed) + + /** + * 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 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 + // the engine lock is busy must not drop a delete the user saw happen. + appScope.launch { engine.delete(timerId)?.let(undo::offer) } + } + + /** + * 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. + */ + fun onUndoDelete(timer: Timer) { + if (!undo.take(timer)) return + appScope.launch { engine.restore(timer) } + } + + /** The chip timed out: the delete stands. */ + fun onUndoExpired(timer: Timer) { + undo.take(timer) + } + + /** Offered too long ago to show a chip for now ([UndoSlot.isStale]). */ + fun isUndoStale(timer: Timer): Boolean = undo.isStale(timer) + + /** The chip 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/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 749ce8c..3138ce7 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -69,6 +69,7 @@ Sound Delete timer Delete this timer? + Timer deleted Timers @@ -166,9 +167,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..b1756c1 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineDeleteTest.kt @@ -0,0 +1,141 @@ +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 `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) + 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 856aaa0..ce261f0 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeAlarmSeams.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeAlarmSeams.kt @@ -229,6 +229,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 4469e06..6562263 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 } } } @@ -133,6 +136,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/AlarmEditorScheduleTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScheduleTest.kt index de014d6..60792c2 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScheduleTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmEditorScheduleTest.kt @@ -46,6 +46,7 @@ class AlarmEditorScheduleTest { ringtones = FakeRingtoneCatalog(), preview = FakeRingtonePreviewer(), announcer = AlarmSetAnnouncer(harness.engine, harness.wallClock, FakeElapsedRealtimeClock(), appScope = this), + undo = AlarmUndo(FakeElapsedRealtimeClock()), wallClock = harness.wallClock, zones = harness.zones, appScope = this, 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 83ba6f2..3a7be1d 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 @@ -19,7 +19,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 @@ -69,6 +68,7 @@ class AlarmEditorViewModelTest { harness: AlarmEngineHarness, alarmId: Long = 1L, announcer: AlarmSetAnnouncer = announcer(harness), + undo: AlarmUndo = AlarmUndo(FakeElapsedRealtimeClock()), ) = AlarmEditorViewModel( savedStateHandle = SavedStateHandle(mapOf(AlarmRoutes.ARG_ALARM_ID to alarmId)), @@ -80,6 +80,7 @@ class AlarmEditorViewModelTest { announcer = announcer, wallClock = harness.wallClock, zones = harness.zones, + undo = undo, appScope = this, ) @@ -957,19 +958,43 @@ 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) + viewModel.openRingScreen.test { assertThat(awaitItem()).isEqualTo(Unit) } + } + + @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(FakeElapsedRealtimeClock()) + 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) { 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 4e556df..a94c671 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 @@ -52,6 +52,7 @@ class AlarmsViewModelTest { private fun TestScope.viewModel( harness: AlarmEngineHarness, announcer: AlarmSetAnnouncer = AlarmSetAnnouncer(harness.engine, harness.wallClock, FakeElapsedRealtimeClock(), this), + undo: AlarmUndo = AlarmUndo(FakeElapsedRealtimeClock()), ) = AlarmsViewModel( alarms = harness.alarms, engine = harness.engine, @@ -59,6 +60,8 @@ class AlarmsViewModelTest { zones = harness.zones, source = AlarmListSource(harness.engine, harness.wallClock, harness.ticker, harness.zones), announcer = announcer, + undo = undo, + appScope = this, ).also { viewModel -> backgroundScope.launch { viewModel.state.collect {} } } private fun AlarmsUiState.rows(): List = (this as AlarmsUiState.Loaded).rows @@ -469,6 +472,96 @@ 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 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) + 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 `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) + harness.givenRinging(alarmId = 1L, at = sevenToday) + val viewModel = viewModel(harness) + advanceUntilIdle() + + viewModel.onDelete(1L) + advanceUntilIdle() + + 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 --- @Test 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..20dc526 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/common/UndoSlotTest.kt @@ -0,0 +1,49 @@ +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) + } + + @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/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..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,8 @@ 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 {} } } private fun TimersUiState.loaded(): TimersUiState.Loaded = this as TimersUiState.Loaded @@ -158,6 +160,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 9ad3ba3..9e3b983 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1098,7 +1098,7 @@ wake lock. | The volume-button setting (#18) while the alarm rings only as a heads-up over another app, or with the screen turned off (the system keeps volume keys from a non-interactive device's windows). A background app sees no key events; the one route, a remote-playback `MediaSession` whose `VolumeProvider` takes the keys, is refused by `MediaSessionRecord.canHandleVolumeKey` on Android 13+ for an app with no MediaRouter2 routing session, loses to whichever app is in front or played last, and shows a cast-volume slider and a phantom media player while it works. AOSP DeskClock and BlackyHawky's Clock handle the keys in their alarm activity only, as `AlarmRingActivity` does | not planned | | 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 dismiss challenge or a snooze for a timer. (A timer that ramps its volume landed with #22, as an app-wide timer setting.) | 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 | @@ -1909,13 +1909,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)` and before `delete(id)`. It 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. @@ -1945,10 +1947,65 @@ 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. 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. 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 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 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. + +`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 +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 +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 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 +`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 +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. --- @@ -1984,6 +2041,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 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 +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 chip 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 @@ -2791,8 +2859,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 de854a8..017477e 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,8 +317,8 @@ 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 (since added as - an app-wide timer setting, #22), no presets + a confirmation, as with alarms; added since, #20), no ramp on a timer's ring + (since added as an app-wide timer setting, #22), 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