From 5f8eb39206c31036b8cd4c9eedeb64db6a34dd8f Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 13 Nov 2026 11:27:46 +0100 Subject: [PATCH 1/5] Queue a drop made while another move is still saving (#494) --- .../ui/common/RescheduleViewModel.kt | 46 ++++++++++++++++--- .../ui/common/RescheduleViewModelTest.kt | 44 ++++++++++++++++++ 2 files changed, 83 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt index dad5cb1f..4650f104 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt @@ -189,6 +189,15 @@ class RescheduleViewModel @Inject constructor( _inFlight.value = value } + /** The event whose drop owns the pipeline, while [busy]. */ + private var busyEventId: Long? = null + + /** + * A drop of another event that arrived while one was still being written, + * run once that write settles rather than snapping back unannounced (#494). + */ + private var queued: MoveRequest? = null + /** * The calendars whose events may be dragged. Nothing below the UI guards * this, so a block outside this set registers no drag gesture at all. @@ -219,7 +228,7 @@ class RescheduleViewModel @Inject constructor( } /** - * Take a drop, unless one is already being written. False means nothing will + * Take a drop, or queue it behind the one being written (see [canQueue]). False means nothing will * be written; [abandoned] ticks for that refusal too, so a held copy is let * go by the one signal rather than by every caller remembering to. * @@ -228,27 +237,32 @@ class RescheduleViewModel @Inject constructor( * from two places, and it answers before the write is resolved anyway. */ fun move(request: MoveRequest): Boolean { + if (busy && canQueue(request)) { + queued = request + return true + } if (busy || _scopePrompt.value != null) { _abandoned.value += 1 return false } busy = true + busyEventId = request.eventId viewModelScope.launch { val prepared = prepare(request) if (prepared == null) { _abandoned.value += 1 - busy = false + settle() return@launch } // A cancelled series only moves as a whole (#388). val seriesOnly = !prepared.original.canSaveApartFromSeries() if (!prepared.isRecurring || (seriesOnly && prepared.canRealign)) { write(prepared, RecurringWriteScope.AllEvents) - busy = false + settle() } else if (seriesOnly) { _outcome.value = MoveOutcome.BlockedCancelledSeries _abandoned.value += 1 - busy = false + settle() } else { // Still busy: the scope dialog is now the thing in flight. pending = prepared @@ -270,16 +284,33 @@ class RescheduleViewModel @Inject constructor( _scopePrompt.value = null viewModelScope.launch { write(prepared, scope) - busy = false + settle() } } /** Dismiss the scope dialog without writing. */ fun cancelScope() { pending = null - busy = false _scopePrompt.value = null _abandoned.value += 1 + settle() + } + + /** + * Only a drop of a different event, and only before a scope dialog is up: + * a second drop of the same event carries the occurrence times from before + * the first write, and would compound with it. + */ + private fun canQueue(request: MoveRequest): Boolean = + queued == null && _scopePrompt.value == null && request.eventId != busyEventId + + /** Free the pipeline, then run the drop that waited for it, if any. */ + private fun settle() { + busy = false + busyEventId = null + val next = queued ?: return + queued = null + move(next) } /** @@ -297,6 +328,7 @@ class RescheduleViewModel @Inject constructor( fun undo(undo: MoveUndo): Boolean { if (busy) return false busy = true + busyEventId = undo.eventId _undoStarted.value += 1 viewModelScope.launch { _outcome.value = try { @@ -314,7 +346,7 @@ class RescheduleViewModel @Inject constructor( // Read off the outcome, the way [write] does — no second copy of the // same fact to drift from it. if (_outcome.value !is MoveOutcome.Undone) _abandoned.value += 1 - busy = false + settle() } return true } diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt index e44e6f33..1576608b 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt @@ -17,6 +17,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job import kotlinx.coroutines.launch +import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import kotlinx.datetime.LocalDate @@ -88,6 +89,17 @@ class RescheduleViewModelTest { return RescheduleViewModel(repo, dispatcher, FakeZoneSource()) } + /** A view model whose provider reads suspend until the scheduler runs, so a drop stays in flight. */ + private fun slowViewModel(tempDir: Path, fake: FakeCalendarDataSource): RescheduleViewModel { + val io = StandardTestDispatcher(dispatcher.scheduler) + val repo = testRepository( + fake, tempDir, io, + prefs = testPrefs(tempDir, CoroutineScope(dispatcher)), + settings = testSettings(tempDir, CoroutineScope(dispatcher)), + ) + return RescheduleViewModel(repo, io, FakeZoneSource()) + } + private fun CoroutineScope.activate(vm: RescheduleViewModel): Job = launch { vm.movableCalendarIds.collect {} } @@ -276,6 +288,38 @@ class RescheduleViewModelTest { assertThat(fake.updatedEvents).isEmpty() } + @Test + fun `a drop of another event waits for the one in flight, then lands`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail() } } + val vm = slowViewModel(tempDir, fake) + + assertThat(vm.move(oneHourLater())).isTrue() + assertThat(vm.inFlight.value).isTrue() + val abandoned = vm.abandoned.value + assertThat(vm.move(toWednesday().copy(eventId = 7L))).isTrue() + advanceUntilIdle() + + assertThat(fake.updatedEvents.map { it.first }).containsExactly(42L, 7L).inOrder() + assertThat(vm.abandoned.value).isEqualTo(abandoned) + assertThat(vm.inFlight.value).isFalse() + } + + @Test + fun `a second drop of the event in flight is still refused`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail() } } + val vm = slowViewModel(tempDir, fake) + + assertThat(vm.move(oneHourLater())).isTrue() + assertThat(vm.move(toWednesday())).isFalse() + advanceUntilIdle() + + assertThat(fake.updatedEvents).hasSize(1) + } + @Test fun `undo is refused while a write is running`(@TempDir tempDir: Path) = runTest(dispatcher) { val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail() } } From 8e3202d1ad547c8709277ac137ee8d6dca1cc47f Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 13 Nov 2026 12:00:37 +0100 Subject: [PATCH 2/5] Give every drop its own token and queue drops in order (#494) --- .../calendula/ui/CalendarHost.kt | 3 +- .../calendula/ui/common/EventMoveHost.kt | 1 + .../calendula/ui/common/EventMoveScope.kt | 89 +++++---- .../ui/common/RescheduleViewModel.kt | 181 +++++++++--------- .../calendula/ui/common/TimelineDrag.kt | 38 ++-- .../calendula/ui/month/MonthDrag.kt | 27 +-- .../calendula/ui/month/MonthScreen.kt | 16 +- .../calendula/ui/week/WeekScreen.kt | 7 + app/src/main/res/values/strings.xml | 1 + .../ui/common/RescheduleViewModelTest.kt | 123 ++++++++---- 10 files changed, 269 insertions(+), 217 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHost.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHost.kt index 07349082..d3b8bfb1 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHost.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHost.kt @@ -485,9 +485,8 @@ fun CalendarHost( movableCalendarIds = movableCalendarIds, dragEnabled = dragToReschedule, move = { reschedule.move(it) }, - inFlight = reschedule.inFlight, + drops = reschedule.drops, undoStarted = reschedule.undoStarted, - abandoned = reschedule.abandoned, edit = onEditEvent, ) } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveHost.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveHost.kt index f827dc99..73cfe836 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveHost.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveHost.kt @@ -90,6 +90,7 @@ fun EventMoveHost( MoveOutcome.Undone -> stringResource(R.string.event_move_undone) MoveOutcome.WriteDenied -> stringResource(R.string.event_move_write_denied) MoveOutcome.Gone -> stringResource(R.string.event_move_gone) + MoveOutcome.Stale -> stringResource(R.string.event_move_stale) MoveOutcome.BlockedSeriesEnd -> stringResource(R.string.event_move_blocked_series_end) MoveOutcome.BlockedCancelledSeries -> stringResource(R.string.event_move_blocked_cancelled_series) MoveOutcome.Failed -> stringResource(R.string.event_move_failed) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveScope.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveScope.kt index 92b8eae1..6eb9050f 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveScope.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveScope.kt @@ -6,7 +6,7 @@ import androidx.compose.runtime.Immutable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.compositionLocalOf import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableIntStateOf +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.res.stringResource @@ -16,6 +16,7 @@ import de.jeanlucmakiola.calendula.R import de.jeanlucmakiola.calendula.domain.EventInstance import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow +import java.util.concurrent.atomic.AtomicLong /** * Drag-to-reschedule wiring (#68), provided once at `CalendarHost` and read by @@ -37,24 +38,18 @@ class EventMoveScope( val dragEnabled: Boolean, /** * Hand a drop to the writer. Returns nothing on purpose: a drop is answered - * before its write resolves, so [abandoned] — not a return value — is what - * tells a held copy that nothing landed. + * before its write resolves, so [drops] — not a return value — is what tells + * a held copy whether anything landed. */ val move: (MoveRequest) -> Unit, /** - * True while a dropped event is being written, including the time its scope - * dialog is up. A flow rather than a value so this scope stays the same - * object across a move — every visible block reads it as a composition local. + * Each drop's progress by its token — see `RescheduleViewModel.drops`. A flow + * rather than a value so this scope stays the same object across a move — + * every visible block reads it as a composition local. */ - val inFlight: StateFlow, - /** Ticks when an undo write begins — see `RescheduleViewModel.undoStarted`. */ - val undoStarted: StateFlow, - /** - * Ticks when a drop ends with nothing landing — see - * `RescheduleViewModel.abandoned`. [move] answers before the write is - * resolved, so this is what tells a held copy to stop waiting. - */ - val abandoned: StateFlow, + val drops: StateFlow, + /** The token of the undo write that just began — see `RescheduleViewModel.undoStarted`. */ + val undoStarted: StateFlow, /** Open an event in the edit form — the pointer-free route to the same change. */ val edit: (EventInstance) -> Unit, ) { @@ -63,46 +58,56 @@ class EventMoveScope( val LocalEventMove = compositionLocalOf { null } -private val NEVER_IN_FLIGHT = MutableStateFlow(false) +private val moveTokens = AtomicLong() -private val NEVER_UNDONE = MutableStateFlow(0) +/** A fresh identity for one drop or undo, unique for the process. */ +fun nextMoveToken(): Long = moveTokens.incrementAndGet() -private val NEVER_ABANDONED = MutableStateFlow(0) +/** How many ended drops [DropStates] remembers — far more than can be on screen at once. */ +private const val ENDED_KEPT = 32 + +/** Per-drop progress, by token — see `RescheduleViewModel.drops`. */ +@Immutable +data class DropStates( + val pending: Set = emptySet(), + val abandoned: Set = emptySet(), +) { + fun started(token: Long): DropStates = copy(pending = pending + token) + + fun ended(token: Long, landed: Boolean): DropStates = copy( + pending = pending - token, + abandoned = if (landed) abandoned else (abandoned + token).toList().takeLast(ENDED_KEPT).toSet(), + ) +} + +private val NO_DROPS = MutableStateFlow(DropStates()) + +private val NEVER_UNDONE = MutableStateFlow(null) /** - * Whether a dropped event is still being written — false wherever moving is off. - * The drag overlays hold a landed block on its target for this window. + * Every drop's progress — empty wherever moving is off. A drag overlay keeps + * the token of its own drop and holds its copy while that token is pending. */ @Composable -fun moveInFlight(): Boolean { - val flow = LocalEventMove.current?.inFlight ?: NEVER_IN_FLIGHT +fun dropStates(): DropStates { + val flow = LocalEventMove.current?.drops ?: NO_DROPS return flow.collectAsStateWithLifecycle().value } /** - * How many drops have ended with nothing landing. Read as a plain count so a - * drag overlay can capture it at the moment of the drop and tell, later, whether - * *its own* drop was one of them. + * Runs [onUndo] with the undo's token when an undo write begins, and never for + * one that began before this composable came on screen. */ @Composable -fun abandonedMoves(): Int { - val flow = LocalEventMove.current?.abandoned ?: NEVER_ABANDONED - return flow.collectAsStateWithLifecycle().value -} - -/** - * Runs [onUndo] when an undo write begins, and never for one that began before - * this composable came on screen. - */ -@Composable -fun OnUndoStarted(onUndo: () -> Unit) { +fun OnUndoStarted(onUndo: (token: Long) -> Unit) { val flow = LocalEventMove.current?.undoStarted ?: NEVER_UNDONE - val tick by flow.collectAsStateWithLifecycle() - var seen by remember { mutableIntStateOf(tick) } - LaunchedEffect(tick) { - if (tick == seen) return@LaunchedEffect - seen = tick - onUndo() + val token by flow.collectAsStateWithLifecycle() + var seen by remember { mutableStateOf(token) } + LaunchedEffect(token) { + val started = token + if (started == null || started == seen) return@LaunchedEffect + seen = started + onUndo(started) } } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt index 4650f104..d40b0622 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt @@ -32,6 +32,7 @@ import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import kotlinx.datetime.DateTimeUnit import kotlinx.datetime.LocalDate @@ -46,6 +47,9 @@ import javax.inject.Inject private const val MILLIS_PER_MINUTE = 60_000 private const val MINUTES_PER_DAY = 24 * 60 +/** How many drops may wait behind the one in flight before more are refused. */ +private const val MAX_QUEUED = 8 + /** Where a dragged event should land. */ sealed interface MoveTarget { /** A new start instant — a timeline drag, which moves time and day at once. */ @@ -74,6 +78,8 @@ data class MoveRequest( val beginMillis: Long, val endMillis: Long, val target: MoveTarget, + /** The drop's own identity, from [nextMoveToken], which the view holding its copy waits on. */ + val token: Long = nextMoveToken(), ) /** @@ -115,6 +121,12 @@ sealed interface MoveOutcome { /** The event vanished (sync, another device) between the read and the write. */ data object Gone : MoveOutcome + /** + * A drop that waited for an earlier one found its occurrence no longer where + * it was picked up — the earlier write moved it — so it is not written (#494). + */ + data object Stale : MoveOutcome + /** The drop would push the series past its own `UNTIL`, generating nothing. */ data object BlockedSeriesEnd : MoveOutcome @@ -152,51 +164,37 @@ class RescheduleViewModel @Inject constructor( private val _inFlight = MutableStateFlow(false) - /** - * True from the moment a drop is accepted until its write settles — the - * window the dropped block holds its landing position for. - */ + /** True while any write runs or waits, a queued drop included — what the confirmation chip holds for. */ val inFlight: StateFlow = _inFlight.asStateFlow() - private val _abandoned = MutableStateFlow(0) + private val _drops = MutableStateFlow(DropStates()) /** - * Ticks when a drop ends with nothing on the target day: refused outright, - * refused before the write, written and failed, or its scope dialog - * dismissed. [move] answers before any of that is known, so without this - * signal the view that drew the drop held its copy out for the full settle - * timeout waiting for a grid that was never going to draw it (#253). + * Each drop's own state, by its [MoveRequest.token]: pending from the moment + * it is accepted until its write settles, and abandoned once it ends with + * nothing on the target day — refused, failed, or its scope dialog dismissed. + * Per drop, so a queued drop's held copy waits on its own write and is never + * let go by another drop's failure (#253, #494). */ - val abandoned: StateFlow = _abandoned.asStateFlow() + val drops: StateFlow = _drops.asStateFlow() - private val _undoStarted = MutableStateFlow(0) + private val _undoStarted = MutableStateFlow(null) /** - * Ticks the moment an undo write begins, before the provider has anything to - * re-read, so the view that drew the drop can carry its chip back rather than - * let it reappear on the old day. + * The token of an undo write the moment it begins, before the provider has + * anything to re-read, so the view that drew the drop can carry its chip back + * rather than let it reappear on the old day. Its fate then reads off [drops]. */ - val undoStarted: StateFlow = _undoStarted.asStateFlow() + val undoStarted: StateFlow = _undoStarted.asStateFlow() /** - * Set from the moment a drop is accepted until its write settles. Two drops - * of the same recurring event inside that window would compute their shift - * from the same pre-move occurrence, and the shifts would compound. + * Set while a drop is being prepared, waits on its scope dialog, or is + * written. Drops arriving in that window wait in [queue] and run in order. */ private var busy = false - set(value) { - field = value - _inFlight.value = value - } - /** The event whose drop owns the pipeline, while [busy]. */ - private var busyEventId: Long? = null - - /** - * A drop of another event that arrived while one was still being written, - * run once that write settles rather than snapping back unannounced (#494). - */ - private var queued: MoveRequest? = null + /** Drops that arrived while another was in flight, oldest first (#494). */ + private val queue = ArrayDeque() /** * The calendars whose events may be dragged. Nothing below the UI guards @@ -228,41 +226,44 @@ class RescheduleViewModel @Inject constructor( } /** - * Take a drop, or queue it behind the one being written (see [canQueue]). False means nothing will - * be written; [abandoned] ticks for that refusal too, so a held copy is let - * go by the one signal rather than by every caller remembering to. + * Take a drop: run it now, or queue it behind the ones in flight. False means + * nothing will be written — the queue is full — and the drop is marked + * abandoned in [drops], so its held copy is let go by the one signal rather + * than by every caller remembering to. * * The Boolean is diagnostic — the tests read it, [EventMoveScope] does not - * hand it on. A caller that gated on it would be back to releasing its copy - * from two places, and it answers before the write is resolved anyway. + * hand it on. It answers before the write is resolved anyway. */ fun move(request: MoveRequest): Boolean { - if (busy && canQueue(request)) { - queued = request - return true - } - if (busy || _scopePrompt.value != null) { - _abandoned.value += 1 + if (queue.size >= MAX_QUEUED) { + _drops.update { it.ended(request.token, landed = false) } return false } + _drops.update { it.started(request.token) } + if (busy) { + queue.addLast(request) + } else { + run(request, waited = false) + } + syncInFlight() + return true + } + + private fun run(request: MoveRequest, waited: Boolean) { busy = true - busyEventId = request.eventId viewModelScope.launch { - val prepared = prepare(request) + val prepared = prepare(request, waited) if (prepared == null) { - _abandoned.value += 1 - settle() + settle(request.token, landed = false) return@launch } // A cancelled series only moves as a whole (#388). val seriesOnly = !prepared.original.canSaveApartFromSeries() if (!prepared.isRecurring || (seriesOnly && prepared.canRealign)) { - write(prepared, RecurringWriteScope.AllEvents) - settle() + settle(request.token, write(prepared, RecurringWriteScope.AllEvents)) } else if (seriesOnly) { _outcome.value = MoveOutcome.BlockedCancelledSeries - _abandoned.value += 1 - settle() + settle(request.token, landed = false) } else { // Still busy: the scope dialog is now the thing in flight. pending = prepared @@ -272,7 +273,6 @@ class RescheduleViewModel @Inject constructor( ) } } - return true } /** Answer the scope dialog. */ @@ -283,53 +283,45 @@ class RescheduleViewModel @Inject constructor( pending = null _scopePrompt.value = null viewModelScope.launch { - write(prepared, scope) - settle() + settle(prepared.request.token, write(prepared, scope)) } } /** Dismiss the scope dialog without writing. */ fun cancelScope() { + val prepared = pending ?: return pending = null _scopePrompt.value = null - _abandoned.value += 1 - settle() + settle(prepared.request.token, landed = false) } - /** - * Only a drop of a different event, and only before a scope dialog is up: - * a second drop of the same event carries the occurrence times from before - * the first write, and would compound with it. - */ - private fun canQueue(request: MoveRequest): Boolean = - queued == null && _scopePrompt.value == null && request.eventId != busyEventId - - /** Free the pipeline, then run the drop that waited for it, if any. */ - private fun settle() { + /** End the drop [token], then run the next queued one, if any. */ + private fun settle(token: Long, landed: Boolean) { + _drops.update { it.ended(token, landed) } busy = false - busyEventId = null - val next = queued ?: return - queued = null - move(next) + queue.removeFirstOrNull()?.let { run(it, waited = true) } + syncInFlight() + } + + private fun syncInFlight() { + _inFlight.value = busy || queue.isNotEmpty() } /** * Put a completed move back where it came from. The outcome stands until the * inverse write reports back, so the one chip changes what it says rather * than closing and reopening. False when another write is already running — - * the chip hides its action for that window, so this is the backstop. - * - * [abandoned] ticks once an inverse that *started* fails to land: [undoStarted] - * has sent the chip back on its journey by then and nothing else will seat it. - * A refusal ticks nothing — it launched no journey of its own, and the counter - * is global, so a tick here would release some other drop's held copy before - * the grid has drawn it. + * the chip hides its action for that window, so this is the backstop. An + * undo is a journey of its own: [undoStarted] names its token, and [drops] + * reports whether it landed. */ fun undo(undo: MoveUndo): Boolean { - if (busy) return false + if (busy || queue.isNotEmpty()) return false + val token = nextMoveToken() + _drops.update { it.started(token) } busy = true - busyEventId = undo.eventId - _undoStarted.value += 1 + syncInFlight() + _undoStarted.value = token viewModelScope.launch { _outcome.value = try { repository.updateEvent(undo.eventId, undo.moved, undo.restored) @@ -343,10 +335,7 @@ class RescheduleViewModel @Inject constructor( } catch (e: Exception) { MoveOutcome.Failed } - // Read off the outcome, the way [write] does — no second copy of the - // same fact to drift from it. - if (_outcome.value !is MoveOutcome.Undone) _abandoned.value += 1 - settle() + settle(token, landed = _outcome.value is MoveOutcome.Undone) } return true } @@ -356,8 +345,13 @@ class RescheduleViewModel @Inject constructor( _outcome.value = null } - private suspend fun prepare(request: MoveRequest): PreparedMove? { + private suspend fun prepare(request: MoveRequest, waited: Boolean): PreparedMove? { val detail = try { + // A drop that waited carries times from before the writes ahead of it. + if (waited && !occurrenceStillAt(request)) { + _outcome.value = MoveOutcome.Stale + return null + } repository.eventDetail(request.eventId) } catch (e: CancellationException) { throw e @@ -427,12 +421,12 @@ class RescheduleViewModel @Inject constructor( ) } - private suspend fun write(prepared: PreparedMove, scope: RecurringWriteScope) { + /** Write [prepared] in [scope]; true when it landed. */ + private suspend fun write(prepared: PreparedMove, scope: RecurringWriteScope): Boolean { val request = prepared.request if (endsBeforeItStarts(prepared, scope)) { _outcome.value = MoveOutcome.BlockedSeriesEnd - _abandoned.value += 1 - return + return false } _outcome.value = try { when { @@ -469,9 +463,16 @@ class RescheduleViewModel @Inject constructor( } catch (e: Exception) { MoveOutcome.Failed } - // Nothing landed, so the copy the view is holding has nothing to fade - // into — let it go now rather than at the end of the settle timeout. - if (_outcome.value !is MoveOutcome.Moved) _abandoned.value += 1 + return _outcome.value is MoveOutcome.Moved + } + + /** Whether [request]'s occurrence still begins where it was picked up. */ + private suspend fun occurrenceStillAt(request: MoveRequest): Boolean { + val begin = Instant.fromEpochMilliseconds(request.beginMillis) + val end = Instant.fromEpochMilliseconds(maxOf(request.endMillis, request.beginMillis + 1)) + return repository.eventOccurrences(request.eventId, begin..end).any { + it.eventId == request.eventId && it.start == begin + } } /** diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt index 9ed6cf4d..39910c46 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt @@ -60,7 +60,6 @@ import de.jeanlucmakiola.floret.identity.LocalFloretDarkTheme import de.jeanlucmakiola.floret.identity.rememberReduceMotion import de.jeanlucmakiola.floret.locale.currentLocale import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.StateFlow import kotlinx.datetime.DateTimeUnit import kotlinx.datetime.LocalDate import kotlinx.datetime.atStartOfDayIn @@ -611,14 +610,8 @@ class TimelineDragController { var settlingSelected: Boolean = false private set - /** - * The abandoned-move counter, read at the drop so the mark can't be a - * composition behind the tick it is compared against. - */ - var abandonedTicks: StateFlow? = null - - /** What [abandonedTicks] stood at when the settling drop was let go. */ - var settledAbandonedMark: Int = 0 + /** The token of the drop [settling] waits on; a drop's caller hands it to its `MoveRequest`. */ + var settledToken: Long = 0L private set private var source: TimedBlock? = null @@ -661,8 +654,8 @@ class TimelineDragController { var stripSeated: Boolean by mutableStateOf(false) private set - /** What [abandonedTicks] stood at when [stripSettling] was let go. */ - var stripAbandonedMark: Int = 0 + /** The token of the drop [stripSettling] waits on. */ + var stripToken: Long = 0L private set /** The shown day the finger picked the strip bar up on. */ @@ -756,7 +749,7 @@ class TimelineDragController { if (held.overGrid || held.dayShift == 0) return null stripSettling = held stripSeated = false - stripAbandonedMark = abandonedTicks?.value ?: 0 + stripToken = nextMoveToken() drops++ return StripDrop.Days(held.event, held.dayShift) } @@ -873,7 +866,7 @@ class TimelineDragController { settling = if (landed.edgeOnly || leavesGrid) landed.copy(pieces = emptyList()) else landed liftedInstanceId = landed.event.instanceId settledOrigin = landed.event.eventId to landed.event.start - settledAbandonedMark = abandonedTicks?.value ?: 0 + settledToken = nextMoveToken() settledEnd = end drops++ } @@ -1199,7 +1192,6 @@ fun rememberTimelineDragController(): TimelineDragController { controller.geometry.edgePx = with(density) { AUTO_SCROLL_EDGE.toPx() } controller.geometry.stepPx = with(density) { AUTO_SCROLL_STEP.toPx() } controller.slopPx = LocalViewConfiguration.current.touchSlop - controller.abandonedTicks = LocalEventMove.current?.abandoned return controller } @@ -1313,7 +1305,9 @@ fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = val use24Hour = LocalUse24HourFormat.current val locale = currentLocale() val reduceMotion = rememberReduceMotion() - val moveInFlight = moveInFlight() + val drops = dropStates() + val moveInFlight = controller.settledToken in drops.pending + val stripInFlight = controller.stripToken in drops.pending // Both live here rather than beside the controller: they read [drag], which // changes every frame, and this composable is the one that is meant to. @@ -1327,12 +1321,11 @@ fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = // recurring drop's scope dialog is up — and then until the grid draws the // drop itself. var handingOver by remember(controller.settling) { mutableStateOf(false) } - val abandoned = abandonedMoves() + val abandoned = controller.settledToken in drops.abandoned LaunchedEffect(controller.settling, moveInFlight, controller.settledOnGrid, abandoned) { if (controller.settling == null || moveInFlight) return@LaunchedEffect - // Past the mark this drop was let go on, it was abandoned: nothing will - // land and there is nothing to wait for. - if (abandoned == controller.settledAbandonedMark) { + // An abandoned drop: nothing will land and there is nothing to wait for. + if (!abandoned) { delay(if (controller.settledOnGrid) SETTLE_GRACE_MILLIS else SETTLE_TIMEOUT_MILLIS) } handingOver = true @@ -1342,9 +1335,10 @@ fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = } // The same hold for a bar moved along the all-day strip. - LaunchedEffect(controller.stripSettling, moveInFlight, controller.stripSeated, abandoned) { - if (controller.stripSettling == null || moveInFlight) return@LaunchedEffect - if (abandoned == controller.stripAbandonedMark) { + val stripAbandoned = controller.stripToken in drops.abandoned + LaunchedEffect(controller.stripSettling, stripInFlight, controller.stripSeated, stripAbandoned) { + if (controller.stripSettling == null || stripInFlight) return@LaunchedEffect + if (!stripAbandoned) { delay(if (controller.stripSeated) SETTLE_GRACE_MILLIS else SETTLE_TIMEOUT_MILLIS) } controller.releaseStrip() diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthDrag.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthDrag.kt index 9e7df88e..1f79fffe 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthDrag.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthDrag.kt @@ -14,8 +14,7 @@ import androidx.compose.ui.layout.boundsInRoot import androidx.compose.ui.layout.positionInRoot import androidx.compose.ui.unit.IntSize import de.jeanlucmakiola.calendula.domain.EventInstance -import de.jeanlucmakiola.calendula.ui.common.LocalEventMove -import kotlinx.coroutines.flow.StateFlow +import de.jeanlucmakiola.calendula.ui.common.nextMoveToken import kotlinx.datetime.LocalDate import kotlin.math.abs @@ -112,6 +111,8 @@ data class MonthChipDrop( val event: EventInstance, val grabDate: LocalDate, val targetDate: LocalDate, + /** The drop's identity, which its held copy waits on — see [MonthDragController.settledToken]. */ + val token: Long, ) /** @@ -172,14 +173,8 @@ class MonthDragController { */ private var undoable: MonthChipDrag? = null - /** - * The abandoned-move counter, read at the drop so the mark can't be a - * composition behind the tick it is compared against. - */ - var abandonedTicks: StateFlow? = null - - /** What [abandonedTicks] stood at when the settling chip was let go. */ - var settledAbandonedMark: Int = 0 + /** The token of the drop (or undo) [settling] is waiting on. */ + var settledToken: Long = 0L private set private var event: EventInstance? = null @@ -265,7 +260,7 @@ class MonthDragController { * there is no drop of this controller's left to undo, or the grid doesn't * seat the moved event where the chip would have to start from. */ - fun beginUndo(): Boolean { + fun beginUndo(token: Long): Boolean { val last = undoable ?: return false undoable = null val from = last.targetDate ?: return false @@ -274,7 +269,7 @@ class MonthDragController { liftedInstanceId = last.event.instanceId settledGhost = true settledInRoot = null - settledAbandonedMark = abandonedTicks?.value ?: 0 + settledToken = token return true } @@ -295,9 +290,9 @@ class MonthDragController { liftedInstanceId = landed.event.instanceId settledGhost = true undoable = settling - settledAbandonedMark = abandonedTicks?.value ?: 0 + settledToken = nextMoveToken() drops++ - return MonthChipDrop(landed.event, landed.grabDate, target) + return MonthChipDrop(landed.event, landed.grabDate, target, settledToken) } /** @@ -355,9 +350,7 @@ class MonthDragController { @Composable fun rememberMonthDragController(): MonthDragController { - val controller = remember { MonthDragController() } - controller.abandonedTicks = LocalEventMove.current?.abandoned - return controller + return remember { MonthDragController() } } val LocalMonthDrag = compositionLocalOf { null } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt index 83001dfb..777f4c21 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt @@ -125,8 +125,7 @@ import de.jeanlucmakiola.calendula.ui.common.eventTitleOverflow import de.jeanlucmakiola.calendula.ui.common.MoveTarget import de.jeanlucmakiola.calendula.ui.common.MoveRequest import de.jeanlucmakiola.calendula.ui.common.LocalEventMove -import de.jeanlucmakiola.calendula.ui.common.moveInFlight -import de.jeanlucmakiola.calendula.ui.common.abandonedMoves +import de.jeanlucmakiola.calendula.ui.common.dropStates import de.jeanlucmakiola.calendula.ui.common.OnUndoStarted import de.jeanlucmakiola.calendula.ui.common.ghostAlpha import de.jeanlucmakiola.calendula.ui.common.DragSnapHaptics @@ -515,7 +514,7 @@ fun MonthScreen( val chipDrag = rememberMonthDragController() // Undo moves the event back, so the chip travels back too. The // signal comes from the write, not from the chip that offers it. - OnUndoStarted { chipDrag.beginUndo() } + OnUndoStarted { token -> chipDrag.beginUndo(token) } Box( modifier = Modifier .padding(innerPadding) @@ -595,7 +594,9 @@ private fun MonthDragOverlay(controller: MonthDragController, showStartTimes: Bo val density = LocalDensity.current val timeFit = rememberMonthChipTimeFit(showStartTimes) val reduceMotion = rememberReduceMotion() - val moveInFlight = moveInFlight() + val drops = dropStates() + val moveInFlight = controller.settledToken in drops.pending + val abandoned = controller.settledToken in drops.abandoned // Here rather than beside the controller: this reads [drag], which changes // every frame, and this composable is the one that is meant to. DragSnapHaptics(controller.drag?.targetDate, controller.drops) @@ -608,14 +609,12 @@ private fun MonthDragOverlay(controller: MonthDragController, showStartTimes: Bo val glide = remember { Animatable(Offset.Zero, Offset.VectorConverter) } val glideSpec = MaterialTheme.motionScheme.fastSpatialSpec() val seat = controller.settledInRoot - val abandoned = abandonedMoves() LaunchedEffect(controller.settling, moveInFlight, seat, abandoned) { val landed = controller.settling if (landed == null || moveInFlight) return@LaunchedEffect if (seat == null) { - // Past the mark this drop was let go on, it was abandoned and no - // seat is ever coming. - if (abandoned == controller.settledAbandonedMark) delay(MONTH_SETTLE_TIMEOUT_MILLIS) + // An abandoned drop has no seat coming. + if (!abandoned) delay(MONTH_SETTLE_TIMEOUT_MILLIS) } else { if (!gliding) glide.snapTo(landed.topLeftInRoot) gliding = true @@ -2691,6 +2690,7 @@ private fun monthChipDragModifier( beginMillis = drop.event.start.toEpochMilliseconds(), endMillis = drop.event.end.toEpochMilliseconds(), target = MoveTarget.ByDays(delta), + token = drop.token, ), ) } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt index f05c0fc8..f84dd47e 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt @@ -592,6 +592,7 @@ private fun WeekContent( beginMillis = drop.event.start.toEpochMilliseconds(), endMillis = drop.event.end.toEpochMilliseconds(), target = MoveTarget.Start(start), + token = dragController.settledToken, ), ) } @@ -611,6 +612,7 @@ private fun WeekContent( beginMillis = drop.event.start.toEpochMilliseconds(), endMillis = drop.event.end.toEpochMilliseconds(), target = MoveTarget.Span(drop.start, drop.end), + token = dragController.settledToken, ), ) } @@ -626,6 +628,11 @@ private fun WeekContent( is StripDrop.Days -> MoveTarget.ByDays(drop.days) is StripDrop.Timed -> MoveTarget.Span(drop.start, drop.end) }, + // A bar dropped into the grid settles as a timed copy, not on the strip. + token = when (drop) { + is StripDrop.Days -> dragController.stripToken + is StripDrop.Timed -> dragController.settledToken + }, ), ) } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index ed0a4437..f1fc21d2 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -110,6 +110,7 @@ Couldn\'t move the event Calendula needs write access to move events That event no longer exists + That event moved before your drop could be saved Can\'t move an event past the end of its series A cancelled series can only move as a whole, and this would change which days it falls on diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt index 1576608b..172c1350 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt @@ -100,6 +100,8 @@ class RescheduleViewModelTest { return RescheduleViewModel(repo, io, FakeZoneSource()) } + private val RescheduleViewModel.abandonedCount: Int get() = drops.value.abandoned.size + private fun CoroutineScope.activate(vm: RescheduleViewModel): Job = launch { vm.movableCalendarIds.collect {} } @@ -270,54 +272,103 @@ class RescheduleViewModelTest { assertThat(vm.scopePrompt.value).isEqualTo(MoveScopePrompt(occurrenceOnly = true)) } - @Test - fun `a second drop is refused while the first is still in flight`( - @TempDir tempDir: Path, - ) = runTest(dispatcher) { - val fake = FakeCalendarDataSource().apply { - eventDetailResult = { detail(rrule = "FREQ=WEEKLY;BYDAY=MO") } + /** The occurrences [ids] still at the dragged start, as a waiting drop re-checks them. */ + private fun FakeCalendarDataSource.occurrencesAt(vararg ids: Long) { + instancesResult = { _, _ -> + ids.map { id -> + testEvent( + id = id, + start = Instant.fromEpochMilliseconds(beginMillis), + end = Instant.fromEpochMilliseconds(endMillis), + ) + } } - val vm = viewModel(tempDir, fake) - - assertThat(vm.move(toWednesday())).isTrue() - advanceUntilIdle() - - // Parked on the scope dialog, so the first drop still owns the pipeline. - assertThat(vm.move(oneHourLater())).isFalse() - advanceUntilIdle() - assertThat(fake.updatedEvents).isEmpty() } @Test fun `a drop of another event waits for the one in flight, then lands`( @TempDir tempDir: Path, ) = runTest(dispatcher) { - val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail() } } + val fake = FakeCalendarDataSource().apply { + eventDetailResult = { detail() } + occurrencesAt(7L) + } val vm = slowViewModel(tempDir, fake) - assertThat(vm.move(oneHourLater())).isTrue() - assertThat(vm.inFlight.value).isTrue() - val abandoned = vm.abandoned.value - assertThat(vm.move(toWednesday().copy(eventId = 7L))).isTrue() + val first = oneHourLater() + val second = toWednesday().copy(eventId = 7L) + assertThat(vm.move(first)).isTrue() + assertThat(vm.move(second)).isTrue() + assertThat(vm.drops.value.pending).containsExactly(first.token, second.token) advanceUntilIdle() assertThat(fake.updatedEvents.map { it.first }).containsExactly(42L, 7L).inOrder() - assertThat(vm.abandoned.value).isEqualTo(abandoned) + assertThat(vm.drops.value).isEqualTo(DropStates()) assertThat(vm.inFlight.value).isFalse() } @Test - fun `a second drop of the event in flight is still refused`( + fun `a failed drop lets go of its own copy only, not the one queued behind it`( @TempDir tempDir: Path, ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { + eventDetailResult = { detail() } + occurrencesAt(7L) + } + val vm = slowViewModel(tempDir, fake) + + // Zero distance: refused in prepare. + val nowhere = oneHourLater().copy(target = MoveTarget.Start(Instant.fromEpochMilliseconds(beginMillis))) + val queued = toWednesday().copy(eventId = 7L) + vm.move(nowhere) + vm.move(queued) + advanceUntilIdle() + + assertThat(vm.drops.value.abandoned).containsExactly(nowhere.token) + assertThat(fake.updatedEvents.map { it.first }).containsExactly(7L) + } + + @Test + fun `a waiting drop whose occurrence an earlier write moved is not written`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + // The first drop moved event 42, so nothing of it begins at the old start any more. val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail() } } val vm = slowViewModel(tempDir, fake) - assertThat(vm.move(oneHourLater())).isTrue() - assertThat(vm.move(toWednesday())).isFalse() + val again = toWednesday() + vm.move(oneHourLater()) + vm.move(again) advanceUntilIdle() assertThat(fake.updatedEvents).hasSize(1) + assertThat(vm.outcome.value).isEqualTo(MoveOutcome.Stale) + assertThat(vm.drops.value.abandoned).containsExactly(again.token) + } + + @Test + fun `a drop made while a scope dialog is up runs once it is answered`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { + eventDetailResult = { id -> if (id == 42L) detail(rrule = "FREQ=WEEKLY") else detail() } + occurrencesAt(7L) + } + val vm = viewModel(tempDir, fake) + + vm.move(oneHourLater()) + advanceUntilIdle() + val queued = toWednesday().copy(eventId = 7L) + assertThat(vm.move(queued)).isTrue() + advanceUntilIdle() + assertThat(fake.updatedEvents).isEmpty() + assertThat(vm.drops.value.pending).contains(queued.token) + + vm.moveWithScope(RecurringWriteScope.AllEvents) + advanceUntilIdle() + + assertThat(fake.updatedEvents.map { it.first }).containsExactly(42L, 7L).inOrder() + assertThat(vm.drops.value.pending).isEmpty() } @Test @@ -333,14 +384,14 @@ class RescheduleViewModelTest { vm.move(toWednesday()) advanceUntilIdle() - val abandoned = vm.abandoned.value + val abandoned = vm.abandonedCount assertThat(vm.undo(undo)).isFalse() advanceUntilIdle() assertThat(fake.updatedEvents).hasSize(1) // Nothing ticks: the refusal started no journey of its own, and the // counter is global — a tick here would release the copy the drop that // *is* in flight is holding, before the grid has drawn it. - assertThat(vm.abandoned.value).isEqualTo(abandoned) + assertThat(vm.abandonedCount).isEqualTo(abandoned) } @Test @@ -355,13 +406,13 @@ class RescheduleViewModelTest { advanceUntilIdle() val undo = (vm.outcome.value as MoveOutcome.Moved).undo!! - val abandoned = vm.abandoned.value + val abandoned = vm.abandonedCount fake.writeError = SecurityException("permission revoked") assertThat(vm.undo(undo)).isTrue() advanceUntilIdle() assertThat(vm.outcome.value).isEqualTo(MoveOutcome.WriteDenied) - assertThat(vm.abandoned.value).isEqualTo(abandoned + 1) + assertThat(vm.abandonedCount).isEqualTo(abandoned + 1) } @Test @@ -374,12 +425,12 @@ class RescheduleViewModelTest { advanceUntilIdle() val undo = (vm.outcome.value as MoveOutcome.Moved).undo!! - val abandoned = vm.abandoned.value + val abandoned = vm.abandonedCount vm.undo(undo) advanceUntilIdle() assertThat(vm.outcome.value).isEqualTo(MoveOutcome.Undone) - assertThat(vm.abandoned.value).isEqualTo(abandoned) + assertThat(vm.abandonedCount).isEqualTo(abandoned) } @Test @@ -603,14 +654,14 @@ class RescheduleViewModelTest { ), )).isTrue() advanceUntilIdle() - assertThat(vm.abandoned.value).isEqualTo(1) + assertThat(vm.abandonedCount).isEqualTo(1) // Refused by the provider, after the write was attempted. fake.writeError = SecurityException("revoked") vm.move(oneHourLater()) advanceUntilIdle() assertThat(vm.outcome.value).isEqualTo(MoveOutcome.WriteDenied) - assertThat(vm.abandoned.value).isEqualTo(2) + assertThat(vm.abandonedCount).isEqualTo(2) } @Test @@ -624,11 +675,11 @@ class RescheduleViewModelTest { vm.move(oneHourLater()) advanceUntilIdle() - assertThat(vm.abandoned.value).isEqualTo(0) + assertThat(vm.abandonedCount).isEqualTo(0) vm.cancelScope() advanceUntilIdle() - assertThat(vm.abandoned.value).isEqualTo(1) + assertThat(vm.abandonedCount).isEqualTo(1) } @Test @@ -640,7 +691,7 @@ class RescheduleViewModelTest { advanceUntilIdle() assertThat(vm.outcome.value).isInstanceOf(MoveOutcome.Moved::class.java) - assertThat(vm.abandoned.value).isEqualTo(0) + assertThat(vm.abandonedCount).isEqualTo(0) } @Test @@ -753,7 +804,7 @@ class RescheduleViewModelTest { assertThat(vm.scopePrompt.value).isNull() assertThat(vm.outcome.value).isEqualTo(MoveOutcome.BlockedCancelledSeries) - assertThat(vm.abandoned.value).isEqualTo(1) + assertThat(vm.abandonedCount).isEqualTo(1) assertThat(fake.updatedEvents).isEmpty() assertThat(fake.updatedOccurrences).isEmpty() } From 0595124ecd16e14627a6d97ae0e065fcb92234aa Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 13 Nov 2026 12:45:48 +0100 Subject: [PATCH 3/5] Drops follow their event if it moved, failures stay on the chip (#494) --- .../calendula/ui/common/EventMoveScope.kt | 9 +- .../ui/common/RescheduleViewModel.kt | 123 ++++++++++----- .../calendula/ui/common/TimelineDrag.kt | 45 ++++-- .../calendula/ui/week/WeekScreen.kt | 10 +- .../ui/common/RescheduleViewModelTest.kt | 149 +++++++++++++----- 5 files changed, 235 insertions(+), 101 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveScope.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveScope.kt index 6eb9050f..ba46db11 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveScope.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveScope.kt @@ -63,8 +63,8 @@ private val moveTokens = AtomicLong() /** A fresh identity for one drop or undo, unique for the process. */ fun nextMoveToken(): Long = moveTokens.incrementAndGet() -/** How many ended drops [DropStates] remembers — far more than can be on screen at once. */ -private const val ENDED_KEPT = 32 +/** How many abandoned drops [DropStates] remembers — far more than can be on screen at once. */ +private const val ABANDONED_KEPT = 32 /** Per-drop progress, by token — see `RescheduleViewModel.drops`. */ @Immutable @@ -76,7 +76,10 @@ data class DropStates( fun ended(token: Long, landed: Boolean): DropStates = copy( pending = pending - token, - abandoned = if (landed) abandoned else (abandoned + token).toList().takeLast(ENDED_KEPT).toSet(), + // Insertion-ordered, so the oldest go first. + abandoned = if (landed) abandoned else (abandoned + token).let { kept -> + if (kept.size > ABANDONED_KEPT) kept.drop(kept.size - ABANDONED_KEPT).toSet() else kept + }, ) } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt index d40b0622..78ee7eb0 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt @@ -79,7 +79,7 @@ data class MoveRequest( val endMillis: Long, val target: MoveTarget, /** The drop's own identity, from [nextMoveToken], which the view holding its copy waits on. */ - val token: Long = nextMoveToken(), + val token: Long, ) /** @@ -122,8 +122,9 @@ sealed interface MoveOutcome { data object Gone : MoveOutcome /** - * A drop that waited for an earlier one found its occurrence no longer where - * it was picked up — the earlier write moved it — so it is not written (#494). + * The drop's occurrence is no longer where it was picked up — an earlier + * write moved it — and a recurring one can't be followed there, so it is not + * written (#494). */ data object Stale : MoveOutcome @@ -164,7 +165,7 @@ class RescheduleViewModel @Inject constructor( private val _inFlight = MutableStateFlow(false) - /** True while any write runs or waits, a queued drop included — what the confirmation chip holds for. */ + /** True while any drop or undo runs or waits — what the confirmation chip holds for. */ val inFlight: StateFlow = _inFlight.asStateFlow() private val _drops = MutableStateFlow(DropStates()) @@ -192,6 +193,13 @@ class RescheduleViewModel @Inject constructor( * written. Drops arriving in that window wait in [queue] and run in order. */ private var busy = false + set(value) { + field = value + _inFlight.value = value + } + + /** Whether a drop since the pipeline was last idle failed — see [report]. */ + private var failedThisRun = false /** Drops that arrived while another was in flight, oldest first (#494). */ private val queue = ArrayDeque() @@ -240,19 +248,14 @@ class RescheduleViewModel @Inject constructor( return false } _drops.update { it.started(request.token) } - if (busy) { - queue.addLast(request) - } else { - run(request, waited = false) - } - syncInFlight() + if (busy) queue.addLast(request) else run(request) return true } - private fun run(request: MoveRequest, waited: Boolean) { + private fun run(request: MoveRequest) { busy = true viewModelScope.launch { - val prepared = prepare(request, waited) + val prepared = prepare(request) if (prepared == null) { settle(request.token, landed = false) return@launch @@ -262,7 +265,7 @@ class RescheduleViewModel @Inject constructor( if (!prepared.isRecurring || (seriesOnly && prepared.canRealign)) { settle(request.token, write(prepared, RecurringWriteScope.AllEvents)) } else if (seriesOnly) { - _outcome.value = MoveOutcome.BlockedCancelledSeries + report(MoveOutcome.BlockedCancelledSeries) settle(request.token, landed = false) } else { // Still busy: the scope dialog is now the thing in flight. @@ -298,13 +301,26 @@ class RescheduleViewModel @Inject constructor( /** End the drop [token], then run the next queued one, if any. */ private fun settle(token: Long, landed: Boolean) { _drops.update { it.ended(token, landed) } - busy = false - queue.removeFirstOrNull()?.let { run(it, waited = true) } - syncInFlight() + // Straight on to the next, so busy never reads false between two drops. + val next = queue.removeFirstOrNull() + if (next != null) { + run(next) + } else { + busy = false + failedThisRun = false + } } - private fun syncInFlight() { - _inFlight.value = busy || queue.isNotEmpty() + /** + * Show [outcome] on the chip. While drops run back to back, a failure stays + * up rather than being replaced by a later drop's success: the grid shows + * what moved, but only the chip says what didn't. + */ + private fun report(outcome: MoveOutcome) { + val failure = outcome !is MoveOutcome.Moved && outcome !is MoveOutcome.Undone + if (!failure && failedThisRun) return + if (failure) failedThisRun = true + _outcome.value = outcome } /** @@ -316,11 +332,10 @@ class RescheduleViewModel @Inject constructor( * reports whether it landed. */ fun undo(undo: MoveUndo): Boolean { - if (busy || queue.isNotEmpty()) return false + if (busy) return false val token = nextMoveToken() _drops.update { it.started(token) } busy = true - syncInFlight() _undoStarted.value = token viewModelScope.launch { _outcome.value = try { @@ -345,25 +360,27 @@ class RescheduleViewModel @Inject constructor( _outcome.value = null } - private suspend fun prepare(request: MoveRequest, waited: Boolean): PreparedMove? { - val detail = try { - // A drop that waited carries times from before the writes ahead of it. - if (waited && !occurrenceStillAt(request)) { - _outcome.value = MoveOutcome.Stale - return null - } - repository.eventDetail(request.eventId) + private suspend fun prepare(dropped: MoveRequest): PreparedMove? { + val zone = zoneSource.current() + val (detail, request) = try { + val detail = repository.eventDetail(dropped.eventId) + // The drop carries the times the grid drew, which an earlier write + // may have moved on since (#494). + detail to if (occurrenceStillAt(dropped)) dropped else followed(dropped, detail, zone) } catch (e: CancellationException) { throw e } catch (e: NoSuchEventException) { - _outcome.value = MoveOutcome.Gone + report(MoveOutcome.Gone) return null } catch (e: Exception) { - _outcome.value = MoveOutcome.Failed + report(MoveOutcome.Failed) + return null + } + if (request == null) { + report(MoveOutcome.Stale) return null } - val zone = zoneSource.current() val original = detail.toEditForm(request.beginMillis, request.endMillis, zone) val shifted = when (val target = request.target) { is MoveTarget.Start -> original.shiftedTo(target.instant, zone) @@ -376,7 +393,7 @@ class RescheduleViewModel @Inject constructor( // The UNTIL check is deferred to the write, which knows how far the move // reaches and so which date has to clear it. if ((shifted.problems(zone) - EventFormProblem.RecurrenceEndsBeforeStart).isNotEmpty()) { - _outcome.value = MoveOutcome.Failed + report(MoveOutcome.Failed) return null } @@ -425,10 +442,10 @@ class RescheduleViewModel @Inject constructor( private suspend fun write(prepared: PreparedMove, scope: RecurringWriteScope): Boolean { val request = prepared.request if (endsBeforeItStarts(prepared, scope)) { - _outcome.value = MoveOutcome.BlockedSeriesEnd + report(MoveOutcome.BlockedSeriesEnd) return false } - _outcome.value = try { + val outcome = try { when { !prepared.isRecurring || scope == RecurringWriteScope.AllEvents -> repository.updateEvent(request.eventId, prepared.original, prepared.updated) @@ -463,18 +480,46 @@ class RescheduleViewModel @Inject constructor( } catch (e: Exception) { MoveOutcome.Failed } - return _outcome.value is MoveOutcome.Moved + report(outcome) + return outcome is MoveOutcome.Moved } - /** Whether [request]'s occurrence still begins where it was picked up. */ + /** Whether [request]'s occurrence is still exactly where it was picked up. */ private suspend fun occurrenceStillAt(request: MoveRequest): Boolean { val begin = Instant.fromEpochMilliseconds(request.beginMillis) - val end = Instant.fromEpochMilliseconds(maxOf(request.endMillis, request.beginMillis + 1)) - return repository.eventOccurrences(request.eventId, begin..end).any { - it.eventId == request.eventId && it.start == begin + val end = Instant.fromEpochMilliseconds(request.endMillis) + val range = begin..maxOf(end, Instant.fromEpochMilliseconds(request.beginMillis + 1)) + return repository.eventOccurrences(request.eventId, range).any { + it.eventId == request.eventId && it.start == begin && it.end == end } } + /** + * [dropped] rebased onto where its event now is, so the last drop wins: + * absolute targets stand as they are, and a whole-day shift is re-aimed at + * the day it was dropped on. Null for a recurring event, whose moved + * occurrence can't be told apart from the rest of its series. + */ + private fun followed(dropped: MoveRequest, detail: EventDetail, zone: TimeZone): MoveRequest? { + if (detail.rrule != null && !detail.isException) return null + val now = detail.instance + val target = when (val target = dropped.target) { + is MoveTarget.ByDays -> { + val dayZone = if (now.isAllDay) TimeZone.UTC else zone + val aimed = Instant.fromEpochMilliseconds(dropped.beginMillis).toLocalDateTime(dayZone).date + .plus(target.days, DateTimeUnit.DAY) + val from = now.start.toLocalDateTime(dayZone).date + MoveTarget.ByDays((aimed.toEpochDays() - from.toEpochDays()).toInt()) + } + else -> target + } + return dropped.copy( + beginMillis = now.start.toEpochMilliseconds(), + endMillis = now.end.toEpochMilliseconds(), + target = target, + ) + } + /** * Whether this write would leave a rule whose `UNTIL` precedes the first day * it now applies to — the provider then generates nothing at all. Which date diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt index 39910c46..102d0df1 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt @@ -138,9 +138,17 @@ data class StripDrag(val event: EventInstance, val dayShift: Int = 0, val overGr sealed interface StripDrop { val event: EventInstance - data class Days(override val event: EventInstance, val days: Int) : StripDrop + /** The drop's identity, which its held copy waits on. */ + val token: Long - data class Timed(override val event: EventInstance, val start: Instant, val end: Instant) : StripDrop + data class Days(override val event: EventInstance, val days: Int, override val token: Long) : StripDrop + + data class Timed( + override val event: EventInstance, + val start: Instant, + val end: Instant, + override val token: Long, + ) : StripDrop } /** The event a long press left selected (#141). */ @@ -297,6 +305,8 @@ data class TimelineDrop( val startMin: Int, /** Minutes from the event's start to the dragged block's top edge. */ val clipOffsetMin: Int = 0, + /** The drop's identity, which its held copy waits on; set by the controller's finish. */ + val token: Long = 0L, ) /** The id a drafted block's stand-in event carries, which no provider row has (#153). */ @@ -340,7 +350,13 @@ internal fun draftBlock(draft: EventInstance, date: LocalDate, zone: TimeZone): enum class ResizeEdge { Top, Bottom } /** Where a finished resize asks its event to begin and end. */ -data class TimelineResizeDrop(val event: EventInstance, val start: Instant, val end: Instant) +data class TimelineResizeDrop( + val event: EventInstance, + val start: Instant, + val end: Instant, + /** The drop's identity, which its held copy waits on; set by the controller's finishResize. */ + val token: Long = 0L, +) /** * [event] resized by its [edge] to the span [startMin]..+[spanMin] from [date]'s @@ -610,7 +626,7 @@ class TimelineDragController { var settlingSelected: Boolean = false private set - /** The token of the drop [settling] waits on; a drop's caller hands it to its `MoveRequest`. */ + /** The token of the drop [settling] waits on. */ var settledToken: Long = 0L private set @@ -742,16 +758,16 @@ class TimelineDragController { val zone = dragZone cancel() if (held.overGrid && landed != null && zone != null && !landed.edgeOnly) { - settle(landed) + val token = settle(landed) val start = TimelineDrop(landed.event, landed.date, landed.startMin).startInstant(zone) - return StripDrop.Timed(held.event, start, start + draftMinutes.minutes) + return StripDrop.Timed(held.event, start, start + draftMinutes.minutes, token) } if (held.overGrid || held.dayShift == 0) return null stripSettling = held stripSeated = false stripToken = nextMoveToken() drops++ - return StripDrop.Days(held.event, held.dayShift) + return StripDrop.Days(held.event, held.dayShift, stripToken) } /** Let the held strip drop go, once the strip shows the event at its new days or the move failed. */ @@ -848,12 +864,15 @@ class TimelineDragController { if (landed == null || landed.slot == origin) return null // A draft dropped on a day off screen would vanish, so it stays where it was (#153). if (landed.edgeOnly && landed.event.isDraft) return null - settle(landed) - return TimelineDrop(landed.event, landed.date, landed.startMin, clipOffset) + val token = settle(landed) + return TimelineDrop(landed.event, landed.date, landed.startMin, clipOffset, token) } - /** Keep [landed]'s copy on the target as [settling] until [release]; [end] is a resize's. */ - private fun settle(landed: TimelineDrag, end: Instant? = null) { + /** + * Keep [landed]'s copy on the target as [settling] until [release]; [end] is a + * resize's. Returns the drop's fresh token. + */ + private fun settle(landed: TimelineDrag, end: Instant? = null): Long { // Landed on midnight to midnight, the event moves to the all-day strip (#296). val leavesGrid = !landed.event.isDraft && landed.eventSpanMin > 0 && landed.eventStartMin.mod(MINUTES_PER_DAY) == 0 && @@ -869,6 +888,7 @@ class TimelineDragController { settledToken = nextMoveToken() settledEnd = end drops++ + return settledToken } /** @@ -937,8 +957,7 @@ class TimelineDragController { cancel() if (landed == null || landed.slot == origin) return null val drop = resizedDrop(landed.event, edge, landed.date, landed.eventStartMin, landed.eventSpanMin, zone) - settle(landed, end = drop.end) - return drop + return drop.copy(token = settle(landed, end = drop.end)) } /** diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt index f84dd47e..f79e4914 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt @@ -592,7 +592,7 @@ private fun WeekContent( beginMillis = drop.event.start.toEpochMilliseconds(), endMillis = drop.event.end.toEpochMilliseconds(), target = MoveTarget.Start(start), - token = dragController.settledToken, + token = drop.token, ), ) } @@ -612,7 +612,7 @@ private fun WeekContent( beginMillis = drop.event.start.toEpochMilliseconds(), endMillis = drop.event.end.toEpochMilliseconds(), target = MoveTarget.Span(drop.start, drop.end), - token = dragController.settledToken, + token = drop.token, ), ) } @@ -628,11 +628,7 @@ private fun WeekContent( is StripDrop.Days -> MoveTarget.ByDays(drop.days) is StripDrop.Timed -> MoveTarget.Span(drop.start, drop.end) }, - // A bar dropped into the grid settles as a timed copy, not on the strip. - token = when (drop) { - is StripDrop.Days -> dragController.stripToken - is StripDrop.Timed -> dragController.settledToken - }, + token = drop.token, ), ) } diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt index 172c1350..fc174f1b 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt @@ -13,6 +13,7 @@ import de.jeanlucmakiola.calendula.testEvent import de.jeanlucmakiola.calendula.testPrefs import de.jeanlucmakiola.calendula.testRepository import de.jeanlucmakiola.calendula.testSettings +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job @@ -80,18 +81,17 @@ class RescheduleViewModelTest { status = status, ) - private fun viewModel(tempDir: Path, fake: FakeCalendarDataSource): RescheduleViewModel { - val repo = testRepository( - fake, tempDir, dispatcher, - prefs = testPrefs(tempDir, CoroutineScope(dispatcher)), - settings = testSettings(tempDir, CoroutineScope(dispatcher)), - ) - return RescheduleViewModel(repo, dispatcher, FakeZoneSource()) - } - - /** A view model whose provider reads suspend until the scheduler runs, so a drop stays in flight. */ - private fun slowViewModel(tempDir: Path, fake: FakeCalendarDataSource): RescheduleViewModel { - val io = StandardTestDispatcher(dispatcher.scheduler) + /** + * [present] are the events whose occurrence the provider still reports where + * a drop picked it up; [io] a [StandardTestDispatcher] keeps a drop in flight. + */ + private fun viewModel( + tempDir: Path, + fake: FakeCalendarDataSource, + present: List = listOf(42L, 7L), + io: CoroutineDispatcher = dispatcher, + ): RescheduleViewModel { + fake.occurrencesAt(present) val repo = testRepository( fake, tempDir, io, prefs = testPrefs(tempDir, CoroutineScope(dispatcher)), @@ -100,6 +100,22 @@ class RescheduleViewModelTest { return RescheduleViewModel(repo, io, FakeZoneSource()) } + private fun slowViewModel(tempDir: Path, fake: FakeCalendarDataSource): RescheduleViewModel = + viewModel(tempDir, fake, io = StandardTestDispatcher(dispatcher.scheduler)) + + /** Every query answers with an occurrence of each of [ids] exactly at the queried span. */ + private fun FakeCalendarDataSource.occurrencesAt(ids: List) { + instancesResult = { begin, end -> + ids.map { id -> + testEvent( + id = id, + start = Instant.fromEpochMilliseconds(begin), + end = Instant.fromEpochMilliseconds(end), + ) + } + } + } + private val RescheduleViewModel.abandonedCount: Int get() = drops.value.abandoned.size private fun CoroutineScope.activate(vm: RescheduleViewModel): Job = @@ -111,6 +127,7 @@ class RescheduleViewModelTest { beginMillis = beginMillis, endMillis = endMillis, target = MoveTarget.ByDays(2), + token = nextMoveToken(), ) /** A drop an hour later, expressed the way a timeline drag expresses one. */ @@ -119,6 +136,7 @@ class RescheduleViewModelTest { beginMillis = beginMillis, endMillis = endMillis, target = MoveTarget.Start(Instant.fromEpochMilliseconds(beginMillis + 3_600_000L)), + token = nextMoveToken(), ) /** The end dragged an hour later, the start left where it is. */ @@ -130,6 +148,7 @@ class RescheduleViewModelTest { Instant.fromEpochMilliseconds(beginMillis), Instant.fromEpochMilliseconds(endMillis + 3_600_000L), ), + token = nextMoveToken(), ) @Test @@ -272,31 +291,17 @@ class RescheduleViewModelTest { assertThat(vm.scopePrompt.value).isEqualTo(MoveScopePrompt(occurrenceOnly = true)) } - /** The occurrences [ids] still at the dragged start, as a waiting drop re-checks them. */ - private fun FakeCalendarDataSource.occurrencesAt(vararg ids: Long) { - instancesResult = { _, _ -> - ids.map { id -> - testEvent( - id = id, - start = Instant.fromEpochMilliseconds(beginMillis), - end = Instant.fromEpochMilliseconds(endMillis), - ) - } - } - } - @Test fun `a drop of another event waits for the one in flight, then lands`( @TempDir tempDir: Path, ) = runTest(dispatcher) { val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail() } - occurrencesAt(7L) } val vm = slowViewModel(tempDir, fake) val first = oneHourLater() - val second = toWednesday().copy(eventId = 7L) + val second = toWednesday().copy(eventId = 7L, token = nextMoveToken()) assertThat(vm.move(first)).isTrue() assertThat(vm.move(second)).isTrue() assertThat(vm.drops.value.pending).containsExactly(first.token, second.token) @@ -313,13 +318,15 @@ class RescheduleViewModelTest { ) = runTest(dispatcher) { val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail() } - occurrencesAt(7L) } val vm = slowViewModel(tempDir, fake) // Zero distance: refused in prepare. - val nowhere = oneHourLater().copy(target = MoveTarget.Start(Instant.fromEpochMilliseconds(beginMillis))) - val queued = toWednesday().copy(eventId = 7L) + val nowhere = oneHourLater().copy( + target = MoveTarget.Start(Instant.fromEpochMilliseconds(beginMillis)), + token = nextMoveToken(), + ) + val queued = toWednesday().copy(eventId = 7L, token = nextMoveToken()) vm.move(nowhere) vm.move(queued) advanceUntilIdle() @@ -329,21 +336,80 @@ class RescheduleViewModelTest { } @Test - fun `a waiting drop whose occurrence an earlier write moved is not written`( + fun `the same event dropped twice ends up where it was dropped last`( @TempDir tempDir: Path, ) = runTest(dispatcher) { - // The first drop moved event 42, so nothing of it begins at the old start any more. - val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail() } } + val hour = 3_600_000L + val fake = FakeCalendarDataSource() val vm = slowViewModel(tempDir, fake) + // Once the first drop is written, the event sits an hour later. + fake.eventDetailResult = { + if (fake.updatedEvents.isEmpty()) detail() else detail(anchorMillis = beginMillis + hour) + } + fake.instancesResult = { _, _ -> + val shift = if (fake.updatedEvents.isEmpty()) 0L else hour + listOf( + testEvent( + id = 42L, + start = Instant.fromEpochMilliseconds(beginMillis + shift), + end = Instant.fromEpochMilliseconds(endMillis + shift), + ), + ) + } - val again = toWednesday() vm.move(oneHourLater()) - vm.move(again) + // Picked up from where the grid still drew it, and dropped two days on. + vm.move(toWednesday()) advanceUntilIdle() - assertThat(fake.updatedEvents).hasSize(1) + assertThat(fake.updatedEvents).hasSize(2) + val last = fake.updatedEvents.last().third + assertThat(last.start.dateTime).isEqualTo(LocalDateTime(LocalDate(2026, 6, 10), LocalTime(13, 0))) + } + + @Test + fun `a recurring occurrence that moved since the drop is not written`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail(rrule = "FREQ=WEEKLY") } } + val vm = viewModel(tempDir, fake, present = emptyList()) + + val drop = toWednesday() + vm.move(drop) + advanceUntilIdle() + + assertThat(vm.scopePrompt.value).isNull() assertThat(vm.outcome.value).isEqualTo(MoveOutcome.Stale) - assertThat(vm.drops.value.abandoned).containsExactly(again.token) + assertThat(vm.drops.value.abandoned).containsExactly(drop.token) + assertThat(fake.updatedEvents).isEmpty() + } + + @Test + fun `a deleted event reports itself as gone, not moved`(@TempDir tempDir: Path) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { eventDetailResult = { null } } + val vm = viewModel(tempDir, fake, present = emptyList()) + + vm.move(oneHourLater()) + advanceUntilIdle() + + assertThat(vm.outcome.value).isEqualTo(MoveOutcome.Gone) + } + + @Test + fun `a failure stays on the chip while later drops land`(@TempDir tempDir: Path) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { + eventDetailResult = { id -> + if (id == 42L) detail(rrule = "FREQ=WEEKLY;BYDAY=MO,WE", status = EventStatus.Cancelled) else detail() + } + } + val vm = slowViewModel(tempDir, fake) + + vm.move(toWednesday()) + vm.move(toWednesday().copy(eventId = 7L, token = nextMoveToken())) + advanceUntilIdle() + + assertThat(fake.updatedEvents.map { it.first }).containsExactly(7L) + assertThat(vm.outcome.value).isEqualTo(MoveOutcome.BlockedCancelledSeries) } @Test @@ -352,13 +418,12 @@ class RescheduleViewModelTest { ) = runTest(dispatcher) { val fake = FakeCalendarDataSource().apply { eventDetailResult = { id -> if (id == 42L) detail(rrule = "FREQ=WEEKLY") else detail() } - occurrencesAt(7L) } val vm = viewModel(tempDir, fake) vm.move(oneHourLater()) advanceUntilIdle() - val queued = toWednesday().copy(eventId = 7L) + val queued = toWednesday().copy(eventId = 7L, token = nextMoveToken()) assertThat(vm.move(queued)).isTrue() advanceUntilIdle() assertThat(fake.updatedEvents).isEmpty() @@ -510,6 +575,7 @@ class RescheduleViewModelTest { target = MoveTarget.Start( Instant.fromEpochMilliseconds(beginMillis + 26 * 3_600_000L), ), + token = nextMoveToken(), ), ) advanceUntilIdle() @@ -530,6 +596,7 @@ class RescheduleViewModelTest { beginMillis = beginMillis, endMillis = endMillis, target = MoveTarget.Start(Instant.fromEpochMilliseconds(beginMillis)), + token = nextMoveToken(), ), ) advanceUntilIdle() @@ -651,6 +718,7 @@ class RescheduleViewModelTest { beginMillis = beginMillis, endMillis = endMillis, target = MoveTarget.Start(Instant.fromEpochMilliseconds(beginMillis)), + token = nextMoveToken(), ), )).isTrue() advanceUntilIdle() @@ -763,6 +831,7 @@ class RescheduleViewModelTest { beginMillis = beginMillis, endMillis = endMillis, target = MoveTarget.Span(sundayEvening, Instant.fromEpochMilliseconds(endMillis)), + token = nextMoveToken(), ), ) advanceUntilIdle() @@ -834,6 +903,7 @@ class RescheduleViewModelTest { beginMillis = dayStart, endMillis = dayStart + 86_400_000L, target = MoveTarget.Span(start, Instant.fromEpochMilliseconds(endMillis)), + token = nextMoveToken(), ), ) advanceUntilIdle() @@ -870,6 +940,7 @@ class RescheduleViewModelTest { beginMillis = dayStart, endMillis = dayStart + 86_400_000L, target = MoveTarget.Span(eight, eight + 1.hours), + token = nextMoveToken(), ), ) advanceUntilIdle() From 1925e1c2f4a9572f792f8e312ce74f1153b0707a Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 13 Nov 2026 13:18:11 +0100 Subject: [PATCH 4/5] Hold every dropped copy until its own write lands, cancel drops behind a dialog, keep the first Undo (#494) --- .../calendula/ui/common/EventMoveHost.kt | 1 + .../ui/common/RescheduleViewModel.kt | 49 +++++-- .../calendula/ui/common/TimelineDrag.kt | 106 +++++++++++++-- .../calendula/ui/month/MonthDrag.kt | 40 +++++- .../calendula/ui/month/MonthScreen.kt | 123 +++++++++++++----- app/src/main/res/values/strings.xml | 1 + .../ui/common/RescheduleViewModelTest.kt | 50 +++++-- 7 files changed, 307 insertions(+), 63 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveHost.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveHost.kt index 73cfe836..1d026711 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveHost.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveHost.kt @@ -91,6 +91,7 @@ fun EventMoveHost( MoveOutcome.WriteDenied -> stringResource(R.string.event_move_write_denied) MoveOutcome.Gone -> stringResource(R.string.event_move_gone) MoveOutcome.Stale -> stringResource(R.string.event_move_stale) + MoveOutcome.Skipped -> stringResource(R.string.event_move_skipped) MoveOutcome.BlockedSeriesEnd -> stringResource(R.string.event_move_blocked_series_end) MoveOutcome.BlockedCancelledSeries -> stringResource(R.string.event_move_blocked_cancelled_series) MoveOutcome.Failed -> stringResource(R.string.event_move_failed) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt index 78ee7eb0..37d1d380 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt @@ -121,6 +121,9 @@ sealed interface MoveOutcome { /** The event vanished (sync, another device) between the read and the write. */ data object Gone : MoveOutcome + /** Drops that waited behind a scope dialog, cancelled rather than run after it (#494). */ + data object Skipped : MoveOutcome + /** * The drop's occurrence is no longer where it was picked up — an earlier * write moved it — and a recurring one can't be followed there, so it is not @@ -198,8 +201,8 @@ class RescheduleViewModel @Inject constructor( _inFlight.value = value } - /** Whether a drop since the pipeline was last idle failed — see [report]. */ - private var failedThisRun = false + /** Whether the chip has shown anything since the pipeline was last idle — see [report]. */ + private var reportedThisRun = false /** Drops that arrived while another was in flight, oldest first (#494). */ private val queue = ArrayDeque() @@ -247,6 +250,11 @@ class RescheduleViewModel @Inject constructor( _drops.update { it.ended(request.token, landed = false) } return false } + if (_scopePrompt.value != null) { + _drops.update { it.ended(request.token, landed = false) } + report(MoveOutcome.Skipped) + return false + } _drops.update { it.started(request.token) } if (busy) queue.addLast(request) else run(request) return true @@ -268,7 +276,10 @@ class RescheduleViewModel @Inject constructor( report(MoveOutcome.BlockedCancelledSeries) settle(request.token, landed = false) } else { - // Still busy: the scope dialog is now the thing in flight. + // Still busy: the scope dialog is now the thing in flight. Drops + // waiting behind it are cancelled, not held behind a question + // about another event. + skipQueued() pending = prepared _scopePrompt.value = MoveScopePrompt( occurrenceOnly = !prepared.canRealign, @@ -307,19 +318,35 @@ class RescheduleViewModel @Inject constructor( run(next) } else { busy = false - failedThisRun = false + reportedThisRun = false } } + private fun skipQueued() { + if (queue.isEmpty()) return + val skipped = queue.map { it.token } + queue.clear() + _drops.update { states -> skipped.fold(states) { acc, token -> acc.ended(token, landed = false) } } + report(MoveOutcome.Skipped) + } + /** - * Show [outcome] on the chip. While drops run back to back, a failure stays - * up rather than being replaced by a later drop's success: the grid shows - * what moved, but only the chip says what didn't. + * Show [outcome] on the chip. While drops run back to back, the first move's + * chip and its Undo stay up rather than being replaced by the next success, + * and a failure replaces it: the grid shows what moved, but only the chip + * says what didn't. A later write to the shown move's own event takes that + * Undo away, since putting the event back would undo the later move too. */ - private fun report(outcome: MoveOutcome) { + private fun report(outcome: MoveOutcome, eventId: Long? = null) { val failure = outcome !is MoveOutcome.Moved && outcome !is MoveOutcome.Undone - if (!failure && failedThisRun) return - if (failure) failedThisRun = true + if (!failure && reportedThisRun) { + val shown = _outcome.value as? MoveOutcome.Moved + if (shown?.undo != null && shown.undo.eventId == eventId) { + _outcome.value = shown.copy(undo = null) + } + return + } + reportedThisRun = true _outcome.value = outcome } @@ -480,7 +507,7 @@ class RescheduleViewModel @Inject constructor( } catch (e: Exception) { MoveOutcome.Failed } - report(outcome) + report(outcome, request.eventId) return outcome is MoveOutcome.Moved } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt index 102d0df1..f3bb03e1 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt @@ -20,6 +20,7 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.Stable import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue +import androidx.compose.runtime.key import androidx.compose.runtime.mutableStateListOf import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf @@ -350,6 +351,19 @@ internal fun draftBlock(draft: EventInstance, date: LocalDate, zone: TimeZone): enum class ResizeEdge { Top, Bottom } /** Where a finished resize asks its event to begin and end. */ +/** A drop parked by a newer pickup while its write still runs — see [TimelineDragController.held]. */ +data class HeldDrop( + val copy: TimelineDrag, + val token: Long, + val instanceId: Long, + /** The event row and start the drop came from, as [TimelineDragController.ghosts] matches them. */ + val origin: Pair, + val handedOver: Boolean = false, +) { + fun ghosts(block: TimedBlock): Boolean = block.event.instanceId == instanceId || + (block.event.eventId == origin.first && block.event.start == origin.second) +} + data class TimelineResizeDrop( val event: EventInstance, val start: Instant, @@ -528,6 +542,14 @@ class TimelineDragController { */ private var settledOrigin: Pair? by mutableStateOf(null) + /** + * Earlier drops still being written when another block was picked up. They + * keep their copy and their ghost until their own token settles, rather than + * vanishing and flashing back at the old slot (#494). + */ + var held: List by mutableStateOf(emptyList()) + private set + /** How long a block drafted on an empty slot starts out, in minutes (#153). */ var draftMinutes: Int = SettingsPrefs.DEFAULT_EVENT_DURATION @@ -696,7 +718,7 @@ class TimelineDragController { dragDuration = (block.event.end - block.event.start).coerceAtLeast(Duration.ZERO) dragKept = zone.repeatedHourOffsetAt(block.event.start) this.titleLines = titleLines - settling = null + parkSettling() isDragging = true liftedInstanceId = block.event.instanceId grab = pointerInRoot - blockInRoot @@ -735,7 +757,7 @@ class TimelineDragController { dragDuration = draftMinutes.minutes dragKept = null titleLines = 1 - settling = null + parkSettling() isDragging = true liftedInstanceId = event.instanceId grab = Offset(0f, geometry.hourPx * STRIP_GRAB_MINUTES / 60f) @@ -815,12 +837,36 @@ class TimelineDragController { * an event that crosses midnight (#253). */ fun ghosts(block: TimedBlock): Boolean { + if (held.any { !it.handedOver && it.ghosts(block) }) return true if (liftedInstanceId == null) return false if (block.event.instanceId == liftedInstanceId) return true val (eventId, start) = settledOrigin ?: return false return block.event.eventId == eventId && block.event.start == start } + /** Move a still-settling drop to [held] before a new pickup takes the slot. */ + private fun parkSettling() { + val landed = settling + val origin = settledOrigin + if (landed != null && origin != null && !landed.event.isDraft) { + held = held + HeldDrop(landed, settledToken, landed.event.instanceId, origin) + } + settling = null + settledOnGrid = false + settledOrigin = null + settledEnd = null + } + + /** Stop ghosting the held drop [token]'s source while its copy fades. */ + fun handOverHeld(token: Long) { + held = held.map { if (it.token == token) it.copy(handedOver = true) else it } + } + + /** Drop the held copy [token] once it has faded. */ + fun releaseHeld(token: Long) { + held = held.filterNot { it.token == token } + } + /** * What a day column now holds, so a settled drop can tell when the grid has * caught up with it. Matched on the landing slot plus either the event row @@ -908,7 +954,7 @@ class TimelineDragController { resize = session clipOffsetMin = 0 this.titleLines = titleLines - settling = null + parkSettling() isDragging = true pointerMoved = true liftedInstanceId = block.event.instanceId @@ -1312,6 +1358,9 @@ private const val SETTLE_TIMEOUT_MILLIS = 900L */ const val SETTLE_FADE_MILLIS: Int = 250 +/** How long a held copy waits after its write before fading — the grid's re-read, unconfirmed. */ +const val HELD_SETTLE_MILLIS: Long = 300L + /** * The floating block, drawn over the whole calendar so it is free of the day * column's clip and of the scroll viewport's rounded corners. @@ -1319,10 +1368,6 @@ const val SETTLE_FADE_MILLIS: Int = 250 @Composable fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = Modifier) { var origin by remember { mutableStateOf(Offset.Zero) } - val density = LocalDensity.current - val dark = LocalFloretDarkTheme.current - val use24Hour = LocalUse24HourFormat.current - val locale = currentLocale() val reduceMotion = rememberReduceMotion() val drops = dropStates() val moveInFlight = controller.settledToken in drops.pending @@ -1371,6 +1416,9 @@ fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = .clearAndSetSemantics { } .onGloballyPositioned { origin = it.positionInRoot() }, ) { + controller.held.forEach { parked -> + key(parked.token) { HeldTimelineCopy(controller, parked, origin) } + } val drag = controller.drag ?: controller.settling ?: return@Box // Landed: the copy sinks back to the grid's own plane while the write runs. val landed = controller.drag == null @@ -1384,6 +1432,47 @@ fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = animationSpec = tween(SETTLE_FADE_MILLIS), label = "drag-handover", ) + TimelineCopy(drag, controller.geometry, origin, lift, copyAlpha) + } +} + +/** A drop parked by a newer pickup: held at its landing slot until its own write settles. */ +@Composable +private fun HeldTimelineCopy(controller: TimelineDragController, parked: HeldDrop, origin: Offset) { + val drops = dropStates() + val pending = parked.token in drops.pending + val abandoned = parked.token in drops.abandoned + var fading by remember { mutableStateOf(false) } + LaunchedEffect(pending, abandoned) { + if (pending) return@LaunchedEffect + // No seat to confirm this one, so give the grid a beat to re-read. + if (!abandoned) delay(HELD_SETTLE_MILLIS) + fading = true + controller.handOverHeld(parked.token) + delay(SETTLE_FADE_MILLIS.toLong()) + controller.releaseHeld(parked.token) + } + val alpha by animateFloatAsState( + targetValue = if (fading) 0f else 1f, + animationSpec = tween(SETTLE_FADE_MILLIS), + label = "held-handover", + ) + TimelineCopy(parked.copy, controller.geometry, origin, lift = 0f, copyAlpha = alpha) +} + +/** [drag]'s pieces drawn over the calendar, clipped to the timeline's viewport. */ +@Composable +private fun TimelineCopy( + drag: TimelineDrag, + geometry: TimelineGeometry, + origin: Offset, + lift: Float, + copyAlpha: Float, +) { + val density = LocalDensity.current + val dark = LocalFloretDarkTheme.current + val use24Hour = LocalUse24HourFormat.current + val locale = currentLocale() val paint = eventPaint(drag.event, dark) val title = drag.event.title.ifBlank { stringResource(R.string.event_untitled) } // Read off the event rather than the held block, so grabbing either half @@ -1399,7 +1488,7 @@ fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = // so no column clip or rounded corner cuts it, but it still belongs to // the timeline: a piece a whole day tall reaches far past both ends of // the viewport, and unclipped it paints over the headers above (#267). - val port = controller.geometry.viewport?.takeIf { it.isAttached }?.boundsInRoot() + val port = geometry.viewport?.takeIf { it.isAttached }?.boundsInRoot() // How much of each piece the viewport actually shows. A multi-day event // has a piece per day, and the widest one is a full 24 hours — taller // than the screen, its top at a midnight scrolled out of sight. Picking @@ -1451,7 +1540,6 @@ fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = ) } } - } } /** One piece of a dragged block, floating over the calendar. */ diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthDrag.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthDrag.kt index 1f79fffe..004d595a 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthDrag.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthDrag.kt @@ -106,6 +106,13 @@ data class MonthChipDrag( val time: String? = null, ) +/** A drop parked by a newer pickup while its write still runs — see [MonthDragController.held]. */ +data class HeldChip( + val copy: MonthChipDrag, + val token: Long, + val handedOver: Boolean = false, +) + /** Where a finished chip drag asks its event to go, as a whole-day shift. */ data class MonthChipDrop( val event: EventInstance, @@ -173,6 +180,14 @@ class MonthDragController { */ private var undoable: MonthChipDrag? = null + /** + * Earlier drops still being written when another chip was picked up. They + * keep their copy and their ghost until their own token settles, rather than + * vanishing and flashing back on the old day (#494). + */ + var held: List by mutableStateOf(emptyList()) + private set + /** The token of the drop (or undo) [settling] is waiting on. */ var settledToken: Long = 0L private set @@ -203,7 +218,7 @@ class MonthDragController { this.event = event this.grabDate = grabDate this.time = time - settling = null + parkSettling() isDragging = true liftedInstanceId = event.instanceId grab = pointerInRoot - chipInRoot @@ -229,6 +244,29 @@ class MonthDragController { settledGhost = false } + /** Whether [event]'s chip is the source of a held drop, so still a ghost. */ + fun isHeldSource(event: EventInstance): Boolean = + held.any { !it.handedOver && it.copy.event.instanceId == event.instanceId } + + /** Move a still-settling drop to [held] before a new pickup takes the slot. */ + private fun parkSettling() { + val landed = settling + if (landed != null) held = held + HeldChip(landed, settledToken) + settling = null + settledInRoot = null + settledGhost = false + } + + /** Stop ghosting the held drop [token]'s source while its copy fades. */ + fun handOverHeld(token: Long) { + held = held.map { if (it.token == token) it.copy(handedOver = true) else it } + } + + /** Drop the held copy [token] once it has faded. */ + fun releaseHeld(token: Long) { + held = held.filterNot { it.token == token } + } + /** * Whether [days] holds the chip the grid has drawn for the settled drop — * the seat the copy is on its way to, which must ghost until it gets there. diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt index 777f4c21..f183c8b0 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt @@ -129,6 +129,7 @@ import de.jeanlucmakiola.calendula.ui.common.dropStates import de.jeanlucmakiola.calendula.ui.common.OnUndoStarted import de.jeanlucmakiola.calendula.ui.common.ghostAlpha import de.jeanlucmakiola.calendula.ui.common.DragSnapHaptics +import de.jeanlucmakiola.calendula.ui.common.HELD_SETTLE_MILLIS import de.jeanlucmakiola.calendula.ui.common.SETTLE_FADE_MILLIS import de.jeanlucmakiola.calendula.ui.common.SETTLE_GRACE_MILLIS import de.jeanlucmakiola.calendula.ui.common.EventMoveScope @@ -590,8 +591,6 @@ private const val MONTH_SETTLE_TIMEOUT_MILLIS = 450L @Composable private fun MonthDragOverlay(controller: MonthDragController, showStartTimes: Boolean) { var origin by remember { mutableStateOf(Offset.Zero) } - val dark = LocalFloretDarkTheme.current - val density = LocalDensity.current val timeFit = rememberMonthChipTimeFit(showStartTimes) val reduceMotion = rememberReduceMotion() val drops = dropStates() @@ -634,6 +633,9 @@ private fun MonthDragOverlay(controller: MonthDragController, showStartTimes: Bo .clearAndSetSemantics { } .onGloballyPositioned { origin = it.positionInRoot() }, ) { + controller.held.forEach { parked -> + key(parked.token) { HeldMonthChip(controller, parked, origin, timeFit) } + } val drag = controller.drag ?: controller.settling ?: return@Box // Landed: the copy sinks back to the grid's own plane while the write runs. val lift by animateFloatAsState( @@ -645,42 +647,94 @@ private fun MonthDragOverlay(controller: MonthDragController, showStartTimes: Bo animationSpec = tween(SETTLE_FADE_MILLIS), label = "chip-handover", ) - MonthBar( - event = drag.event, - dark = dark, - continuesLeft = false, - continuesRight = false, - // The time the source chip carried, drawn only if this one-column - // cut-out of it has the room — which a slice of a multi-day bar has - // not, however wide the bar was. - time = drag.time, + MonthChipCopy( + drag = drag, + // Read at layout, so the glide doesn't recompose every frame. + at = { if (gliding) glide.value else drag.topLeftInRoot }, + origin = origin, + lift = lift, + alpha = copyAlpha, timeFit = timeFit, - chipWidth = with(density) { drag.sizePx.width.toDp() }, - modifier = Modifier - // Absolute: these are root coordinates, and the direction-aware - // offset would mirror them across the screen in an RTL layout. - .absoluteOffset { - val at = if (gliding) glide.value else drag.topLeftInRoot - IntOffset( - (at.x - origin.x).roundToInt(), - (at.y - origin.y).roundToInt(), - ) - } - .width(with(density) { drag.sizePx.width.toDp() }) - .height(with(density) { drag.sizePx.height.toDp() }) - .padding(horizontal = MONTH_CHIP_INSET, vertical = 1.dp) - .graphicsLayer { - scaleX = 1f + 0.04f * lift - scaleY = 1f + 0.04f * lift - shadowElevation = 8.dp.toPx() * lift - alpha = copyAlpha - shape = EventChipShape - clip = false - }, ) } } +/** A drop parked by a newer pickup: held on its landing day until its own write settles. */ +@Composable +private fun HeldMonthChip( + controller: MonthDragController, + parked: HeldChip, + origin: Offset, + timeFit: MonthChipTimeFit?, +) { + val drops = dropStates() + val pending = parked.token in drops.pending + val abandoned = parked.token in drops.abandoned + var fading by remember { mutableStateOf(false) } + LaunchedEffect(pending, abandoned) { + if (pending) return@LaunchedEffect + // No seat to confirm this one, so give the grid a beat to re-read. + if (!abandoned) delay(HELD_SETTLE_MILLIS) + fading = true + controller.handOverHeld(parked.token) + delay(SETTLE_FADE_MILLIS.toLong()) + controller.releaseHeld(parked.token) + } + val alpha by animateFloatAsState( + targetValue = if (fading) 0f else 1f, + animationSpec = tween(SETTLE_FADE_MILLIS), + label = "held-chip-handover", + ) + MonthChipCopy(parked.copy, { parked.copy.topLeftInRoot }, origin, lift = 0f, alpha = alpha, timeFit = timeFit) +} + +/** [drag]'s chip floating over the grid with its top left at [at], in root coordinates. */ +@Composable +private fun MonthChipCopy( + drag: MonthChipDrag, + at: () -> Offset, + origin: Offset, + lift: Float, + alpha: Float, + timeFit: MonthChipTimeFit?, +) { + val density = LocalDensity.current + val dark = LocalFloretDarkTheme.current + MonthBar( + event = drag.event, + dark = dark, + continuesLeft = false, + continuesRight = false, + // The time the source chip carried, drawn only if this one-column + // cut-out of it has the room — which a slice of a multi-day bar has + // not, however wide the bar was. + time = drag.time, + timeFit = timeFit, + chipWidth = with(density) { drag.sizePx.width.toDp() }, + modifier = Modifier + // Absolute: these are root coordinates, and the direction-aware + // offset would mirror them across the screen in an RTL layout. + .absoluteOffset { + val topLeft = at() + IntOffset( + (topLeft.x - origin.x).roundToInt(), + (topLeft.y - origin.y).roundToInt(), + ) + } + .width(with(density) { drag.sizePx.width.toDp() }) + .height(with(density) { drag.sizePx.height.toDp() }) + .padding(horizontal = MONTH_CHIP_INSET, vertical = 1.dp) + .graphicsLayer { + scaleX = 1f + 0.04f * lift + scaleY = 1f + 0.04f * lift + shadowElevation = 8.dp.toPx() * lift + this.alpha = alpha + shape = EventChipShape + clip = false + }, + ) +} + /** * Content for both scrolling styles. No swipe detector and no [AnimatedContent]: * vertical scrolling owns the gesture, and the list never swaps wholesale — it @@ -2837,7 +2891,8 @@ private fun MonthBar( // The source stays put as a ghost while its floating copy travels. val monthDrag = LocalMonthDrag.current val lifted = monthDrag?.liftedInstanceId == event.instanceId || - monthDrag?.isSettledChip(event, days) == true + monthDrag?.isSettledChip(event, days) == true || + monthDrag?.isHeldSource(event) == true val ghost = ghostAlpha(lifted) val shape = monthBarShape(continuesLeft, continuesRight) Box( diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index f1fc21d2..5d3a342d 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -111,6 +111,7 @@ Calendula needs write access to move events That event no longer exists That event moved before your drop could be saved + Moves made while the question was open weren\'t saved Can\'t move an event past the end of its series A cancelled series can only move as a whole, and this would change which days it falls on diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt index fc174f1b..5085188f 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt @@ -413,27 +413,61 @@ class RescheduleViewModelTest { } @Test - fun `a drop made while a scope dialog is up runs once it is answered`( + fun `drops waiting when a scope dialog opens are cancelled, not held behind it`( @TempDir tempDir: Path, ) = runTest(dispatcher) { val fake = FakeCalendarDataSource().apply { eventDetailResult = { id -> if (id == 42L) detail(rrule = "FREQ=WEEKLY") else detail() } } - val vm = viewModel(tempDir, fake) + val vm = slowViewModel(tempDir, fake) vm.move(oneHourLater()) - advanceUntilIdle() val queued = toWednesday().copy(eventId = 7L, token = nextMoveToken()) - assertThat(vm.move(queued)).isTrue() + vm.move(queued) advanceUntilIdle() - assertThat(fake.updatedEvents).isEmpty() - assertThat(vm.drops.value.pending).contains(queued.token) + + assertThat(vm.scopePrompt.value).isNotNull() + assertThat(vm.drops.value.abandoned).containsExactly(queued.token) + assertThat(vm.outcome.value).isEqualTo(MoveOutcome.Skipped) + + // One made while the dialog is up is refused the same way. + val during = toWednesday().copy(eventId = 7L, token = nextMoveToken()) + assertThat(vm.move(during)).isFalse() vm.moveWithScope(RecurringWriteScope.AllEvents) advanceUntilIdle() + assertThat(fake.updatedEvents.map { it.first }).containsExactly(42L) + // The skip stays on the chip; the grid shows the move itself. + assertThat(vm.outcome.value).isEqualTo(MoveOutcome.Skipped) + } - assertThat(fake.updatedEvents.map { it.first }).containsExactly(42L, 7L).inOrder() - assertThat(vm.drops.value.pending).isEmpty() + @Test + fun `the first move of a run keeps the chip and its undo`(@TempDir tempDir: Path) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail() } } + val vm = slowViewModel(tempDir, fake) + + vm.move(oneHourLater()) + vm.move(toWednesday().copy(eventId = 7L, token = nextMoveToken())) + advanceUntilIdle() + + val shown = vm.outcome.value as MoveOutcome.Moved + assertThat(fake.updatedEvents).hasSize(2) + assertThat(shown.undo?.eventId).isEqualTo(42L) + } + + @Test + fun `a later move of the same event takes the first move's undo away`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail() } } + val vm = slowViewModel(tempDir, fake) + + vm.move(oneHourLater()) + vm.move(toWednesday()) + advanceUntilIdle() + + assertThat(fake.updatedEvents).hasSize(2) + assertThat((vm.outcome.value as MoveOutcome.Moved).undo).isNull() } @Test From 8d640b595eb84b3426d30d418e6a8dc8d5b94e37 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 13 Nov 2026 13:30:49 +0100 Subject: [PATCH 5/5] Third review round: undo by drop, queued chip messages, recent-write stale check, held strip and seat (#494) --- .../calendula/ui/common/EventMoveHost.kt | 1 + .../calendula/ui/common/EventMoveScope.kt | 58 +++++-- .../ui/common/RescheduleViewModel.kt | 143 ++++++++++++------ .../calendula/ui/common/TimelineDrag.kt | 136 ++++++++++++----- .../calendula/ui/month/MonthDrag.kt | 44 ++++-- .../calendula/ui/month/MonthScreen.kt | 54 ++++--- .../calendula/ui/week/WeekScreen.kt | 63 +++++--- app/src/main/res/values/strings.xml | 1 + .../ui/common/RescheduleViewModelTest.kt | 89 ++++++++--- .../calendula/ui/common/TimelineDropTest.kt | 26 ++-- .../calendula/ui/week/WeekLayoutTest.kt | 4 +- 11 files changed, 431 insertions(+), 188 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveHost.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveHost.kt index 1d026711..542222aa 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveHost.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveHost.kt @@ -92,6 +92,7 @@ fun EventMoveHost( MoveOutcome.Gone -> stringResource(R.string.event_move_gone) MoveOutcome.Stale -> stringResource(R.string.event_move_stale) MoveOutcome.Skipped -> stringResource(R.string.event_move_skipped) + MoveOutcome.QueueFull -> stringResource(R.string.event_move_queue_full) MoveOutcome.BlockedSeriesEnd -> stringResource(R.string.event_move_blocked_series_end) MoveOutcome.BlockedCancelledSeries -> stringResource(R.string.event_move_blocked_cancelled_series) MoveOutcome.Failed -> stringResource(R.string.event_move_failed) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveScope.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveScope.kt index ba46db11..8eaa5b6c 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveScope.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventMoveScope.kt @@ -1,6 +1,9 @@ package de.jeanlucmakiola.calendula.ui.common import androidx.compose.animation.core.animateFloatAsState +import kotlinx.coroutines.delay +import androidx.compose.animation.core.tween +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.Composable import androidx.compose.runtime.Immutable import androidx.compose.runtime.LaunchedEffect @@ -48,8 +51,8 @@ class EventMoveScope( * every visible block reads it as a composition local. */ val drops: StateFlow, - /** The token of the undo write that just began — see `RescheduleViewModel.undoStarted`. */ - val undoStarted: StateFlow, + /** The undo write that just began — see `RescheduleViewModel.undoStarted`. */ + val undoStarted: StateFlow, /** Open an event in the edit form — the pointer-free route to the same change. */ val edit: (EventInstance) -> Unit, ) { @@ -85,7 +88,7 @@ data class DropStates( private val NO_DROPS = MutableStateFlow(DropStates()) -private val NEVER_UNDONE = MutableStateFlow(null) +private val NEVER_UNDONE = MutableStateFlow(null) /** * Every drop's progress — empty wherever moving is off. A drag overlay keeps @@ -98,22 +101,57 @@ fun dropStates(): DropStates { } /** - * Runs [onUndo] with the undo's token when an undo write begins, and never for - * one that began before this composable came on screen. + * Runs [onUndo] when an undo write begins, and never for one that began before + * this composable came on screen. */ @Composable -fun OnUndoStarted(onUndo: (token: Long) -> Unit) { +fun OnUndoStarted(onUndo: (UndoStart) -> Unit) { val flow = LocalEventMove.current?.undoStarted ?: NEVER_UNDONE - val token by flow.collectAsStateWithLifecycle() - var seen by remember { mutableStateOf(token) } - LaunchedEffect(token) { - val started = token + val start by flow.collectAsStateWithLifecycle() + var seen by remember { mutableStateOf(start) } + LaunchedEffect(start) { + val started = start if (started == null || started == seen) return@LaunchedEffect seen = started onUndo(started) } } +/** + * The life of a drop parked by a newer pickup (#494): held while its [token] + * is pending, then a beat after the grid [seated] it — or [timeoutMillis] if it + * never does — handed over and faded, then released. Returns the copy's alpha. + * An abandoned drop skips the wait; nothing is coming to seat it. + */ +@Composable +fun rememberHeldSettle( + token: Long, + seated: Boolean, + timeoutMillis: Long, + onHandOver: () -> Unit, + onRelease: () -> Unit, +): Float { + val drops = dropStates() + val pending = token in drops.pending + val abandoned = token in drops.abandoned + val handOver by rememberUpdatedState(onHandOver) + val release by rememberUpdatedState(onRelease) + var fading by remember(token) { mutableStateOf(false) } + LaunchedEffect(token, pending, abandoned, seated) { + if (pending) return@LaunchedEffect + if (!abandoned) delay(if (seated) SETTLE_GRACE_MILLIS else timeoutMillis) + fading = true + handOver() + delay(SETTLE_FADE_MILLIS.toLong()) + release() + } + return animateFloatAsState( + targetValue = if (fading) 0f else 1f, + animationSpec = tween(SETTLE_FADE_MILLIS), + label = "held-settle", + ).value +} + /** Opacity the source block keeps while its floating copy travels. */ const val GHOST_ALPHA: Float = 0.3f diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt index 37d1d380..5ef83d4a 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModel.kt @@ -41,7 +41,10 @@ import kotlinx.datetime.atStartOfDayIn import kotlinx.datetime.plus import kotlinx.datetime.toLocalDateTime import kotlin.coroutines.cancellation.CancellationException +import kotlin.time.Duration.Companion.seconds import kotlin.time.Instant +import kotlin.time.TimeMark +import kotlin.time.TimeSource import javax.inject.Inject private const val MILLIS_PER_MINUTE = 60_000 @@ -50,6 +53,9 @@ private const val MINUTES_PER_DAY = 24 * 60 /** How many drops may wait behind the one in flight before more are refused. */ private const val MAX_QUEUED = 8 +/** How long after a write its event's drawn times may still be the old ones. */ +private val RECENT_WRITE = 3.seconds + /** Where a dragged event should land. */ sealed interface MoveTarget { /** A new start instant — a timeline drag, which moves time and day at once. */ @@ -78,6 +84,8 @@ data class MoveRequest( val beginMillis: Long, val endMillis: Long, val target: MoveTarget, + /** Whether the dragged occurrence is all-day, which decides the zone its day is read in. */ + val isAllDay: Boolean, /** The drop's own identity, from [nextMoveToken], which the view holding its copy waits on. */ val token: Long, ) @@ -98,8 +106,13 @@ data class MoveUndo( val moved: EventForm, /** The form as it stood before the move. */ val restored: EventForm, + /** The token of the drop this undoes, so the view that drew it carries the right chip back. */ + val dropToken: Long, ) +/** An undo write that just began: its own [token], and the [dropToken] of the drop it undoes. */ +data class UndoStart(val token: Long, val dropToken: Long) + sealed interface MoveOutcome { /** * Written. [startMillis] is where the dragged occurrence now begins, for the @@ -124,9 +137,12 @@ sealed interface MoveOutcome { /** Drops that waited behind a scope dialog, cancelled rather than run after it (#494). */ data object Skipped : MoveOutcome + /** More drops waiting than the queue holds; this one was not taken. */ + data object QueueFull : MoveOutcome + /** - * The drop's occurrence is no longer where it was picked up — an earlier - * write moved it — and a recurring one can't be followed there, so it is not + * The drop's recurring occurrence may no longer be where it was picked up — + * its series was just written — and can't be followed there, so it is not * written (#494). */ data object Stale : MoveOutcome @@ -182,14 +198,14 @@ class RescheduleViewModel @Inject constructor( */ val drops: StateFlow = _drops.asStateFlow() - private val _undoStarted = MutableStateFlow(null) + private val _undoStarted = MutableStateFlow(null) /** - * The token of an undo write the moment it begins, before the provider has - * anything to re-read, so the view that drew the drop can carry its chip back - * rather than let it reappear on the old day. Its fate then reads off [drops]. + * An undo write the moment it begins, before the provider has anything to + * re-read, so the view that drew the drop can carry its chip back rather than + * let it reappear on the old day. Its fate then reads off [drops]. */ - val undoStarted: StateFlow = _undoStarted.asStateFlow() + val undoStarted: StateFlow = _undoStarted.asStateFlow() /** * Set while a drop is being prepared, waits on its scope dialog, or is @@ -201,8 +217,18 @@ class RescheduleViewModel @Inject constructor( _inFlight.value = value } - /** Whether the chip has shown anything since the pipeline was last idle — see [report]. */ - private var reportedThisRun = false + /** Whether a move has been announced since the pipeline was last idle — see [report]. */ + private var movedThisRun = false + + /** Outcomes waiting for the chip, shown one after another as each is consumed. */ + private val notices = ArrayDeque() + + /** + * When each event's series (by id) or single occurrence (by id and begin) + * was last written. The grid draws the old times until it re-reads, so a + * drop on a recurring event written moments ago may carry stale times (#494). + */ + private val written = mutableMapOf, TimeMark>() /** Drops that arrived while another was in flight, oldest first (#494). */ private val queue = ArrayDeque() @@ -248,6 +274,7 @@ class RescheduleViewModel @Inject constructor( fun move(request: MoveRequest): Boolean { if (queue.size >= MAX_QUEUED) { _drops.update { it.ended(request.token, landed = false) } + report(MoveOutcome.QueueFull) return false } if (_scopePrompt.value != null) { @@ -318,7 +345,7 @@ class RescheduleViewModel @Inject constructor( run(next) } else { busy = false - reportedThisRun = false + movedThisRun = false } } @@ -331,23 +358,41 @@ class RescheduleViewModel @Inject constructor( } /** - * Show [outcome] on the chip. While drops run back to back, the first move's - * chip and its Undo stay up rather than being replaced by the next success, - * and a failure replaces it: the grid shows what moved, but only the chip - * says what didn't. A later write to the shown move's own event takes that - * Undo away, since putting the event back would undo the later move too. + * Queue [outcome] for the chip. While drops run back to back only the first + * move is announced, so its Undo stays on offer; every failure gets its own + * turn after it, since the grid shows what moved but only the chip says what + * didn't. A later write to the announced move's event takes that Undo away, + * as putting the event back would undo the later move too. */ private fun report(outcome: MoveOutcome, eventId: Long? = null) { - val failure = outcome !is MoveOutcome.Moved && outcome !is MoveOutcome.Undone - if (!failure && reportedThisRun) { - val shown = _outcome.value as? MoveOutcome.Moved - if (shown?.undo != null && shown.undo.eventId == eventId) { - _outcome.value = shown.copy(undo = null) + if (outcome is MoveOutcome.Moved) { + if (movedThisRun) { + dropUndoFor(eventId) + return } + movedThisRun = true + } else if (outcome == (notices.lastOrNull() ?: _outcome.value)) { return } - reportedThisRun = true - _outcome.value = outcome + if (_outcome.value == null) _outcome.value = outcome else notices.addLast(outcome) + } + + private fun dropUndoFor(eventId: Long?) { + fun MoveOutcome.withoutUndo(): MoveOutcome = + if (this is MoveOutcome.Moved && undo?.eventId == eventId) copy(undo = null) else this + _outcome.value = _outcome.value?.withoutUndo() + val rest = notices.map { it.withoutUndo() } + notices.clear() + notices.addAll(rest) + } + + private fun markWritten(eventId: Long, beginMillis: Long?) { + written[eventId to beginMillis] = TimeSource.Monotonic.markNow() + } + + private fun writtenRecently(eventId: Long, beginMillis: Long): Boolean { + written.entries.removeAll { it.value.elapsedNow() > RECENT_WRITE } + return (eventId to null) in written || (eventId to beginMillis) in written } /** @@ -363,7 +408,8 @@ class RescheduleViewModel @Inject constructor( val token = nextMoveToken() _drops.update { it.started(token) } busy = true - _undoStarted.value = token + _undoStarted.value = UndoStart(token, undo.dropToken) + markWritten(undo.eventId, null) viewModelScope.launch { _outcome.value = try { repository.updateEvent(undo.eventId, undo.moved, undo.restored) @@ -382,18 +428,15 @@ class RescheduleViewModel @Inject constructor( return true } - /** Clear the outcome once the screen has shown it. */ + /** Clear the outcome once the screen has shown it, bringing on the next one waiting. */ fun consumeOutcome() { - _outcome.value = null + _outcome.value = notices.removeFirstOrNull() } private suspend fun prepare(dropped: MoveRequest): PreparedMove? { val zone = zoneSource.current() - val (detail, request) = try { - val detail = repository.eventDetail(dropped.eventId) - // The drop carries the times the grid drew, which an earlier write - // may have moved on since (#494). - detail to if (occurrenceStillAt(dropped)) dropped else followed(dropped, detail, zone) + val detail = try { + repository.eventDetail(dropped.eventId) } catch (e: CancellationException) { throw e } catch (e: NoSuchEventException) { @@ -403,9 +446,15 @@ class RescheduleViewModel @Inject constructor( report(MoveOutcome.Failed) return null } - if (request == null) { + // The drop carries the times the grid drew, which an earlier write may + // have moved on since (#494). + val request = if (detail.rrule == null || detail.isException) { + followed(dropped, detail, zone) + } else if (writtenRecently(dropped.eventId, dropped.beginMillis)) { report(MoveOutcome.Stale) return null + } else { + dropped } val original = detail.toEditForm(request.beginMillis, request.endMillis, zone) @@ -507,35 +556,27 @@ class RescheduleViewModel @Inject constructor( } catch (e: Exception) { MoveOutcome.Failed } + if (outcome is MoveOutcome.Moved) { + markWritten(request.eventId, request.beginMillis.takeIf { scope == RecurringWriteScope.ThisEvent && prepared.isRecurring }) + } report(outcome, request.eventId) return outcome is MoveOutcome.Moved } - /** Whether [request]'s occurrence is still exactly where it was picked up. */ - private suspend fun occurrenceStillAt(request: MoveRequest): Boolean { - val begin = Instant.fromEpochMilliseconds(request.beginMillis) - val end = Instant.fromEpochMilliseconds(request.endMillis) - val range = begin..maxOf(end, Instant.fromEpochMilliseconds(request.beginMillis + 1)) - return repository.eventOccurrences(request.eventId, range).any { - it.eventId == request.eventId && it.start == begin && it.end == end - } - } - /** - * [dropped] rebased onto where its event now is, so the last drop wins: - * absolute targets stand as they are, and a whole-day shift is re-aimed at - * the day it was dropped on. Null for a recurring event, whose moved - * occurrence can't be told apart from the rest of its series. + * A one-off [dropped] rebased onto where its event now is, so the last drop + * wins: absolute targets stand as they are, and a whole-day shift is re-aimed + * at the day it was dropped on. Unchanged when the event hasn't moved. */ - private fun followed(dropped: MoveRequest, detail: EventDetail, zone: TimeZone): MoveRequest? { - if (detail.rrule != null && !detail.isException) return null + private fun followed(dropped: MoveRequest, detail: EventDetail, zone: TimeZone): MoveRequest { val now = detail.instance val target = when (val target = dropped.target) { is MoveTarget.ByDays -> { - val dayZone = if (now.isAllDay) TimeZone.UTC else zone - val aimed = Instant.fromEpochMilliseconds(dropped.beginMillis).toLocalDateTime(dayZone).date + // Each day read the way its own kind anchors it: all-day at UTC. + val aimed = Instant.fromEpochMilliseconds(dropped.beginMillis) + .toLocalDateTime(if (dropped.isAllDay) TimeZone.UTC else zone).date .plus(target.days, DateTimeUnit.DAY) - val from = now.start.toLocalDateTime(dayZone).date + val from = now.start.toLocalDateTime(if (now.isAllDay) TimeZone.UTC else zone).date MoveTarget.ByDays((aimed.toEpochDays() - from.toEpochDays()).toInt()) } else -> target @@ -544,6 +585,7 @@ class RescheduleViewModel @Inject constructor( beginMillis = now.start.toEpochMilliseconds(), endMillis = now.end.toEpochMilliseconds(), target = target, + isAllDay = now.isAllDay, ) } @@ -611,6 +653,7 @@ class RescheduleViewModel @Inject constructor( eventId = prepared.request.eventId, moved = prepared.updated, restored = prepared.original, + dropToken = prepared.request.token, ) } else { null diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt index f3bb03e1..33eb4050 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt @@ -305,9 +305,9 @@ data class TimelineDrop( val date: LocalDate, val startMin: Int, /** Minutes from the event's start to the dragged block's top edge. */ - val clipOffsetMin: Int = 0, - /** The drop's identity, which its held copy waits on; set by the controller's finish. */ - val token: Long = 0L, + val clipOffsetMin: Int, + /** The drop's identity, which its held copy waits on. */ + val token: Long, ) /** The id a drafted block's stand-in event carries, which no provider row has (#153). */ @@ -350,7 +350,6 @@ internal fun draftBlock(draft: EventInstance, date: LocalDate, zone: TimeZone): /** Which end of an event a resize handle moves (#141). */ enum class ResizeEdge { Top, Bottom } -/** Where a finished resize asks its event to begin and end. */ /** A drop parked by a newer pickup while its write still runs — see [TimelineDragController.held]. */ data class HeldDrop( val copy: TimelineDrag, @@ -358,18 +357,29 @@ data class HeldDrop( val instanceId: Long, /** The event row and start the drop came from, as [TimelineDragController.ghosts] matches them. */ val origin: Pair, + /** A resize's new end, which has to have landed too before the drop counts as seated. */ + val end: Instant?, + /** Whether the grid now draws the drop at its landing slot. */ + val seated: Boolean = false, val handedOver: Boolean = false, ) { fun ghosts(block: TimedBlock): Boolean = block.event.instanceId == instanceId || (block.event.eventId == origin.first && block.event.start == origin.second) } +/** A strip drop parked by a newer one while its write still runs — see [TimelineDragController.heldStrips]. */ +data class HeldStrip(val drag: StripDrag, val token: Long, val seated: Boolean = false) + +/** An event's new start and end, as a resize leaves them. */ +data class ResizedSpan(val start: Instant, val end: Instant) + +/** Where a finished resize asks its event to begin and end. */ data class TimelineResizeDrop( val event: EventInstance, val start: Instant, val end: Instant, - /** The drop's identity, which its held copy waits on; set by the controller's finishResize. */ - val token: Long = 0L, + /** The drop's identity, which its held copy waits on. */ + val token: Long, ) /** @@ -381,17 +391,16 @@ data class TimelineResizeDrop( * it was drawn (see [repicked]). An end takes a pass the grid draws at the drop * point, so the block settles under the finger. */ -internal fun resizedDrop( +internal fun resizedSpan( event: EventInstance, edge: ResizeEdge?, date: LocalDate, startMin: Int, spanMin: Int, zone: TimeZone, -): TimelineResizeDrop { +): ResizedSpan { val drawn = wallClockSpan(event, zone) - return TimelineResizeDrop( - event = event, + return ResizedSpan( start = if (edge == ResizeEdge.Top) { event.start.toFormTime(zone).repicked( wallClockTime(date, startMin), @@ -692,6 +701,10 @@ class TimelineDragController { var stripSeated: Boolean by mutableStateOf(false) private set + /** Strip drops parked by a newer strip drop while their writes still run (#494). */ + var heldStrips: List by mutableStateOf(emptyList()) + private set + /** The token of the drop [stripSettling] waits on. */ var stripToken: Long = 0L private set @@ -781,10 +794,12 @@ class TimelineDragController { cancel() if (held.overGrid && landed != null && zone != null && !landed.edgeOnly) { val token = settle(landed) - val start = TimelineDrop(landed.event, landed.date, landed.startMin).startInstant(zone) + val start = dropStartInstant(landed.event, landed.date, landed.startMin, clipOffsetMin = 0, zone) return StripDrop.Timed(held.event, start, start + draftMinutes.minutes, token) } if (held.overGrid || held.dayShift == 0) return null + // One still settling keeps its place on the strip until its own write lands (#494). + stripSettling?.let { heldStrips = heldStrips + HeldStrip(it, stripToken, stripSeated) } stripSettling = held stripSeated = false stripToken = nextMoveToken() @@ -803,6 +818,21 @@ class TimelineDragController { if (stripSettling != null) stripSeated = true } + /** Note that the strip draws the parked strip drop [token] at its new days. */ + fun seatHeldStrip(token: Long) { + if (heldStrips.any { it.token == token && !it.seated }) { + heldStrips = heldStrips.map { if (it.token == token) it.copy(seated = true) else it } + } + } + + /** Let the parked strip drop [token] go. */ + fun releaseHeldStrip(token: Long) { + heldStrips = heldStrips.filterNot { it.token == token } + } + + /** Every strip drop still shown at its new days: the parked ones, then the latest. */ + fun stripPreviews(): List = heldStrips.map { it.drag } + listOfNotNull(stripDrag ?: stripSettling) + fun move(pointerInRoot: Offset) { pointer = pointerInRoot if (!pointerMoved && (pointerInRoot - pickupPointer).getDistance() > slopPx) pointerMoved = true @@ -849,7 +879,7 @@ class TimelineDragController { val landed = settling val origin = settledOrigin if (landed != null && origin != null && !landed.event.isDraft) { - held = held + HeldDrop(landed, settledToken, landed.event.instanceId, origin) + held = held + HeldDrop(landed, settledToken, landed.event.instanceId, origin, settledEnd, settledOnGrid) } settling = null settledOnGrid = false @@ -874,16 +904,23 @@ class TimelineDragController { * blank title matches nothing: every untitled event has one (#253). */ fun noteGrid(date: LocalDate, blocks: List) { + if (held.any { !it.seated && drawsDrop(it.copy, it.end, date, blocks) }) { + held = held.map { if (!it.seated && drawsDrop(it.copy, it.end, date, blocks)) it.copy(seated = true) else it } + } val landed = settling ?: return if (settledOnGrid) return + settledOnGrid = drawsDrop(landed, settledEnd, date, blocks) + } + + /** Whether [blocks], [date]'s column, draws the drop [landed] where it landed. */ + private fun drawsDrop(landed: TimelineDrag, end: Instant?, date: LocalDate, blocks: List): Boolean { // Where this day would draw it, which for a drop clipped at midnight is // the day's own start rather than the edge the finger held (#253) — and // which for a tail dragged clear of its day is the day before it. val offset = (date.toEpochDays() - landed.date.toEpochDays()).toInt() val slice = dragSlices(landed.eventStartMin, landed.eventSpanMin, offset..offset) - .firstOrNull() ?: return - val end = settledEnd - settledOnGrid = blocks.any { block -> + .firstOrNull() ?: return false + return blocks.any { block -> block.startMin == slice.startMin && // A resize may keep its start, so its end has to have landed too. (end == null || block.event.end == end) && @@ -985,10 +1022,10 @@ class TimelineDragController { val event = source?.event ?: return null val edge = resize?.edge if (edge != null) { - val drop = resizedDrop(event, edge, date, startMin, spanMin, zone) + val drop = resizedSpan(event, edge, date, startMin, spanMin, zone) return event.copy(start = drop.start, end = drop.end) } - val start = TimelineDrop(event, date, startMin, clipOffsetMin).startInstant(zone) + val start = dropStartInstant(event, date, startMin, clipOffsetMin, zone) return event.copy(start = start, end = start + dragDuration) } @@ -1002,8 +1039,8 @@ class TimelineDragController { val edge = resize?.edge cancel() if (landed == null || landed.slot == origin) return null - val drop = resizedDrop(landed.event, edge, landed.date, landed.eventStartMin, landed.eventSpanMin, zone) - return drop.copy(token = settle(landed, end = drop.end)) + val span = resizedSpan(landed.event, edge, landed.date, landed.eventStartMin, landed.eventSpanMin, zone) + return TimelineResizeDrop(landed.event, span.start, span.end, settle(landed, end = span.end)) } /** @@ -1286,7 +1323,16 @@ fun TimedBlock.continuesBefore(day: LocalDate, zone: TimeZone): Boolean = * visibly settles there. */ fun TimelineDrop.startInstant(zone: TimeZone): Instant = - wallClockInstant(date, startMin - clipOffsetMin, zone, kept = zone.repeatedHourOffsetAt(event.start)) + dropStartInstant(event, date, startMin, clipOffsetMin, zone) + +/** Where [event] starts when its block's top edge lands [startMin] past [date]'s midnight — see [TimelineDrop]. */ +internal fun dropStartInstant( + event: EventInstance, + date: LocalDate, + startMin: Int, + clipOffsetMin: Int, + zone: TimeZone, +): Instant = wallClockInstant(date, startMin - clipOffsetMin, zone, kept = zone.repeatedHourOffsetAt(event.start)) /** * The wall-clock minutes an event lasting [duration] covers when it starts @@ -1358,8 +1404,6 @@ private const val SETTLE_TIMEOUT_MILLIS = 900L */ const val SETTLE_FADE_MILLIS: Int = 250 -/** How long a held copy waits after its write before fading — the grid's re-read, unconfirmed. */ -const val HELD_SETTLE_MILLIS: Long = 300L /** * The floating block, drawn over the whole calendar so it is free of the day @@ -1419,6 +1463,9 @@ fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = controller.held.forEach { parked -> key(parked.token) { HeldTimelineCopy(controller, parked, origin) } } + controller.heldStrips.forEach { parked -> + key(parked.token) { HeldStripSettle(controller, parked) } + } val drag = controller.drag ?: controller.settling ?: return@Box // Landed: the copy sinks back to the grid's own plane while the write runs. val landed = controller.drag == null @@ -1436,30 +1483,41 @@ fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = } } +/** + * [TimelineDragController.ghosts] for [block], derived so the block recomposes + * only when its own answer changes rather than on every lift, park and release. + */ +@Composable +fun TimelineDragController.ghostsDerived(block: TimedBlock): Boolean { + val ghosted by remember(this, block) { derivedStateOf { ghosts(block) } } + return ghosted +} + /** A drop parked by a newer pickup: held at its landing slot until its own write settles. */ @Composable private fun HeldTimelineCopy(controller: TimelineDragController, parked: HeldDrop, origin: Offset) { - val drops = dropStates() - val pending = parked.token in drops.pending - val abandoned = parked.token in drops.abandoned - var fading by remember { mutableStateOf(false) } - LaunchedEffect(pending, abandoned) { - if (pending) return@LaunchedEffect - // No seat to confirm this one, so give the grid a beat to re-read. - if (!abandoned) delay(HELD_SETTLE_MILLIS) - fading = true - controller.handOverHeld(parked.token) - delay(SETTLE_FADE_MILLIS.toLong()) - controller.releaseHeld(parked.token) - } - val alpha by animateFloatAsState( - targetValue = if (fading) 0f else 1f, - animationSpec = tween(SETTLE_FADE_MILLIS), - label = "held-handover", + val alpha = rememberHeldSettle( + token = parked.token, + seated = parked.seated, + timeoutMillis = SETTLE_TIMEOUT_MILLIS, + onHandOver = { controller.handOverHeld(parked.token) }, + onRelease = { controller.releaseHeld(parked.token) }, ) TimelineCopy(parked.copy, controller.geometry, origin, lift = 0f, copyAlpha = alpha) } +/** A strip drop parked by a newer one: shown at its new days until its own write settles. */ +@Composable +private fun HeldStripSettle(controller: TimelineDragController, parked: HeldStrip) { + rememberHeldSettle( + token = parked.token, + seated = parked.seated, + timeoutMillis = SETTLE_TIMEOUT_MILLIS, + onHandOver = {}, + onRelease = { controller.releaseHeldStrip(parked.token) }, + ) +} + /** [drag]'s pieces drawn over the calendar, clipped to the timeline's viewport. */ @Composable private fun TimelineCopy( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthDrag.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthDrag.kt index 004d595a..f1c9d14e 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthDrag.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthDrag.kt @@ -111,6 +111,8 @@ data class HeldChip( val copy: MonthChipDrag, val token: Long, val handedOver: Boolean = false, + /** Where the grid now draws the drop, in root coordinates, once a row seats it. */ + val seat: Offset? = null, ) /** Where a finished chip drag asks its event to go, as a whole-day shift. */ @@ -175,10 +177,11 @@ class MonthDragController { private set /** - * The last drop this controller made, kept past [release] for [beginUndo] — - * the confirmation chip carries Undo long after the copy is gone. + * This controller's recent drops by token, kept past [release] for + * [beginUndo] — the confirmation chip carries Undo long after the copy is + * gone, and it may be any of them the chip offers to undo. */ - private var undoable: MonthChipDrag? = null + private val undoable = LinkedHashMap() /** * Earlier drops still being written when another chip was picked up. They @@ -244,9 +247,19 @@ class MonthDragController { settledGhost = false } - /** Whether [event]'s chip is the source of a held drop, so still a ghost. */ - fun isHeldSource(event: EventInstance): Boolean = - held.any { !it.handedOver && it.copy.event.instanceId == event.instanceId } + /** + * Whether [event]'s chip belongs to a held drop and must stay a ghost: its + * source, or once a row seats it, the chip the grid draws on its target day. + */ + fun isHeldChip(event: EventInstance, days: List?): Boolean = held.any { parked -> + !parked.handedOver && ( + parked.copy.event.instanceId == event.instanceId || + ( + parked.seat != null && days != null && parked.copy.targetDate in days && + isSameEvent(event, parked.copy.event) + ) + ) + } /** Move a still-settling drop to [held] before a new pickup takes the slot. */ private fun parkSettling() { @@ -283,6 +296,13 @@ class MonthDragController { * nearest where the chip was let go is the one the finger was over. */ fun noteSettled(token: Any) { + val row = rows[token] + if (row != null && held.any { it.seat == null }) { + held = held.map { parked -> + val target = parked.copy.targetDate + if (parked.seat != null || target == null) parked else parked.copy(seat = row.seatOf(parked.copy.event, target)) + } + } val landed = settling ?: return val target = landed.targetDate ?: return val at = rows[token]?.seatOf(landed.event, target) ?: return @@ -298,9 +318,9 @@ class MonthDragController { * there is no drop of this controller's left to undo, or the grid doesn't * seat the moved event where the chip would have to start from. */ - fun beginUndo(token: Long): Boolean { - val last = undoable ?: return false - undoable = null + fun beginUndo(token: Long, dropToken: Long): Boolean { + val last = undoable.remove(dropToken) ?: return false + parkSettling() val from = last.targetDate ?: return false val at = rows.values.firstNotNullOfOrNull { it.seatOf(last.event, from) } ?: return false settling = last.copy(grabDate = from, targetDate = last.grabDate, topLeftInRoot = at) @@ -327,8 +347,9 @@ class MonthDragController { ) liftedInstanceId = landed.event.instanceId settledGhost = true - undoable = settling settledToken = nextMoveToken() + undoable[settledToken] = requireNotNull(settling) + if (undoable.size > UNDOABLE_KEPT) undoable.remove(undoable.keys.first()) drops++ return MonthChipDrop(landed.event, landed.grabDate, target, settledToken) } @@ -392,3 +413,6 @@ fun rememberMonthDragController(): MonthDragController { } val LocalMonthDrag = compositionLocalOf { null } + +/** How many recent drops a month controller keeps for an undo to carry back. */ +private const val UNDOABLE_KEPT = 8 diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt index f183c8b0..46b8a0f1 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt @@ -129,7 +129,7 @@ import de.jeanlucmakiola.calendula.ui.common.dropStates import de.jeanlucmakiola.calendula.ui.common.OnUndoStarted import de.jeanlucmakiola.calendula.ui.common.ghostAlpha import de.jeanlucmakiola.calendula.ui.common.DragSnapHaptics -import de.jeanlucmakiola.calendula.ui.common.HELD_SETTLE_MILLIS +import de.jeanlucmakiola.calendula.ui.common.rememberHeldSettle import de.jeanlucmakiola.calendula.ui.common.SETTLE_FADE_MILLIS import de.jeanlucmakiola.calendula.ui.common.SETTLE_GRACE_MILLIS import de.jeanlucmakiola.calendula.ui.common.EventMoveScope @@ -515,7 +515,7 @@ fun MonthScreen( val chipDrag = rememberMonthDragController() // Undo moves the event back, so the chip travels back too. The // signal comes from the write, not from the chip that offers it. - OnUndoStarted { token -> chipDrag.beginUndo(token) } + OnUndoStarted { start -> chipDrag.beginUndo(start.token, start.dropToken) } Box( modifier = Modifier .padding(innerPadding) @@ -667,25 +667,24 @@ private fun HeldMonthChip( origin: Offset, timeFit: MonthChipTimeFit?, ) { - val drops = dropStates() - val pending = parked.token in drops.pending - val abandoned = parked.token in drops.abandoned - var fading by remember { mutableStateOf(false) } - LaunchedEffect(pending, abandoned) { - if (pending) return@LaunchedEffect - // No seat to confirm this one, so give the grid a beat to re-read. - if (!abandoned) delay(HELD_SETTLE_MILLIS) - fading = true - controller.handOverHeld(parked.token) - delay(SETTLE_FADE_MILLIS.toLong()) - controller.releaseHeld(parked.token) + val reduceMotion = rememberReduceMotion() + val glide = remember { Animatable(parked.copy.topLeftInRoot, Offset.VectorConverter) } + val glideSpec = MaterialTheme.motionScheme.fastSpatialSpec() + val seat = parked.seat + val landed = parked.token !in dropStates().pending + // Carried onto its seat once the write is in, as the settling chip is. + LaunchedEffect(seat, landed) { + if (seat == null || !landed) return@LaunchedEffect + if (reduceMotion) glide.snapTo(seat) else glide.animateTo(seat, glideSpec) } - val alpha by animateFloatAsState( - targetValue = if (fading) 0f else 1f, - animationSpec = tween(SETTLE_FADE_MILLIS), - label = "held-chip-handover", + val alpha = rememberHeldSettle( + token = parked.token, + seated = seat != null, + timeoutMillis = MONTH_SETTLE_TIMEOUT_MILLIS, + onHandOver = { controller.handOverHeld(parked.token) }, + onRelease = { controller.releaseHeld(parked.token) }, ) - MonthChipCopy(parked.copy, { parked.copy.topLeftInRoot }, origin, lift = 0f, alpha = alpha, timeFit = timeFit) + MonthChipCopy(parked.copy, { glide.value }, origin, lift = 0f, alpha = alpha, timeFit = timeFit) } /** [drag]'s chip floating over the grid with its top left at [at], in root coordinates. */ @@ -2311,7 +2310,7 @@ private fun MonthWeekRow( // Once the grid holds the settled chip, tell the controller where this row // has seated it, so the copy in flight can glide onto it. Keyed on the week, // which is what changes when the re-read lands. - LaunchedEffect(week, dragController?.settling) { + LaunchedEffect(week, dragController?.settling, dragController?.held) { dragController?.noteSettled(rowToken) } @@ -2744,6 +2743,7 @@ private fun monthChipDragModifier( beginMillis = drop.event.start.toEpochMilliseconds(), endMillis = drop.event.end.toEpochMilliseconds(), target = MoveTarget.ByDays(delta), + isAllDay = drop.event.isAllDay, token = drop.token, ), ) @@ -2890,9 +2890,17 @@ private fun MonthBar( val moveAction = eventMoveAction(event) // The source stays put as a ghost while its floating copy travels. val monthDrag = LocalMonthDrag.current - val lifted = monthDrag?.liftedInstanceId == event.instanceId || - monthDrag?.isSettledChip(event, days) == true || - monthDrag?.isHeldSource(event) == true + // Derived, so a chip recomposes only when its own answer changes rather than + // on every lift, park and release anywhere in the grid. + val lifted by remember(monthDrag, event, days) { + derivedStateOf { + monthDrag != null && ( + monthDrag.liftedInstanceId == event.instanceId || + monthDrag.isSettledChip(event, days) || + monthDrag.isHeldChip(event, days) + ) + } + } val ghost = ghostAlpha(lifted) val shape = monthBarShape(continuesLeft, continuesRight) Box( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt index f79e4914..b89a92d8 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt @@ -157,6 +157,7 @@ import de.jeanlucmakiola.calendula.ui.common.LocalEventMove import de.jeanlucmakiola.calendula.ui.common.MoveRequest import de.jeanlucmakiola.calendula.ui.common.MoveTarget import de.jeanlucmakiola.calendula.ui.common.TimelineDragController +import de.jeanlucmakiola.calendula.ui.common.ghostsDerived import de.jeanlucmakiola.calendula.ui.common.StripDrag import de.jeanlucmakiola.calendula.ui.common.StripDrop import de.jeanlucmakiola.calendula.ui.common.TimelineDragOverlay @@ -591,6 +592,7 @@ private fun WeekContent( eventId = drop.event.eventId, beginMillis = drop.event.start.toEpochMilliseconds(), endMillis = drop.event.end.toEpochMilliseconds(), + isAllDay = drop.event.isAllDay, target = MoveTarget.Start(start), token = drop.token, ), @@ -611,6 +613,7 @@ private fun WeekContent( eventId = drop.event.eventId, beginMillis = drop.event.start.toEpochMilliseconds(), endMillis = drop.event.end.toEpochMilliseconds(), + isAllDay = drop.event.isAllDay, target = MoveTarget.Span(drop.start, drop.end), token = drop.token, ), @@ -624,6 +627,7 @@ private fun WeekContent( eventId = drop.event.eventId, beginMillis = drop.event.start.toEpochMilliseconds(), endMillis = drop.event.end.toEpochMilliseconds(), + isAllDay = drop.event.isAllDay, target = when (drop) { is StripDrop.Days -> MoveTarget.ByDays(drop.days) is StripDrop.Timed -> MoveTarget.Span(drop.start, drop.end) @@ -770,8 +774,7 @@ private fun WeekPager( // Until its days arrive a page shows their frame without events. val empty = remember(pageSpan, today) { emptySpan(pageSpan, today) } // A bar held over other days in the strip makes room there before it is dropped. - val held = (dragController.stripDrag ?: dragController.stripSettling) - ?.takeIf { page == pagerState.currentPage } + val held = dragController.stripPreviews().takeIf { page == pagerState.currentPage }.orEmpty() val zone = LocalTimelineZone.current val shown = remember(loaded, held, zone) { loaded?.withStripPreview(held, zone) } val stripHeight = shown?.allDayStripHeight() @@ -796,17 +799,24 @@ private fun WeekPager( } /** - * [this] with the strip bar [held] sideways laid out at the days it is over, so - * the bars there move aside live. The bar keeps its own event, and with it its - * key and the gesture on it; one moved off the page keeps a lane it is not seen in. + * [this] with the strip bars [held] sideways laid out at the days they are over, + * so the bars there move aside live. Each bar keeps its own event, and with it + * its key and the gesture on it; one moved off the page keeps a lane it is not + * seen in. */ -internal fun WeekUiState.Success.withStripPreview(held: StripDrag?, zone: TimeZone): WeekUiState.Success { - if (held == null || held.overGrid || held.dayShift == 0) return this - val id = held.event.instanceId - val original = allDaySpans.firstOrNull { it.event.instanceId == id } ?: return this - val events = allDaySpans.map { if (it.event.instanceId == id) it.event.shiftedDays(held.dayShift, zone) else it.event } - val spans = layoutAllDay(events, days, zone).map { if (it.event.instanceId == id) it.copy(event = original.event) else it } - return copy(allDaySpans = if (spans.any { it.event.instanceId == id }) spans else spans + original.copy(lane = 0)) +internal fun WeekUiState.Success.withStripPreview(held: List, zone: TimeZone): WeekUiState.Success { + val shifts = held.filter { !it.overGrid && it.dayShift != 0 }.associate { it.event.instanceId to it.dayShift } + if (shifts.isEmpty()) return this + val originals = allDaySpans.filter { it.event.instanceId in shifts }.associateBy { it.event.instanceId } + if (originals.isEmpty()) return this + val events = allDaySpans.map { span -> + shifts[span.event.instanceId]?.let { span.event.shiftedDays(it, zone) } ?: span.event + } + val spans = layoutAllDay(events, days, zone).map { span -> + originals[span.event.instanceId]?.let { span.copy(event = it.event) } ?: span + } + val laidOut = spans.mapTo(HashSet()) { it.event.instanceId } + return copy(allDaySpans = spans + originals.values.filter { it.event.instanceId !in laidOut }.map { it.copy(lane = 0) }) } private fun EventInstance.shiftedDays(days: Int, zone: TimeZone): EventInstance { @@ -814,6 +824,17 @@ private fun EventInstance.shiftedDays(days: Int, zone: TimeZone): EventInstance return copy(start = start.plus(days, DateTimeUnit.DAY, dates), end = end.plus(days, DateTimeUnit.DAY, dates)) } +/** Whether these bars draw the strip drop [drop] at the days it was dropped on. */ +private fun List.drawStripDrop(drop: StripDrag, zone: TimeZone): Boolean { + val target = drop.event.spanFirstDay(zone).plus(drop.dayShift, DateTimeUnit.DAY) + return any { bar -> + bar.event.instanceId != drop.event.instanceId && + (bar.event.eventId == drop.event.eventId || + (drop.event.title.isNotBlank() && bar.event.title == drop.event.title)) && + bar.event.spanFirstDay(zone) == target + } +} + /** A page with no events yet, for one whose days are still loading. */ private fun emptySpan(span: DaySpan, today: LocalDate): WeekUiState.Success { val days = (0 until span.count).map { span.start.plus(it, kotlinx.datetime.DateTimeUnit.DAY) } @@ -1196,17 +1217,11 @@ private fun AllDayStrip( val sideways = held?.takeIf { !it.overGrid } val lifted = dragController.liftedInstanceId?.takeIf { active && dragController.stripDrag?.overGrid != false } val settling = dragController.stripSettling?.takeIf { active } + val heldStrips = dragController.heldStrips.takeIf { active }.orEmpty() // Seated once the re-read draws the event at the days it was dropped on. - LaunchedEffect(settling, bars) { - if (settling == null) return@LaunchedEffect - val target = settling.event.spanFirstDay(zone).plus(settling.dayShift, DateTimeUnit.DAY) - val landed = bars.any { bar -> - bar.event.instanceId != settling.event.instanceId && - (bar.event.eventId == settling.event.eventId || - (settling.event.title.isNotBlank() && bar.event.title == settling.event.title)) && - bar.event.spanFirstDay(zone) == target - } - if (landed) dragController.seatStrip() + LaunchedEffect(settling, heldStrips, bars) { + if (settling != null && bars.drawStripDrop(settling, zone)) dragController.seatStrip() + heldStrips.forEach { if (!it.seated && bars.drawStripDrop(it.drag, zone)) dragController.seatHeldStrip(it.token) } } Row( @@ -1591,7 +1606,7 @@ private fun DayColumnCard( widthFraction = 1f / block.laneCount, y = top, height = height, - landing = dragController.ghosts(block), + landing = dragController.ghostsDerived(block), ) val topInset = hourCellBlockInset(showHourGrid, cuts.top) val bottomInset = hourCellBlockInset(showHourGrid, cuts.bottom) @@ -1759,7 +1774,7 @@ private fun EventBlock( onCancel = dragController::cancel, onRelease = { dragController.select(block.event, fromLift = draggable) }, ) - val lifted = draggable && dragController.ghosts(block) + val lifted = draggable && dragController.ghostsDerived(block) // Released in place, the block takes over the copy's lift without a dip. val takesOverLift = selected && dragController.selection?.fromLift == true val ghostFade = ghostAlpha(lifted) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 5d3a342d..278cc7ca 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -111,6 +111,7 @@ Calendula needs write access to move events That event no longer exists That event moved before your drop could be saved + Too many moves at once, so that one wasn\'t saved Moves made while the question was open weren\'t saved Can\'t move an event past the end of its series A cancelled series can only move as a whole, and this would change which days it falls on diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt index 5085188f..86215696 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/RescheduleViewModelTest.kt @@ -127,6 +127,7 @@ class RescheduleViewModelTest { beginMillis = beginMillis, endMillis = endMillis, target = MoveTarget.ByDays(2), + isAllDay = false, token = nextMoveToken(), ) @@ -136,6 +137,7 @@ class RescheduleViewModelTest { beginMillis = beginMillis, endMillis = endMillis, target = MoveTarget.Start(Instant.fromEpochMilliseconds(beginMillis + 3_600_000L)), + isAllDay = false, token = nextMoveToken(), ) @@ -148,6 +150,7 @@ class RescheduleViewModelTest { Instant.fromEpochMilliseconds(beginMillis), Instant.fromEpochMilliseconds(endMillis + 3_600_000L), ), + isAllDay = false, token = nextMoveToken(), ) @@ -368,35 +371,52 @@ class RescheduleViewModelTest { } @Test - fun `a recurring occurrence that moved since the drop is not written`( + fun `a drop on a series written moments ago is not written`( @TempDir tempDir: Path, ) = runTest(dispatcher) { - val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail(rrule = "FREQ=WEEKLY") } } - val vm = viewModel(tempDir, fake, present = emptyList()) + // A whole-series write shifts every occurrence; the grid still draws the + // old ones, and another occurrence may now sit at the dragged time. + val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail(rrule = "FREQ=DAILY") } } + val vm = viewModel(tempDir, fake) + vm.move(oneHourLater()) + advanceUntilIdle() + vm.moveWithScope(RecurringWriteScope.AllEvents) + advanceUntilIdle() + vm.consumeOutcome() - val drop = toWednesday() - vm.move(drop) + val again = toWednesday() + vm.move(again) advanceUntilIdle() assertThat(vm.scopePrompt.value).isNull() assertThat(vm.outcome.value).isEqualTo(MoveOutcome.Stale) - assertThat(vm.drops.value.abandoned).containsExactly(drop.token) - assertThat(fake.updatedEvents).isEmpty() + assertThat(vm.drops.value.abandoned).containsExactly(again.token) + assertThat(fake.updatedEvents).hasSize(1) } @Test - fun `a deleted event reports itself as gone, not moved`(@TempDir tempDir: Path) = runTest(dispatcher) { - val fake = FakeCalendarDataSource().apply { eventDetailResult = { null } } - val vm = viewModel(tempDir, fake, present = emptyList()) - + fun `a single occurrence written leaves the rest of its series movable`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail(rrule = "FREQ=WEEKLY") } } + val vm = viewModel(tempDir, fake) vm.move(oneHourLater()) advanceUntilIdle() + vm.moveWithScope(RecurringWriteScope.ThisEvent) + advanceUntilIdle() - assertThat(vm.outcome.value).isEqualTo(MoveOutcome.Gone) + // Another occurrence, a week on. + val week = 7 * 86_400_000L + vm.move(oneHourLater().copy(beginMillis = beginMillis + week, endMillis = endMillis + week, token = nextMoveToken())) + advanceUntilIdle() + + assertThat(vm.scopePrompt.value).isNotNull() } @Test - fun `a failure stays on the chip while later drops land`(@TempDir tempDir: Path) = runTest(dispatcher) { + fun `chip messages wait their turn instead of replacing each other`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { val fake = FakeCalendarDataSource().apply { eventDetailResult = { id -> if (id == 42L) detail(rrule = "FREQ=WEEKLY;BYDAY=MO,WE", status = EventStatus.Cancelled) else detail() @@ -408,8 +428,35 @@ class RescheduleViewModelTest { vm.move(toWednesday().copy(eventId = 7L, token = nextMoveToken())) advanceUntilIdle() - assertThat(fake.updatedEvents.map { it.first }).containsExactly(7L) assertThat(vm.outcome.value).isEqualTo(MoveOutcome.BlockedCancelledSeries) + vm.consumeOutcome() + assertThat((vm.outcome.value as MoveOutcome.Moved).undo?.eventId).isEqualTo(7L) + vm.consumeOutcome() + assertThat(vm.outcome.value).isNull() + } + + @Test + fun `a full queue says so`(@TempDir tempDir: Path) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail() } } + val vm = slowViewModel(tempDir, fake) + + repeat(9) { i -> vm.move(toWednesday().copy(eventId = 100L + i, token = nextMoveToken())) } + val refused = toWednesday().copy(eventId = 200L, token = nextMoveToken()) + assertThat(vm.move(refused)).isFalse() + + assertThat(vm.drops.value.abandoned).containsExactly(refused.token) + assertThat(vm.outcome.value).isEqualTo(MoveOutcome.QueueFull) + } + + @Test + fun `a deleted event reports itself as gone, not moved`(@TempDir tempDir: Path) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { eventDetailResult = { null } } + val vm = viewModel(tempDir, fake, present = emptyList()) + + vm.move(oneHourLater()) + advanceUntilIdle() + + assertThat(vm.outcome.value).isEqualTo(MoveOutcome.Gone) } @Test @@ -437,8 +484,10 @@ class RescheduleViewModelTest { vm.moveWithScope(RecurringWriteScope.AllEvents) advanceUntilIdle() assertThat(fake.updatedEvents.map { it.first }).containsExactly(42L) - // The skip stays on the chip; the grid shows the move itself. + // The skip has its turn, then the move it waited on, with its Undo. assertThat(vm.outcome.value).isEqualTo(MoveOutcome.Skipped) + vm.consumeOutcome() + assertThat((vm.outcome.value as MoveOutcome.Moved).undo?.eventId).isEqualTo(42L) } @Test @@ -478,9 +527,9 @@ class RescheduleViewModelTest { advanceUntilIdle() val undo = (vm.outcome.value as MoveOutcome.Moved).undo!! - // A second drop parks on its scope dialog, which holds the pipeline. + // A second drop, of another event, parks on its scope dialog, which holds the pipeline. fake.eventDetailResult = { detail(rrule = "FREQ=WEEKLY;BYDAY=MO") } - vm.move(toWednesday()) + vm.move(toWednesday().copy(eventId = 7L, token = nextMoveToken())) advanceUntilIdle() val abandoned = vm.abandonedCount @@ -609,6 +658,7 @@ class RescheduleViewModelTest { target = MoveTarget.Start( Instant.fromEpochMilliseconds(beginMillis + 26 * 3_600_000L), ), + isAllDay = false, token = nextMoveToken(), ), ) @@ -630,6 +680,7 @@ class RescheduleViewModelTest { beginMillis = beginMillis, endMillis = endMillis, target = MoveTarget.Start(Instant.fromEpochMilliseconds(beginMillis)), + isAllDay = false, token = nextMoveToken(), ), ) @@ -752,6 +803,7 @@ class RescheduleViewModelTest { beginMillis = beginMillis, endMillis = endMillis, target = MoveTarget.Start(Instant.fromEpochMilliseconds(beginMillis)), + isAllDay = false, token = nextMoveToken(), ), )).isTrue() @@ -865,6 +917,7 @@ class RescheduleViewModelTest { beginMillis = beginMillis, endMillis = endMillis, target = MoveTarget.Span(sundayEvening, Instant.fromEpochMilliseconds(endMillis)), + isAllDay = false, token = nextMoveToken(), ), ) @@ -937,6 +990,7 @@ class RescheduleViewModelTest { beginMillis = dayStart, endMillis = dayStart + 86_400_000L, target = MoveTarget.Span(start, Instant.fromEpochMilliseconds(endMillis)), + isAllDay = true, token = nextMoveToken(), ), ) @@ -974,6 +1028,7 @@ class RescheduleViewModelTest { beginMillis = dayStart, endMillis = dayStart + 86_400_000L, target = MoveTarget.Span(eight, eight + 1.hours), + isAllDay = true, token = nextMoveToken(), ), ) diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineDropTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineDropTest.kt index 83a4c78e..c6b7fba2 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineDropTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineDropTest.kt @@ -53,7 +53,7 @@ class TimelineDropTest { @Test fun `dropping the head asks for exactly where it landed`() { val target = LocalDate(2026, 9, 2) - val drop = TimelineDrop(overnight, target, 20 * 60, clipOffsetMin = 0) + val drop = TimelineDrop(overnight, target, 20 * 60, clipOffsetMin = 0, token = 0L) assertThat(drop.startInstant(zone)).isEqualTo(target.atTime(20, 0).toInstant(zone)) } @@ -63,7 +63,7 @@ class TimelineDropTest { val tail = blockOn(sun) val offset = tail.clipOffsetMinutes(sun, zone) // Dragged three days back, same height in the column: Sun 6 -> Thu 3. - val drop = TimelineDrop(overnight, LocalDate(2026, 9, 3), 0, offset) + val drop = TimelineDrop(overnight, LocalDate(2026, 9, 3), 0, offset, token = 0L) // The event began Sat 5 Sep 20:00, so it now begins Wed 2 Sep 20:00 — // the whole event moved three days, and no time of day was invented. @@ -75,7 +75,7 @@ class TimelineDropTest { fun `dragging the tail down the column re-times the event across midnight`() { val offset = blockOn(sun).clipOffsetMinutes(sun, zone) // Tail top pulled from 00:00 to 03:00 on its own day. - val drop = TimelineDrop(overnight, sun, 3 * 60, offset) + val drop = TimelineDrop(overnight, sun, 3 * 60, offset, token = 0L) assertThat(drop.startInstant(zone)).isEqualTo(sat.atTime(23, 0).toInstant(zone)) } @@ -84,7 +84,7 @@ class TimelineDropTest { fun `pulling the tail above its own midnight moves the event earlier`() { val offset = blockOn(sun).clipOffsetMinutes(sun, zone) // The tail's top edge dragged an hour above the midnight it rests on. - val drop = TimelineDrop(overnight, sun, -60, offset) + val drop = TimelineDrop(overnight, sun, -60, offset, token = 0L) // Event was Sat 20:00 to Sun 08:00; it now begins an hour earlier. assertThat(drop.startInstant(zone)).isEqualTo(sat.atTime(19, 0).toInstant(zone)) @@ -92,7 +92,7 @@ class TimelineDropTest { @Test fun `a top edge dragged past the next midnight resolves onto the day after`() { - val drop = TimelineDrop(overnight, sat, 25 * 60, clipOffsetMin = 0) + val drop = TimelineDrop(overnight, sat, 25 * 60, clipOffsetMin = 0, token = 0L) assertThat(drop.startInstant(zone)).isEqualTo(sun.atTime(1, 0).toInstant(zone)) } @@ -130,7 +130,7 @@ class TimelineDropTest { val tail = layoutDay(listOf(overnightIntoDst), sun, berlin).single() val offset = tail.clipOffsetMinutes(sun, berlin) // Dropped back onto its own slot a week earlier, clear of the boundary. - val drop = TimelineDrop(overnightIntoDst, LocalDate(2026, 3, 22), 0, offset) + val drop = TimelineDrop(overnightIntoDst, LocalDate(2026, 3, 22), 0, offset, token = 0L) assertThat(drop.startInstant(berlin)) .isEqualTo(LocalDate(2026, 3, 21).atTime(22, 0).toInstant(berlin)) @@ -155,7 +155,7 @@ class TimelineDropTest { ) val block = layoutDay(listOf(event), fallBack, brussels).single() assertThat(block.clipOffsetMinutes(fallBack, brussels)).isEqualTo(0) - val drop = TimelineDrop(event, fallBack, block.startMin, clipOffsetMin = 0) + val drop = TimelineDrop(event, fallBack, block.startMin, clipOffsetMin = 0, token = 0L) assertThat(drop.startInstant(brussels)).isEqualTo(event.start) } @@ -168,7 +168,7 @@ class TimelineDropTest { val tail = layoutDay(listOf(event), fallBack, brussels).single() val offset = tail.clipOffsetMinutes(fallBack, brussels) assertThat(offset).isEqualTo(2 * 60) - val drop = TimelineDrop(event, fallBack, tail.startMin, clipOffsetMin = offset) + val drop = TimelineDrop(event, fallBack, tail.startMin, clipOffsetMin = offset, token = 0L) assertThat(drop.startInstant(brussels)).isEqualTo(event.start) } @@ -189,7 +189,7 @@ class TimelineDropTest { end = Instant.parse("2026-10-25T02:30:00Z"), ) val block = layoutDay(listOf(event), fallBack, brussels).single() - val drop = TimelineDrop(event, fallBack, block.startMin + 15, clipOffsetMin = 0) + val drop = TimelineDrop(event, fallBack, block.startMin + 15, clipOffsetMin = 0, token = 0L) assertThat(drop.startInstant(brussels)).isEqualTo(Instant.parse("2026-10-25T01:45:00Z")) } @@ -234,7 +234,7 @@ class TimelineDropTest { start = Instant.parse("2026-10-25T01:30:00Z"), end = Instant.parse("2026-10-25T03:00:00Z"), ) - val drop = resizedDrop(event, ResizeEdge.Bottom, fallBack, 2 * 60 + 30, 15, brussels) + val drop = resizedSpan(event, ResizeEdge.Bottom, fallBack, 2 * 60 + 30, 15, brussels) assertThat(drop.start).isEqualTo(event.start) assertThat(drop.end).isEqualTo(Instant.parse("2026-10-25T01:45:00Z")) } @@ -246,7 +246,7 @@ class TimelineDropTest { start = Instant.parse("2026-10-25T00:30:00Z"), end = Instant.parse("2026-10-25T01:15:00Z"), ) - val drop = resizedDrop(event, ResizeEdge.Top, fallBack, 2 * 60, 60, brussels) + val drop = resizedSpan(event, ResizeEdge.Top, fallBack, 2 * 60, 60, brussels) assertThat(drop.start).isEqualTo(Instant.parse("2026-10-25T00:00:00Z")) assertThat(drop.end).isEqualTo(event.end) } @@ -258,7 +258,7 @@ class TimelineDropTest { start = Instant.parse("2026-10-25T00:20:00Z"), end = Instant.parse("2026-10-25T01:10:00Z"), ) - val drop = resizedDrop(event, ResizeEdge.Bottom, fallBack, 2 * 60 + 20, 30, brussels) + val drop = resizedSpan(event, ResizeEdge.Bottom, fallBack, 2 * 60 + 20, 30, brussels) assertThat(drop.end).isEqualTo(Instant.parse("2026-10-25T00:50:00Z")) } @@ -270,7 +270,7 @@ class TimelineDropTest { start = Instant.parse("2026-10-24T23:00:00Z"), end = Instant.parse("2026-10-25T03:00:00Z"), ) - val drop = resizedDrop(event, ResizeEdge.Bottom, fallBack, 60, 75, brussels) + val drop = resizedSpan(event, ResizeEdge.Bottom, fallBack, 60, 75, brussels) assertThat(drop.end).isEqualTo(Instant.parse("2026-10-25T00:15:00Z")) } } diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/week/WeekLayoutTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/week/WeekLayoutTest.kt index 7b61872f..96f78bdd 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/week/WeekLayoutTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/week/WeekLayoutTest.kt @@ -356,12 +356,12 @@ class WeekLayoutTest { ) assertThat(state.allDaySpans.map { it.lane }).containsExactly(0, 0) - val preview = state.withStripPreview(StripDrag(held, dayShift = 2), zone).allDaySpans + val preview = state.withStripPreview(listOf(StripDrag(held, dayShift = 2)), zone).allDaySpans val moved = preview.single { it.event.instanceId == held.instanceId } assertThat(moved.event).isEqualTo(held) assertThat(moved.startCol).isEqualTo(2) assertThat(preview.map { it.lane }).containsExactly(0, 1) - assertThat(state.withStripPreview(StripDrag(held, dayShift = 2, overGrid = true), zone)).isSameInstanceAs(state) + assertThat(state.withStripPreview(listOf(StripDrag(held, dayShift = 2, overGrid = true)), zone)).isSameInstanceAs(state) } }