fix(alarms): keep the editor's Delete off a ringing alarm too
With its confirmation gone, one tap on the editor's Delete would have silenced a ringing alarm round its dismiss challenge, which the list's swipe is already kept from doing; the ring screen is the way out. The list's delete now runs on the application scope like the editor's, a thrown-away card ignores a new drag, and the timers snackbar clears the live pill on an empty tab.
This commit is contained in:
@@ -208,8 +208,8 @@ class AlarmEditorViewModel @Inject internal constructor(
|
||||
}
|
||||
|
||||
/**
|
||||
* Deletes through the engine, which silences a ring first (M5 D11), and the
|
||||
* state becomes [AlarmEditorUiState.Gone]. No confirmation: the editor pops
|
||||
* Deletes through the engine, and the state becomes
|
||||
* [AlarmEditorUiState.Gone]. A ringing alarm is left alone. No confirmation: the editor pops
|
||||
* and the list underneath offers the Undo, through [AlarmUndo] because this
|
||||
* ViewModel is about to be cleared.
|
||||
*/
|
||||
@@ -221,8 +221,13 @@ class AlarmEditorViewModel @Inject internal constructor(
|
||||
// The label still in flight goes in first, so the Undo brings back
|
||||
// what the user typed rather than what had been written so far.
|
||||
labelWrites.value?.let { text -> alarms.edit(alarmId) { it.copy(label = text) } }
|
||||
engine.delete(alarmId)?.let(undo::offer)
|
||||
_state.value = AlarmEditorUiState.Gone
|
||||
// A ringing alarm is not deleted from here either: with no
|
||||
// confirmation in the way, one tap would silence it round its
|
||||
// dismiss challenge. Its ring screen is the way out.
|
||||
engine.delete(alarmId, unlessRinging = true)?.let { deleted ->
|
||||
undo.offer(deleted)
|
||||
_state.value = AlarmEditorUiState.Gone
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -93,7 +93,9 @@ class AlarmsViewModel @Inject internal constructor(
|
||||
fun onDelete(alarmId: Long) {
|
||||
// Never a ringing alarm: the card leads to its ring screen then, and the
|
||||
// engine re-checks under its lock in case the ring began mid-swipe.
|
||||
viewModelScope.launch { engine.delete(alarmId, unlessRinging = true)?.let(undo::offer) }
|
||||
// On the application scope: the card has already been thrown away, so
|
||||
// a back press while the engine lock is busy must not drop the delete.
|
||||
appScope.launch { engine.delete(alarmId, unlessRinging = true)?.let(undo::offer) }
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -94,6 +94,8 @@ internal fun SwipeToDelete(
|
||||
if (!enabled) return@pointerInput
|
||||
awaitEachGesture {
|
||||
val down = awaitFirstDown(requireUnconsumed = false)
|
||||
// Thrown away and waiting to leave the list: not a card to swipe.
|
||||
if (travel.value >= width) return@awaitEachGesture
|
||||
val velocity = VelocityTracker()
|
||||
velocity.addPointerInputChange(down)
|
||||
var slopOvershoot = 0f
|
||||
|
||||
@@ -75,7 +75,14 @@ fun TimersScreen(
|
||||
title = stringResource(R.string.tab_timers),
|
||||
modifier = modifier,
|
||||
scrollable = false,
|
||||
snackbarHost = { SnackbarHost(snackbar) },
|
||||
// Above the pill: over a list the FAB (padded by the same inset) lifts
|
||||
// the snackbar, but an empty tab has no FAB to stand on.
|
||||
snackbarHost = {
|
||||
SnackbarHost(
|
||||
snackbar,
|
||||
modifier = Modifier.padding(bottom = if (rows.isNullOrEmpty()) pillInset else 0.dp),
|
||||
)
|
||||
},
|
||||
actions = {
|
||||
IconButton(onClick = onOpenSettings) {
|
||||
Icon(
|
||||
|
||||
+13
-12
@@ -15,7 +15,6 @@ import de.jeanlucmakiola.clockula.testing.FakeRingtonePreviewer
|
||||
import de.jeanlucmakiola.clockula.testing.MainDispatcherExtension
|
||||
import de.jeanlucmakiola.clockula.testing.PreviewEvent
|
||||
import de.jeanlucmakiola.clockula.ui.common.RingtoneChoice
|
||||
import de.jeanlucmakiola.clockula.testing.RingEvent
|
||||
import de.jeanlucmakiola.clockula.testing.alarmAt
|
||||
import de.jeanlucmakiola.clockula.testing.alarmEngineHarness
|
||||
import de.jeanlucmakiola.clockula.testing.instantAt
|
||||
@@ -643,19 +642,21 @@ class AlarmEditorViewModelTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `deleting the ringing alarm silences it first`(@TempDir tempDir: Path) = runTest(main.dispatcher) {
|
||||
val harness = alarmEngineHarness(tempDir, now = fiveToday)
|
||||
harness.givenAlarmWith(hour = 7)
|
||||
harness.givenRinging(alarmId = 1L, at = sevenToday)
|
||||
val viewModel = editor(harness)
|
||||
advanceUntilIdle()
|
||||
fun `the ringing alarm is not deleted round its ring screen`(@TempDir tempDir: Path) =
|
||||
runTest(main.dispatcher) {
|
||||
val harness = alarmEngineHarness(tempDir, now = fiveToday)
|
||||
harness.givenAlarmWith(hour = 7)
|
||||
harness.givenRinging(alarmId = 1L, at = sevenToday)
|
||||
val viewModel = editor(harness)
|
||||
advanceUntilIdle()
|
||||
|
||||
viewModel.onDelete()
|
||||
advanceUntilIdle()
|
||||
viewModel.onDelete()
|
||||
advanceUntilIdle()
|
||||
|
||||
assertThat(harness.ring.events.last() to harness.alarmDao.stored)
|
||||
.isEqualTo(RingEvent.Stopped(1L) to emptyList<Any>())
|
||||
}
|
||||
assertThat(listOf(harness.ring.ringing.value, harness.alarmDao.stored.map { it.id }))
|
||||
.isEqualTo(listOf(1L, listOf(1L)))
|
||||
assertThat(viewModel.state.value).isInstanceOf(AlarmEditorUiState.Editing::class.java)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `deleting hands the alarm to the list's undo, and the undo puts it back`(@TempDir tempDir: Path) =
|
||||
|
||||
@@ -1682,14 +1682,15 @@ auto-silence backstop are single *global* slots (§11), touched only by
|
||||
`closeCycleLocked`, and `onAutoSilence` returns early once `ringingSince` is
|
||||
null. So switching off or deleting a ringing alarm from this screen would clear
|
||||
its state and leave it **sounding until the 15-minute wake-lock timeout, with
|
||||
the backstop disarmed by its own guard**. Both ViewModels therefore call
|
||||
the backstop disarmed by its own guard**. The list therefore calls
|
||||
|
||||
```kotlin
|
||||
if (engine.ringSession()?.alarm?.id == alarmId) engine.dismiss(alarmId)
|
||||
```
|
||||
|
||||
before `setEnabled(id, false)`. Deleting goes further and is an engine verb of
|
||||
its own (`AlarmEngine.delete`, below), which stops the ring itself. The check reads storage under
|
||||
before `setEnabled(id, false)`. Deleting is an engine verb of its own
|
||||
(`AlarmEngine.delete`, below), which stops a ring itself but, from the list and
|
||||
the editor, refuses a ringing alarm outright. The check reads storage under
|
||||
the engine's own mutex, so it cannot race the fire it is asking about, and it is
|
||||
deliberately **conditional**: dismissing unconditionally would also drop a
|
||||
merely *snoozed* alarm's pending snooze, which is wrong for a label edit.
|
||||
@@ -1722,7 +1723,9 @@ asks first. A *ringing* card offers neither the swipe nor the action: it leads
|
||||
to the ring screen, and a swipe must not become a way round its dismiss
|
||||
challenge. The engine re-checks under its lock (`delete(id, unlessRinging = true)`),
|
||||
so a swipe released just as the alarm fires, before the list has caught up,
|
||||
deletes nothing and the card slides back. Each is followed by a snackbar on the list ("07:00 alarm deleted",
|
||||
deletes nothing and the card slides back. The editor's Delete passes the same
|
||||
flag: with its confirmation gone, one tap there would otherwise do what the
|
||||
swipe is kept from doing, so a ringing alarm is dealt with on its ring screen. Each is followed by a snackbar on the list ("07:00 alarm deleted",
|
||||
**Undo**), as in Google Clock. v1 asked before instead, on the argument that a
|
||||
mistaken delete is found at 07:00 rather than now. But a dialog catches no more
|
||||
than the snackbar does: both only catch the slip noticed in the moment, and a
|
||||
|
||||
Reference in New Issue
Block a user