fix(alarms): drop an Undo offered while no list could show it
A delete can finish while no list is on screen, such as the app sent home the instant the editor's Delete was tapped. Its Undo is now dropped unshown once it is 30 seconds old, instead of surfacing hours later. Alarms and timers share one UndoSlot for this. A card whose swipe is re-enabled mid-settle still slides home.
This commit is contained in:
1 parent
62da327771
commit
697678b226
13 files changed
+134
-40
No files matched your search
@@ -1,9 +1,8 @@
|
||||
package de.jeanlucmakiola.clockula.ui.alarms
|
||||
|
||||
import de.jeanlucmakiola.clockula.domain.alarm.DeletedAlarm
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock
|
||||
import de.jeanlucmakiola.clockula.ui.common.UndoSlot
|
||||
import javax.inject.Inject
|
||||
import javax.inject.Singleton
|
||||
|
||||
@@ -16,27 +15,6 @@ import javax.inject.Singleton
|
||||
* answered; the list underneath shows the snackbar instead, and reads the
|
||||
* deletion from here. Held until the snackbar is answered, times out or is
|
||||
* left unanswered ([de.jeanlucmakiola.clockula.ui.common.UndoSnackbarEffect]).
|
||||
*
|
||||
* Memory only, deliberately: an Undo that survived process death would be
|
||||
* offered with no snackbar anyone remembers.
|
||||
*/
|
||||
@Singleton
|
||||
class AlarmUndo @Inject constructor() {
|
||||
|
||||
private val slot = MutableStateFlow<DeletedAlarm?>(null)
|
||||
|
||||
/** Null when there is nothing to undo. */
|
||||
val pending: StateFlow<DeletedAlarm?> = slot.asStateFlow()
|
||||
|
||||
/** A newer deletion replaces an older one: only the latest snackbar is on screen. */
|
||||
fun offer(deleted: DeletedAlarm) {
|
||||
slot.value = deleted
|
||||
}
|
||||
|
||||
/**
|
||||
* Claims [deleted] for its one answer. False when it has already been
|
||||
* answered or replaced, so a second tap cannot restore it twice and a
|
||||
* stale snackbar cannot drop a newer deletion.
|
||||
*/
|
||||
fun take(deleted: DeletedAlarm): Boolean = slot.compareAndSet(deleted, null)
|
||||
}
|
||||
class AlarmUndo @Inject constructor(clock: ElapsedRealtimeClock) : UndoSlot<DeletedAlarm>(clock)
|
||||
@@ -83,6 +83,7 @@ fun AlarmsScreen(
|
||||
message = undoMessage.orEmpty(),
|
||||
onUndo = viewModel::onUndoDelete,
|
||||
onExpired = viewModel::onUndoExpired,
|
||||
isStale = viewModel::isUndoStale,
|
||||
)
|
||||
|
||||
CollapsingScaffold(
|
||||
|
||||
@@ -109,6 +109,9 @@ class AlarmsViewModel @Inject internal constructor(
|
||||
undo.take(deleted)
|
||||
}
|
||||
|
||||
/** Offered too long ago to show a snackbar for now ([AlarmUndo.isStale]). */
|
||||
fun isUndoStale(deleted: DeletedAlarm): Boolean = undo.isStale(deleted)
|
||||
|
||||
/** Dismisses a ring first when switching off (M5 D11), then re-resolves. */
|
||||
fun onToggleEnabled(alarmId: Long, enabled: Boolean) {
|
||||
viewModelScope.launch {
|
||||
|
||||
@@ -80,10 +80,11 @@ internal fun SwipeToDelete(
|
||||
// Derived, so a drag recomposes this once as the red appears, not per frame.
|
||||
val revealed by remember { derivedStateOf { travel.value > 0f } }
|
||||
|
||||
// Disabled mid-drag or mid-settle: the gesture that would have brought the
|
||||
// card home is gone, so it comes home on its own.
|
||||
// Disabled (or re-enabled) mid-drag or mid-settle: the gesture that would
|
||||
// have brought the card home is gone, and so is any settle a quick flip
|
||||
// back just cancelled, so it comes home on its own.
|
||||
LaunchedEffect(enabled) {
|
||||
if (!enabled) travel.animateTo(0f, settleSpec)
|
||||
if (travel.value > 0f) travel.animateTo(0f, settleSpec)
|
||||
}
|
||||
|
||||
Box(
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
package de.jeanlucmakiola.clockula.ui.common
|
||||
|
||||
import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlin.time.Duration
|
||||
import kotlin.time.Duration.Companion.seconds
|
||||
|
||||
/**
|
||||
* The one deletion an Undo snackbar can still bring back: offered by a delete,
|
||||
* claimed once by the snackbar's answer.
|
||||
*
|
||||
* Deliberately memory only: an Undo that survived process death would be
|
||||
* offered with no snackbar anyone remembers.
|
||||
*/
|
||||
open class UndoSlot<T : Any>(private val clock: ElapsedRealtimeClock) {
|
||||
|
||||
private val slot = MutableStateFlow<T?>(null)
|
||||
|
||||
@Volatile
|
||||
private var offeredAt: Duration = Duration.ZERO
|
||||
|
||||
/** Null when there is nothing to undo. */
|
||||
val pending: StateFlow<T?> = slot.asStateFlow()
|
||||
|
||||
/** A newer deletion replaces an older one: only the latest snackbar is on screen. */
|
||||
fun offer(deleted: T) {
|
||||
offeredAt = clock.elapsedRealtime()
|
||||
slot.value = deleted
|
||||
}
|
||||
|
||||
/**
|
||||
* Claims [deleted] for its one answer. False when it has already been
|
||||
* answered or replaced, so a second tap cannot restore it twice and a
|
||||
* stale snackbar cannot drop a newer deletion.
|
||||
*/
|
||||
fun take(deleted: T): Boolean = slot.compareAndSet(deleted, null)
|
||||
|
||||
/**
|
||||
* Whether [deleted] was offered too long ago to put a snackbar up for now.
|
||||
* A delete can land while no list is on screen to show it (the app sent
|
||||
* to the background the moment the editor's Delete was tapped); offered
|
||||
* hours later, its Undo would restore a snapshot out of any context.
|
||||
*/
|
||||
fun isStale(deleted: T): Boolean =
|
||||
slot.value == deleted && clock.elapsedRealtime() - offeredAt > SHOW_WITHIN
|
||||
|
||||
private companion object {
|
||||
/** Comfortably longer than a rotation, far shorter than a forgotten app. */
|
||||
val SHOW_WITHIN: Duration = 30.seconds
|
||||
}
|
||||
}
|
||||
@@ -21,7 +21,9 @@ import kotlinx.coroutines.CancellationException
|
||||
* a pushed screen such as Settings, the activity finishing) counts as expired:
|
||||
* an Undo held back and offered again later would restore a snapshot from
|
||||
* minutes or hours ago, out of any context. A rotation is the one exception —
|
||||
* the activity comes straight back and shows the snackbar again.
|
||||
* the activity comes straight back and shows the snackbar again. A deletion
|
||||
* that was offered while no screen was there to show it, and is [isStale] by
|
||||
* the time one is, expires without a snackbar.
|
||||
*/
|
||||
@Composable
|
||||
internal fun <T : Any> UndoSnackbarEffect(
|
||||
@@ -30,6 +32,7 @@ internal fun <T : Any> UndoSnackbarEffect(
|
||||
message: String,
|
||||
onUndo: (T) -> Unit,
|
||||
onExpired: (T) -> Unit,
|
||||
isStale: (T) -> Boolean = { false },
|
||||
) {
|
||||
val activity = LocalActivity.current
|
||||
val undoLabel = stringResource(R.string.action_undo)
|
||||
@@ -37,6 +40,10 @@ internal fun <T : Any> UndoSnackbarEffect(
|
||||
val currentOnExpired by rememberUpdatedState(onExpired)
|
||||
LaunchedEffect(pending) {
|
||||
val deleted = pending ?: return@LaunchedEffect
|
||||
if (isStale(deleted)) {
|
||||
currentOnExpired(deleted)
|
||||
return@LaunchedEffect
|
||||
}
|
||||
val result = try {
|
||||
host.showSnackbar(message = message, actionLabel = undoLabel, duration = SnackbarDuration.Long)
|
||||
} catch (cancelled: CancellationException) {
|
||||
|
||||
@@ -64,6 +64,7 @@ fun TimersScreen(
|
||||
message = undoMessage,
|
||||
onUndo = viewModel::onUndoDelete,
|
||||
onExpired = viewModel::onUndoExpired,
|
||||
isStale = viewModel::isUndoStale,
|
||||
)
|
||||
|
||||
val loaded = state as? TimersUiState.Loaded
|
||||
|
||||
@@ -7,14 +7,14 @@ import de.jeanlucmakiola.clockula.data.prefs.SettingsPrefs
|
||||
import de.jeanlucmakiola.clockula.data.timers.TimerRepository
|
||||
import de.jeanlucmakiola.clockula.domain.Timer
|
||||
import de.jeanlucmakiola.clockula.domain.TimerDraft
|
||||
import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock
|
||||
import de.jeanlucmakiola.clockula.domain.timer.TimerPresets
|
||||
import de.jeanlucmakiola.clockula.timer.TimerEngine
|
||||
import de.jeanlucmakiola.clockula.ui.common.UndoSlot
|
||||
import de.jeanlucmakiola.floret.di.ApplicationScope
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.SharingStarted
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.flow.combine
|
||||
import kotlinx.coroutines.flow.stateIn
|
||||
import kotlinx.coroutines.launch
|
||||
@@ -33,6 +33,7 @@ class TimersViewModel @Inject internal constructor(
|
||||
private val engine: TimerEngine,
|
||||
private val settings: SettingsPrefs,
|
||||
source: TimerListSource,
|
||||
elapsed: ElapsedRealtimeClock,
|
||||
// `@param:` — the qualifier has no explicit @Target, and K2 resolves a bare
|
||||
// annotation on a constructor `val` to the property instead.
|
||||
@param:ApplicationScope private val appScope: CoroutineScope,
|
||||
@@ -86,21 +87,21 @@ class TimersViewModel @Inject internal constructor(
|
||||
|
||||
fun onSubtractMinute(timerId: Long) = launchOnEngine { engine.addTime(timerId, -1.minutes) }
|
||||
|
||||
private val undoSlot = MutableStateFlow<Timer?>(null)
|
||||
private val undo = UndoSlot<Timer>(elapsed)
|
||||
|
||||
/**
|
||||
* The deleted timer the snackbar offers to bring back. Null when none. Held
|
||||
* here, not in the screen, so a rotation re-shows the snackbar rather than
|
||||
* losing the Undo; a newer delete replaces it.
|
||||
*/
|
||||
val pendingUndo: StateFlow<Timer?> = undoSlot.asStateFlow()
|
||||
val pendingUndo: StateFlow<Timer?> = undo.pending
|
||||
|
||||
/**
|
||||
* Through the engine, so a *ringing* timer cannot be deleted into a stuck
|
||||
* ring (D2). Still no confirmation; the snackbar's Undo now covers the slip.
|
||||
*/
|
||||
fun onDelete(timerId: Long) = launchOnEngine {
|
||||
engine.delete(timerId)?.let { undoSlot.value = it }
|
||||
engine.delete(timerId)?.let(undo::offer)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -109,15 +110,18 @@ class TimersViewModel @Inject internal constructor(
|
||||
* chance, so a back press that clears this ViewModel must not drop it.
|
||||
*/
|
||||
fun onUndoDelete(timer: Timer) {
|
||||
if (!undoSlot.compareAndSet(timer, null)) return
|
||||
if (!undo.take(timer)) return
|
||||
appScope.launch { engine.restore(timer) }
|
||||
}
|
||||
|
||||
/** The snackbar timed out or was swiped away: the delete stands. */
|
||||
fun onUndoExpired(timer: Timer) {
|
||||
undoSlot.compareAndSet(timer, null)
|
||||
undo.take(timer)
|
||||
}
|
||||
|
||||
/** Offered too long ago to show a snackbar for now ([UndoSlot.isStale]). */
|
||||
fun isUndoStale(timer: Timer): Boolean = undo.isStale(timer)
|
||||
|
||||
fun onSavePreset(duration: Duration) {
|
||||
viewModelScope.launch { settings.addTimerPreset(duration) }
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ import de.jeanlucmakiola.clockula.domain.Ringtones
|
||||
import de.jeanlucmakiola.clockula.domain.TimeOfDay
|
||||
import de.jeanlucmakiola.clockula.testing.AlarmEngineHarness
|
||||
import de.jeanlucmakiola.clockula.testing.BERLIN
|
||||
import de.jeanlucmakiola.clockula.testing.FakeElapsedRealtimeClock
|
||||
import de.jeanlucmakiola.clockula.testing.FakeRingtoneCatalog
|
||||
import de.jeanlucmakiola.clockula.testing.FakeRingtonePreviewer
|
||||
import de.jeanlucmakiola.clockula.testing.MainDispatcherExtension
|
||||
@@ -51,7 +52,7 @@ class AlarmEditorViewModelTest {
|
||||
private val sevenToday = offsetInstant("2026-06-10T07:00+02:00")
|
||||
private val sevenTomorrow = offsetInstant("2026-06-11T07:00+02:00")
|
||||
|
||||
private fun TestScope.editor(harness: AlarmEngineHarness, alarmId: Long = 1L, undo: AlarmUndo = AlarmUndo()) =
|
||||
private fun TestScope.editor(harness: AlarmEngineHarness, alarmId: Long = 1L, undo: AlarmUndo = AlarmUndo(FakeElapsedRealtimeClock())) =
|
||||
AlarmEditorViewModel(
|
||||
savedStateHandle = SavedStateHandle(mapOf(AlarmRoutes.ARG_ALARM_ID to alarmId)),
|
||||
alarms = harness.alarms,
|
||||
@@ -662,7 +663,7 @@ class AlarmEditorViewModelTest {
|
||||
val harness = alarmEngineHarness(tempDir, now = fiveToday)
|
||||
harness.givenAlarmWith(hour = 7)
|
||||
harness.engine.reschedule()
|
||||
val undo = AlarmUndo()
|
||||
val undo = AlarmUndo(FakeElapsedRealtimeClock())
|
||||
val viewModel = editor(harness, undo = undo)
|
||||
advanceUntilIdle()
|
||||
val before = harness.alarmDao.stored
|
||||
|
||||
@@ -7,6 +7,7 @@ import de.jeanlucmakiola.clockula.domain.TimeOfDay
|
||||
import de.jeanlucmakiola.clockula.domain.format.NextFireLabel
|
||||
import de.jeanlucmakiola.clockula.testing.AlarmEngineHarness
|
||||
import de.jeanlucmakiola.clockula.testing.BERLIN
|
||||
import de.jeanlucmakiola.clockula.testing.FakeElapsedRealtimeClock
|
||||
import de.jeanlucmakiola.clockula.testing.MainDispatcherExtension
|
||||
import de.jeanlucmakiola.clockula.testing.RingEvent
|
||||
import de.jeanlucmakiola.clockula.testing.alarmEngineHarness
|
||||
@@ -46,7 +47,7 @@ class AlarmsViewModelTest {
|
||||
* `collectAsStateWithLifecycle`; without one there is no subscriber and
|
||||
* nothing upstream would run.
|
||||
*/
|
||||
private fun TestScope.viewModel(harness: AlarmEngineHarness, undo: AlarmUndo = AlarmUndo()) = AlarmsViewModel(
|
||||
private fun TestScope.viewModel(harness: AlarmEngineHarness, undo: AlarmUndo = AlarmUndo(FakeElapsedRealtimeClock())) = AlarmsViewModel(
|
||||
alarms = harness.alarms,
|
||||
engine = harness.engine,
|
||||
wall = harness.wallClock,
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
package de.jeanlucmakiola.clockula.ui.common
|
||||
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import de.jeanlucmakiola.clockula.testing.FakeElapsedRealtimeClock
|
||||
import org.junit.jupiter.api.Test
|
||||
import kotlin.time.Duration.Companion.minutes
|
||||
import kotlin.time.Duration.Companion.seconds
|
||||
|
||||
/** Claim once, newer replaces older, and an offer nobody saw in time goes stale. */
|
||||
class UndoSlotTest {
|
||||
|
||||
private val clock = FakeElapsedRealtimeClock()
|
||||
private val slot = UndoSlot<String>(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)
|
||||
}
|
||||
}
|
||||
@@ -50,6 +50,7 @@ class TimersViewModelTest {
|
||||
engine = harness.engine,
|
||||
settings = harness.settings,
|
||||
source = TimerListSource(harness.timers, harness.elapsed, harness.wallClock, harness.ticker),
|
||||
elapsed = harness.elapsed,
|
||||
appScope = this,
|
||||
).also { viewModel -> backgroundScope.launch { viewModel.state.collect {} } }
|
||||
|
||||
|
||||
@@ -1756,7 +1756,10 @@ replaces it, and when the screen is left with the snackbar unanswered (another
|
||||
tab, Settings, the app closed): an Undo held back and offered later would
|
||||
restore a snapshot from long ago, out of any context, and over a backup that
|
||||
was restored meanwhile. A rotation is the one exception and shows the snackbar
|
||||
again. It is memory only, so process death drops it too.
|
||||
again. A deletion offered while no list was on screen (the app sent home the
|
||||
instant the editor's Delete was tapped) is dropped unshown if it is more than
|
||||
30 seconds old by the time the list is back. Both lists share this as
|
||||
`UndoSlot`. It is memory only, so process death drops it too.
|
||||
|
||||
The swipe is hand-rolled (`ui/common/SwipeToDelete`) rather than M3's
|
||||
`SwipeToDismissBox`, because the tabs are pages of a `HorizontalPager` and the
|
||||
|
||||
Reference in new issue
Block a user