Fix the drag review's findings (#68)

- a zero-day month drop no longer overwrites the pending undo record
- the chip's dismissal timer restarts on undo and waits out any write
- a refused drop releases its held copy instead of timing out
- the drag preview uses the event's real length, not the day-clipped block
- block placement stops springing while a pinch rewrites the hour height
- a time-only drag checks the series anchor stays on its own day
This commit is contained in:
2026-08-02 20:17:50 +02:00
parent 916f64d2df
commit 097b494c4a
11 changed files with 173 additions and 37 deletions

View File

@@ -52,11 +52,17 @@ data class BlockPlacement(val x: Dp, val y: Dp, val width: Dp, val height: Dp)
* A timed block's placement, tweened rather than jumped. Continuity comes from * A timed block's placement, tweened rather than jumped. Continuity comes from
* the caller keying each block by identity; a block composed for the first time * the caller keying each block by identity; a block composed for the first time
* starts at its target, so nothing flies in on the first frame. * starts at its target, so nothing flies in on the first frame.
*
* A pinch-zoom rewrites the hour height every pointer frame, and the gutter and
* grid lines follow it instantly — so the tween stands down for the gesture
* rather than leaving the blocks trailing the ruler they are measured against.
*/ */
@OptIn(ExperimentalMaterial3ExpressiveApi::class) @OptIn(ExperimentalMaterial3ExpressiveApi::class)
@Composable @Composable
fun animatedBlockPlacement(x: Dp, y: Dp, width: Dp, height: Dp): BlockPlacement { fun animatedBlockPlacement(x: Dp, y: Dp, width: Dp, height: Dp): BlockPlacement {
val spec: FiniteAnimationSpec<Dp> = if (rememberReduceMotion()) { val spec: FiniteAnimationSpec<Dp> = if (rememberReduceMotion() ||
LocalTimelineZoom.current.isPinching
) {
snap() snap()
} else { } else {
MaterialTheme.motionScheme.fastSpatialSpec() MaterialTheme.motionScheme.fastSpatialSpec()

View File

@@ -51,6 +51,8 @@ private val FAB_BAND = 88.dp
fun EventMoveHost(viewModel: RescheduleViewModel, modifier: Modifier = Modifier) { fun EventMoveHost(viewModel: RescheduleViewModel, modifier: Modifier = Modifier) {
val prompt by viewModel.scopePrompt.collectAsStateWithLifecycle() val prompt by viewModel.scopePrompt.collectAsStateWithLifecycle()
val outcome by viewModel.outcome.collectAsStateWithLifecycle() val outcome by viewModel.outcome.collectAsStateWithLifecycle()
val undoTick by viewModel.undoStarted.collectAsStateWithLifecycle()
val writeInFlight by viewModel.inFlight.collectAsStateWithLifecycle()
val locale = currentLocale() val locale = currentLocale()
val use24Hour = LocalUse24HourFormat.current val use24Hour = LocalUse24HourFormat.current
@@ -85,8 +87,12 @@ fun EventMoveHost(viewModel: RescheduleViewModel, modifier: Modifier = Modifier)
shown.value = ChipContent(message, moved?.undo) shown.value = ChipContent(message, moved?.undo)
} }
val content = shown.value val content = shown.value
LaunchedEffect(outcome) { // Restarted by the undo tick and held while a write is in flight: an undo
if (outcome == null) return@LaunchedEffect // tapped in the last moments of the window leaves the outcome at Moved on
// purpose, and this timer would otherwise fire mid-undo and close the chip
// just before the same chip has to say "undone".
LaunchedEffect(outcome, undoTick, writeInFlight) {
if (outcome == null || writeInFlight) return@LaunchedEffect
delay(if (outcome == MoveOutcome.Undone) UNDONE_CHIP_MILLIS else CHIP_MILLIS) delay(if (outcome == MoveOutcome.Undone) UNDONE_CHIP_MILLIS else CHIP_MILLIS)
viewModel.consumeOutcome() viewModel.consumeOutcome()
} }
@@ -103,13 +109,15 @@ fun EventMoveHost(viewModel: RescheduleViewModel, modifier: Modifier = Modifier)
.height(SnackChipHeight), .height(SnackChipHeight),
contentAlignment = Alignment.CenterStart, contentAlignment = Alignment.CenterStart,
) { ) {
// The action goes away while a write runs: undo() refuses a second
// write anyway, so offering it would be a button that does nothing.
val undo = content.undo?.takeIf { !writeInFlight }
SnackChip( SnackChip(
visible = outcome != null, visible = outcome != null,
message = content.message, message = content.message,
maxWidth = chipMaxWidth, maxWidth = chipMaxWidth,
actionLabel = stringResource(R.string.event_move_undo) actionLabel = stringResource(R.string.event_move_undo).takeIf { undo != null },
.takeIf { content.undo != null }, onAction = undo?.let { { viewModel.undo(it) } },
onAction = content.undo?.let { undo -> { viewModel.undo(undo) } },
) )
} }
} }

View File

@@ -29,7 +29,8 @@ class EventMoveScope(
* the repository writes whatever it is handed — so this gate is load-bearing. * the repository writes whatever it is handed — so this gate is load-bearing.
*/ */
val movableCalendarIds: Set<Long>, val movableCalendarIds: Set<Long>,
val move: (MoveRequest) -> Unit, /** False when the drop was refused outright, so nothing will be written. */
val move: (MoveRequest) -> Boolean,
/** /**
* True while a dropped event is being written, including the time its scope * 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 * dialog is up. A flow rather than a value so this scope stays the same

View File

@@ -40,6 +40,9 @@ import kotlin.coroutines.cancellation.CancellationException
import kotlin.time.Instant import kotlin.time.Instant
import javax.inject.Inject import javax.inject.Inject
private const val MILLIS_PER_MINUTE = 60_000
private const val MINUTES_PER_DAY = 24 * 60
/** Where a dragged event should land. */ /** Where a dragged event should land. */
sealed interface MoveTarget { sealed interface MoveTarget {
/** A new start instant — a timeline drag, which moves time and day at once. */ /** A new start instant — a timeline drag, which moves time and day at once. */
@@ -182,8 +185,13 @@ class RescheduleViewModel @Inject constructor(
val newAnchorDate: LocalDate, val newAnchorDate: LocalDate,
) )
fun move(request: MoveRequest) { /**
if (busy || _scopePrompt.value != null) return * Take a drop, unless one is already being written. False means nothing will
* be written and the caller must let its held copy go now — waiting on
* [inFlight] would strand it until the settle timeout instead.
*/
fun move(request: MoveRequest): Boolean {
if (busy || _scopePrompt.value != null) return false
busy = true busy = true
viewModelScope.launch { viewModelScope.launch {
val prepared = prepare(request) val prepared = prepare(request)
@@ -200,6 +208,7 @@ class RescheduleViewModel @Inject constructor(
_scopePrompt.value = MoveScopePrompt(occurrenceOnly = !prepared.canRealign) _scopePrompt.value = MoveScopePrompt(occurrenceOnly = !prepared.canRealign)
} }
} }
return true
} }
/** Answer the scope dialog. */ /** Answer the scope dialog. */
@@ -225,10 +234,11 @@ class RescheduleViewModel @Inject constructor(
/** /**
* Put a completed move back where it came from. The outcome stands until the * 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 * inverse write reports back, so the one chip changes what it says rather
* than closing and reopening. * than closing and reopening. False when another write is already running —
* the chip hides its action for that window, so this is the backstop.
*/ */
fun undo(undo: MoveUndo) { fun undo(undo: MoveUndo): Boolean {
if (busy) return if (busy) return false
busy = true busy = true
_undoStarted.value += 1 _undoStarted.value += 1
viewModelScope.launch { viewModelScope.launch {
@@ -246,6 +256,7 @@ class RescheduleViewModel @Inject constructor(
} }
busy = false busy = false
} }
return true
} }
/** Clear the outcome once the screen has shown it. */ /** Clear the outcome once the screen has shown it. */
@@ -309,7 +320,11 @@ class RescheduleViewModel @Inject constructor(
// then is the single occurrence, whose exception row carries no rule. // then is the single occurrence, whose exception row carries no rule.
updated = shifted.copy(rrule = realigned ?: original.rrule), updated = shifted.copy(rrule = realigned ?: original.rrule),
isRecurring = isRecurring, isRecurring = isRecurring,
canRealign = !isRecurring || !movedDay || realigned != null, canRealign = when {
!isRecurring -> true
movedDay -> realigned != null
else -> anchorKeepsItsDay(detail, original, shifted, zone)
},
// Where the series row's own DTSTART lands, given the anchor moves by // Where the series row's own DTSTART lands, given the anchor moves by
// the same shift — only meaningful under wholeDayShift. // the same shift — only meaningful under wholeDayShift.
newAnchorDate = anchorDate(detail, original, zone) newAnchorDate = anchorDate(detail, original, zone)
@@ -377,6 +392,29 @@ class RescheduleViewModel @Inject constructor(
return end.date < firstDay return end.date < firstDay
} }
/**
* Whether a same-date drag leaves the series anchor on its own day too. The
* anchor moves by the same wall-clock delta as the occurrence, and normally
* shares its time of day — but a row with no `EVENT_TIMEZONE` resolves the
* two in zones that can sit a DST hour apart, so a near-midnight drag could
* carry the anchor across a midnight the occurrence never crossed and leave
* `BYDAY` naming the wrong weekday.
*/
private fun anchorKeepsItsDay(
detail: EventDetail,
original: EventForm,
shifted: EventForm,
zone: TimeZone,
): Boolean {
val anchorZone = if (original.isAllDay) TimeZone.UTC else original.resolvedZone(zone)
val anchorMinute = detail.instance.start.toLocalDateTime(anchorZone)
.time.toMillisecondOfDay() / MILLIS_PER_MINUTE
val delta = (
shifted.start.time.toMillisecondOfDay() - original.start.time.toMillisecondOfDay()
) / MILLIS_PER_MINUTE
return anchorMinute + delta in 0 until MINUTES_PER_DAY
}
/** /**
* The series row's own start date — for a recurring master, `EventDetail` * The series row's own start date — for a recurring master, `EventDetail`
* carries the row's DTSTART rather than the tapped occurrence's. Read in the * carries the row's DTSTART rather than the tapped occurrence's. Read in the

View File

@@ -273,7 +273,9 @@ class TimelineDragController {
val snapped = (rawMinutes / DRAG_SNAP_MINUTES).roundToInt() * DRAG_SNAP_MINUTES val snapped = (rawMinutes / DRAG_SNAP_MINUTES).roundToInt() * DRAG_SNAP_MINUTES
// Clamp the start into the target day; a tail past midnight is fine. // Clamp the start into the target day; a tail past midnight is fine.
val startMin = snapped.coerceIn(0, MINUTES_PER_DAY - DRAG_SNAP_MINUTES) val startMin = snapped.coerceIn(0, MINUTES_PER_DAY - DRAG_SNAP_MINUTES)
val span = block.endMin - block.startMin // The event's own length, not the block's: TimedBlock.endMin is clipped
// at midnight, which would draw a 22:0002:00 event as a two-hour copy.
val span = (block.event.end - block.event.start).inWholeMinutes.toInt().coerceAtLeast(0)
// The column the finger is over on screen, and the day that column shows. // The column the finger is over on screen, and the day that column shows.
val column = ((pointer.x - origin.x) / columnPx).toInt().coerceIn(0, days.lastIndex) val column = ((pointer.x - origin.x) / columnPx).toInt().coerceIn(0, days.lastIndex)
val dayIndex = if (geometry.isRtl) days.lastIndex - column else column val dayIndex = if (geometry.isRtl) days.lastIndex - column else column
@@ -426,8 +428,15 @@ fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier =
) )
val fill = eventFill(drag.event.color, dark, soften) val fill = eventFill(drag.event.color, dark, soften)
val title = drag.event.title.ifBlank { stringResource(R.string.event_untitled) } val title = drag.event.title.ifBlank { stringResource(R.string.event_untitled) }
// An end past midnight wraps rather than saturating, so a four-hour
// event dragged to 22:00 reads "22:0002:00" and not "22:0024:00".
val endMin = if (drag.endMin > MINUTES_PER_DAY) {
drag.endMin % MINUTES_PER_DAY
} else {
drag.endMin
}
val label = "${formatMinuteOfDay(drag.startMin, use24Hour, locale)}" + val label = "${formatMinuteOfDay(drag.startMin, use24Hour, locale)}" +
formatMinuteOfDay(drag.endMin.coerceAtMost(MINUTES_PER_DAY), use24Hour, locale) formatMinuteOfDay(endMin, use24Hour, locale)
Box( Box(
modifier = Modifier modifier = Modifier
// Absolute: these are root coordinates, and the direction-aware // Absolute: these are root coordinates, and the direction-aware

View File

@@ -41,7 +41,14 @@ class TimelineZoom(
var scale: TimelineScale by mutableStateOf(initial) var scale: TimelineScale by mutableStateOf(initial)
private set private set
private var pinching = false /**
* Whether fingers are rescaling the timeline right now. Snapshot state, so
* anything that tweens off [scale] can stand down for the gesture: the
* height changes every pointer frame, and a spring would spend the whole
* pinch chasing a target that has already moved.
*/
var isPinching: Boolean by mutableStateOf(false)
private set
/** /**
* Take a value that came from the preference. Ignored mid-pinch: the stored * Take a value that came from the preference. Ignored mid-pinch: the stored
@@ -49,11 +56,11 @@ class TimelineZoom(
* snap the timeline back while the user is still pinching. * snap the timeline back while the user is still pinching.
*/ */
fun adopt(stored: TimelineScale) { fun adopt(stored: TimelineScale) {
if (!pinching) scale = stored if (!isPinching) scale = stored
} }
fun beginPinch() { fun beginPinch() {
pinching = true isPinching = true
} }
fun pinchTo(hourHeight: Dp) { fun pinchTo(hourHeight: Dp) {
@@ -61,7 +68,7 @@ class TimelineZoom(
} }
fun endPinch() { fun endPinch() {
pinching = false isPinching = false
persist(scale) persist(scale)
} }
} }

View File

@@ -346,7 +346,7 @@ private fun DayContent(
onEventClick = onEventClick, onEventClick = onEventClick,
onCreateAt = onCreateAt, onCreateAt = onCreateAt,
onDrop = { drop -> onDrop = { drop ->
move?.move( val took = move?.move(
MoveRequest( MoveRequest(
eventId = drop.event.eventId, eventId = drop.event.eventId,
beginMillis = drop.event.start.toEpochMilliseconds(), beginMillis = drop.event.start.toEpochMilliseconds(),
@@ -354,6 +354,9 @@ private fun DayContent(
target = MoveTarget.Start(drop.startInstant(zone)), target = MoveTarget.Start(drop.startInstant(zone)),
), ),
) )
// Refused, so nothing will land: let the copy go now
// rather than hold it out for a settle that never comes.
if (took != true) dragController.release()
}, },
) )
} }

View File

@@ -226,15 +226,16 @@ class MonthDragController {
} }
/** /**
* End the drag. A real drop keeps its chip on the target day as [settling] * End the drag, handing back where it landed — null when it never resolved,
* until [release], slid over to that day's column so it lands where the grid * or when it landed back on the day it came from. A real drop keeps its chip
* is about to draw it. * on the target day as [settling] until [release], slid over to that day's
* column so it lands where the grid is about to draw it.
*/ */
fun finish(): MonthChipDrop? { fun finish(): MonthChipDrop? {
val landed = drag val landed = drag
val left = landed?.targetDate?.let(::columnLeft) val left = landed?.targetDate?.let(::columnLeft)
cancel() cancel()
val target = landed?.targetDate ?: return null val target = landed?.targetDate?.takeIf { it != landed.grabDate } ?: return null
settling = landed.copy( settling = landed.copy(
topLeftInRoot = Offset(left ?: landed.topLeftInRoot.x, landed.topLeftInRoot.y), topLeftInRoot = Offset(left ?: landed.topLeftInRoot.x, landed.topLeftInRoot.y),
) )

View File

@@ -2209,16 +2209,17 @@ private fun monthChipDragModifier(
// How many columns the finger crossed — grabbing the middle of a // How many columns the finger crossed — grabbing the middle of a
// multi-day bar shifts by what it travelled, not to where it landed. // multi-day bar shifts by what it travelled, not to where it landed.
val delta = (drop.targetDate.toEpochDays() - drop.grabDate.toEpochDays()).toInt() val delta = (drop.targetDate.toEpochDays() - drop.grabDate.toEpochDays()).toInt()
if (delta != 0) { val took = moveScope?.move(
moveScope?.move( MoveRequest(
MoveRequest( eventId = drop.event.eventId,
eventId = drop.event.eventId, beginMillis = drop.event.start.toEpochMilliseconds(),
beginMillis = drop.event.start.toEpochMilliseconds(), endMillis = drop.event.end.toEpochMilliseconds(),
endMillis = drop.event.end.toEpochMilliseconds(), target = MoveTarget.ByDays(delta),
target = MoveTarget.ByDays(delta), ),
), )
) // Refused, so nothing will land: let the copy go now rather than
} // hold the chip out for a settle that can never arrive.
if (took != true) controller?.release()
} }
}, },
onCancel = { controller?.cancel() }, onCancel = { controller?.cancel() },

View File

@@ -382,7 +382,7 @@ private fun WeekContent(
onOpenDay = onOpenDay, onOpenDay = onOpenDay,
onCreateAt = onCreateAt, onCreateAt = onCreateAt,
onDrop = { drop -> onDrop = { drop ->
move?.move( val took = move?.move(
MoveRequest( MoveRequest(
eventId = drop.event.eventId, eventId = drop.event.eventId,
beginMillis = drop.event.start.toEpochMilliseconds(), beginMillis = drop.event.start.toEpochMilliseconds(),
@@ -390,6 +390,9 @@ private fun WeekContent(
target = MoveTarget.Start(drop.startInstant(zone)), target = MoveTarget.Start(drop.startInstant(zone)),
), ),
) )
// Refused, so nothing will land: let the copy go now
// rather than hold it out for a settle that never comes.
if (took != true) dragController.release()
}, },
) )
} }

View File

@@ -67,11 +67,12 @@ class RescheduleViewModelTest {
rrule: String? = null, rrule: String? = null,
isAllDay: Boolean = false, isAllDay: Boolean = false,
isException: Boolean = false, isException: Boolean = false,
anchorMillis: Long = beginMillis,
): EventDetail = EventDetail( ): EventDetail = EventDetail(
instance = EventInstance( instance = EventInstance(
instanceId = 42L, eventId = 42L, calendarId = 1L, title = "Standup", instanceId = 42L, eventId = 42L, calendarId = 1L, title = "Standup",
start = Instant.fromEpochMilliseconds(beginMillis), start = Instant.fromEpochMilliseconds(anchorMillis),
end = Instant.fromEpochMilliseconds(endMillis), end = Instant.fromEpochMilliseconds(anchorMillis + (endMillis - beginMillis)),
isAllDay = isAllDay, color = 0xFF000000.toInt(), location = null, isAllDay = isAllDay, color = 0xFF000000.toInt(), location = null,
), ),
description = null, organizer = null, attendees = emptyList(), rrule = rrule, description = null, organizer = null, attendees = emptyList(), rrule = rrule,
@@ -232,6 +233,64 @@ class RescheduleViewModelTest {
assertThat(vm.scopePrompt.value).isEqualTo(MoveScopePrompt(occurrenceOnly = false)) assertThat(vm.scopePrompt.value).isEqualTo(MoveScopePrompt(occurrenceOnly = false))
} }
@Test
fun `a time-only drop that would carry the anchor past midnight offers only the occurrence`(
@TempDir tempDir: Path,
) = runTest(dispatcher) {
// The anchor sits at 23:30 while the dragged occurrence is at midday —
// only possible when the row pins no zone, so the two resolve an hour
// apart. +1h leaves the occurrence on its day but rolls the anchor onto
// the next one, which would leave BYDAY naming the wrong weekday.
val anchor = LocalDateTime(LocalDate(2026, 6, 1), LocalTime(23, 30))
.toInstant(TimeZone.currentSystemDefault())
.toEpochMilliseconds()
val fake = FakeCalendarDataSource().apply {
eventDetailResult = { detail(rrule = "FREQ=WEEKLY;BYDAY=MO", anchorMillis = anchor) }
}
val vm = viewModel(tempDir, fake)
vm.move(oneHourLater())
advanceUntilIdle()
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") }
}
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 `undo is refused while a write is running`(@TempDir tempDir: Path) = runTest(dispatcher) {
val fake = FakeCalendarDataSource().apply { eventDetailResult = { detail() } }
val vm = viewModel(tempDir, fake)
vm.move(oneHourLater())
advanceUntilIdle()
val undo = (vm.outcome.value as MoveOutcome.Moved).undo!!
// A second drop parks on its scope dialog, which holds the pipeline.
fake.eventDetailResult = { detail(rrule = "FREQ=WEEKLY;BYDAY=MO") }
vm.move(toWednesday())
advanceUntilIdle()
assertThat(vm.undo(undo)).isFalse()
advanceUntilIdle()
assertThat(fake.updatedEvents).hasSize(1)
}
@Test @Test
fun `an exception row is written as a plain event, never as a nested exception`( fun `an exception row is written as a plain event, never as a nested exception`(
@TempDir tempDir: Path, @TempDir tempDir: Path,