From 916f64d2df569ae61e16ada672616579d08020e3 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Sun, 2 Aug 2026 19:29:58 +0200 Subject: [PATCH] Trim the drag comments down to what they have to say (#68) --- .../calendula/data/calendar/Projections.kt | 6 +- .../calendula/domain/CalendarRowState.kt | 6 +- .../calendula/domain/EventShift.kt | 26 ++---- .../jeanlucmakiola/calendula/domain/Models.kt | 5 +- .../calendula/domain/RecurrenceRealign.kt | 34 +++---- .../calendula/ui/CalendarHost.kt | 5 +- .../calendula/ui/common/BlockPlacement.kt | 16 ++-- .../calendula/ui/common/EventDrag.kt | 53 +++-------- .../calendula/ui/common/EventMoveHost.kt | 26 ++---- .../calendula/ui/common/EventMoveScope.kt | 26 ++---- .../calendula/ui/common/HourGutter.kt | 6 +- .../ui/common/RecurringScopeDialog.kt | 14 ++- .../ui/common/RescheduleViewModel.kt | 90 +++++++------------ .../calendula/ui/common/TimeFormat.kt | 4 +- .../calendula/ui/common/TimelineDrag.kt | 85 +++++++----------- .../calendula/ui/day/DayScreen.kt | 10 +-- .../calendula/ui/month/MonthDrag.kt | 81 ++++++----------- .../calendula/ui/month/MonthScreen.kt | 67 ++++++-------- .../calendula/ui/month/MonthUiState.kt | 3 - .../calendula/ui/week/WeekScreen.kt | 17 ++-- .../data/calendar/EventWriteMapperTest.kt | 5 +- .../calendula/domain/CalendarRowStateTest.kt | 3 +- .../calendula/domain/EventShiftTest.kt | 9 +- .../calendula/domain/RecurrenceRealignTest.kt | 9 +- .../ui/common/RescheduleViewModelTest.kt | 14 ++- 25 files changed, 215 insertions(+), 405 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/Projections.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/Projections.kt index 1b665aa..ac3e138 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/Projections.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/Projections.kt @@ -89,10 +89,8 @@ internal object EventDetailProjection { // Recurring rows carry DURATION instead of DTEND; the detail screen // needs it to render a series opened without a named occurrence. CalendarContract.Events.DURATION, - // Non-null on a modified-occurrence exception row (it points at the - // series). "No RRULE" alone can't tell an exception from a master — a - // sync adapter that leaves the rule on the exception would otherwise - // get an exception written against an exception (#68). + // Non-null on a modified-occurrence exception row; "no RRULE" alone + // can't tell an exception from a master (#68). CalendarContract.Events.ORIGINAL_ID, ) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarRowState.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarRowState.kt index 72b91d0..981b7e3 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarRowState.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarRowState.kt @@ -46,10 +46,8 @@ val CalendarSource.isEventTarget: Boolean /** * Whether this calendar's events may have their times rewritten by a drag (#68). - * Deliberately not [isEventTarget]: an event living in a switched-off or - * non-syncing calendar isn't rendered anyway, while a *managed* event is - * editable (reminders, notes) yet must never move — the next contacts sync would - * put it back. + * Deliberately not [isEventTarget]: a managed event stays editable (reminders, + * notes) yet must never move, since the next contacts sync would put it back. */ val CalendarSource.allowsEventMove: Boolean get() = canModifyContents && !isManaged diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/EventShift.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/EventShift.kt index 5af7c98..84a810a 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/EventShift.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/EventShift.kt @@ -11,22 +11,17 @@ import kotlin.time.Instant /** * The zone this form's wall-clock times mean, matching what the data layer * resolves them in at write time: the form's own pinned zone, else [deviceZone]. - * An unparseable pinned id falls back to the device, like the write path does. */ fun EventForm.resolvedZone(deviceZone: TimeZone): TimeZone = timezone?.let { runCatching { TimeZone.of(it) }.getOrNull() } ?: deviceZone /** - * The form moved so it starts at [newStart], keeping its length. The wall-clock - * values are re-derived in the event's own zone, not the device's, so dragging a - * pinned event still means what the event means. + * The form moved so it starts at [newStart], keeping its **instant** duration — + * a recurring event's length travels to the provider as `DURATION`, so keeping + * wall clock instead would rewrite the series' length across a DST boundary. * - * The **instant** duration is preserved, not the wall-clock span: a recurring - * event's length travels to the provider as `DURATION`, so preserving wall clock - * would rewrite a whole series' length whenever a shift crosses a DST boundary. - * - * All-day events carry placeholder times and are date-anchored — move them with - * [shiftedByDays] instead; this returns them untouched. + * All-day events are date-anchored — use [shiftedByDays]; this returns them + * untouched. */ fun EventForm.shiftedTo(newStart: Instant, deviceZone: TimeZone): EventForm { if (isAllDay) return this @@ -39,14 +34,9 @@ fun EventForm.shiftedTo(newStart: Instant, deviceZone: TimeZone): EventForm { } /** - * The form moved [days] calendar days, keeping its time of day and its length. - * A multi-day event keeps its span rather than collapsing to a single day. - * - * An all-day event is pure date arithmetic — both ends move, and the placeholder - * times ride along untouched. A timed one re-derives its end from the preserved - * **instant** duration, for the same reason [shiftedTo] does: the length travels - * to a recurring row as `DURATION`, so a move onto a DST changeover that kept - * wall clock would silently rewrite the whole series' length. + * The form moved [days] calendar days, keeping its time of day and its span. + * All-day events are pure date arithmetic; a timed one preserves its **instant** + * duration, for the same reason [shiftedTo] does. */ fun EventForm.shiftedByDays(days: Int, deviceZone: TimeZone): EventForm { if (days == 0) return this diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt index d3c41d9..90491de 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt @@ -126,9 +126,8 @@ data class EventDetail( val eventColorKey: String? = null, /** * True when this row is a modified occurrence of a series (`ORIGINAL_ID` is - * set), not a master. Such a row stands alone — writing an exception against - * it would nest one exception inside another — so a reschedule always takes - * the plain whole-row path, whatever [rrule] a sync adapter left on it. + * set) rather than a master, so a reschedule takes the plain whole-row path + * whatever [rrule] a sync adapter left on it. */ val isException: Boolean = false, ) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/RecurrenceRealign.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/RecurrenceRealign.kt index cc06298..b55db7d 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/RecurrenceRealign.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/RecurrenceRealign.kt @@ -5,29 +5,16 @@ import kotlinx.datetime.LocalDate /** * [rrule] re-anchored from an occurrence on [oldStart] to one on [newStart]. * - * `Events.RRULE` is written verbatim while DTSTART moves, so a rule that names - * its own day — `FREQ=WEEKLY;BYDAY=MO`, what Google and CalDAV write for nearly - * every weekly series — would keep pointing at Monday after the anchor became a - * Wednesday, and the series would not move at all. `BYDAY` is therefore - * re-derived from [newStart]. + * `Events.RRULE` is written verbatim while DTSTART moves, so `FREQ=WEEKLY;BYDAY=MO` + * would keep naming Monday after the anchor became a Wednesday and the series + * would not move at all — `BYDAY` is re-derived from [newStart]. * - * **Only weekly `BYDAY` is realigned, and only for a whole-day move.** The rule - * has to agree with the series *anchor*, which is not the occurrence being - * dragged: the anchor moves by the same wall-clock shift, and only weekday - * arithmetic survives that unchanged, because it is uniform mod 7 and every - * anchor time-of-day crosses the same number of midnights. Day-of-month does not - * — `BYMONTHDAY=28` on a January anchor dragged from Feb 28 to Mar 1 gives an - * anchor of Jan 29 under a `BYMONTHDAY=1` rule, which is not an instance of its - * own rule and materialises a phantom occurrence on any client that trusts - * DTSTART. Those rules return null instead. The caller must also refuse when the - * shift is not a whole number of days, for the same reason (see - * `RescheduleViewModel`). - * - * Null also covers rules one moved occurrence cannot resolve at all - * (`BYDAY=MO,WE`, an ordinal `2TH`, `BYSETPOS`, …) — dropping the extra days - * would delete occurrences. The accepted parts are deliberately a subset of what - * [parseSimpleRecurrence] understands, so anything realignable is also a rule - * [problems] can check the `UNTIL` of. + * Only weekly `BYDAY` is realigned, and only for a whole-day move: the rule has + * to agree with the series *anchor*, and weekday arithmetic is the only kind + * that survives the same wall-clock shift unchanged. Everything else returns + * null, as do rules one moved occurrence can't resolve (`BYDAY=MO,WE`, `2TH`, + * `BYSETPOS`). The accepted parts are a subset of [parseSimpleRecurrence], so + * anything realignable is also a rule [problems] can check the `UNTIL` of. */ fun realignRecurrence(rrule: String, oldStart: LocalDate, newStart: LocalDate): String? { if (oldStart == newStart) return rrule @@ -54,8 +41,7 @@ fun realignRecurrence(rrule: String, oldStart: LocalDate, newStart: LocalDate): else -> return null } } - // BYDAY is only simple on a weekly rule (matching parseSimpleRecurrence); - // "every Monday of the month" is a shape this has not been reasoned about. + // BYDAY is only simple on a weekly rule, matching parseSimpleRecurrence. if (!weekly && parts.any { it.substringBefore('=').trim().uppercase() == "BYDAY" }) return null if (parts.none { it.substringBefore('=').trim().uppercase() == "FREQ" }) return null return prefix + rebuilt.joinToString(";") 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 e73445d..f2ef6e5 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHost.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHost.kt @@ -398,9 +398,8 @@ fun CalendarHost( } } - // Scope prompt + confirmation/undo snackbar for a dropped event. Declared - // right after the calendar views, so any overlay opened afterwards covers - // the snackbar rather than the other way round. + // Scope prompt + confirmation/undo snackbar for a dropped event, declared + // right after the calendar views so later overlays cover it. EventMoveHost(reschedule, modifier = Modifier.fillMaxSize()) // Search overlay — below detail/edit in the Box so a tapped result's diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/BlockPlacement.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/BlockPlacement.kt index a886d5c..c1fda30 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/BlockPlacement.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/BlockPlacement.kt @@ -17,9 +17,8 @@ import androidx.compose.ui.unit.Dp import de.jeanlucmakiola.floret.identity.rememberReduceMotion /** - * A timed block's own time label, crossfaded rather than replaced. The block - * keeps its identity across a move and slides to the new slot; the label is the - * one thing on it that would otherwise change in a single frame. + * A timed block's own time label, crossfaded rather than replaced — the block + * slides to its new slot, so the label shouldn't change in a single frame. */ @OptIn(ExperimentalMaterial3ExpressiveApi::class) @Composable @@ -50,14 +49,9 @@ fun BlockTimeLabel(label: String, color: Color, modifier: Modifier = Modifier) { data class BlockPlacement(val x: Dp, val y: Dp, val width: Dp, val height: Dp) /** - * A timed block's placement, tweened rather than jumped. Every bound a block - * has changes for a reason the user just caused — a drop landing at a new time, - * an undo putting it back, a neighbour arriving and halving both lanes — and all - * of them read better as motion than as a new layout appearing. - * - * Continuity comes from the caller keying each block by identity; a block - * composed for the first time starts *at* its target, so nothing flies in from - * the corner on the first frame. + * 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 + * starts at its target, so nothing flies in on the first frame. */ @OptIn(ExperimentalMaterial3ExpressiveApi::class) @Composable diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventDrag.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventDrag.kt index 88cf1ac..12bf70f 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventDrag.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventDrag.kt @@ -21,29 +21,15 @@ import androidx.compose.ui.platform.LocalViewConfiguration import androidx.compose.ui.unit.IntSize import androidx.compose.ui.unit.dp -/** - * How far the finger may wander during the hold. Deliberately well under touch - * slop (~18dp): past that an ancestor — the vertical scroll, the page swipe — - * claims the gesture, and a hold that survived to there would have opened a dead - * zone where neither the drag nor the page turn happens. - */ +/** How far the finger may wander during the hold — well under touch slop. */ private val PICKUP_TOLERANCE = 6.dp /** * Pick an event block up with a long press and drag it, without - * `detectDragGesturesAfterLongPress`. - * - * The stock detector cancels the press as soon as an ancestor consumes — and - * during the hold the block itself consumes nothing, so the scroll and the page - * swipe are free to claim at their own slop — and it also cancels when the finger - * leaves the block, which a `MIN_EVENT_FRACTION`-tall block loses immediately. - * This one keeps its own timeout and its own (much smaller) tolerance, and never - * cancels on leaving the bounds. - * - * Movement before the timeout is deliberately **not** consumed: consuming it - * would kill the ancestor's gesture outright, so a scroll that happens to start - * on top of a block would die. Fast movement means the user meant to scroll — - * abandon quietly. A second finger means a pinch, which owns the gesture. + * `detectDragGesturesAfterLongPress`: the stock detector cancels as soon as an + * ancestor consumes or the finger leaves the block, which a + * `MIN_EVENT_FRACTION`-tall block loses immediately. Movement before the timeout + * is deliberately not consumed, so a scroll starting on top of a block survives. */ @Composable fun rememberEventDragSource( @@ -68,9 +54,8 @@ fun rememberEventDragSource( /** * The same pickup, for a surface that carries many draggable pieces rather than * being one itself — the month grid, whose chips are covered by a full-bleed tap - * layer and so can never take pointer input of their own. [onPickUp] receives the - * press position local to this node and answers whether anything is there; - * returning false abandons the gesture as if the hold had never completed. + * layer. [onPickUp] receives the press position local to this node and answers + * whether anything is there; false abandons the gesture. */ @Composable fun rememberDragSurface( @@ -121,16 +106,10 @@ fun rememberDragSurface( if (!took) return@awaitEachGesture var dropped = false try { - // Once the block is lifted the gesture is ours, so it is - // driven on the initial pass and consumed there. That pass - // runs parent → child, which gets both halves right: an - // ancestor that outranks us (the pinch, which claims the - // moment a second finger lands anywhere in the timeline) has - // already consumed by the time we look, and everything below - // us — the month grid's full-bleed tap layer, which is a - // descendant and would otherwise open the day on lift — sees - // ours. Consumption persists across passes, so the scroll and - // the page swipe stand down on main as well. + // Driven on the initial pass, which runs parent → child: an + // ancestor that outranks us (the pinch) has already consumed + // by the time we look, and descendants — the month grid's + // full-bleed tap layer — see our consumption. while (true) { val event = awaitPointerEvent(PointerEventPass.Initial) val change = event.changes.firstOrNull { it.id == down.id } ?: break @@ -146,19 +125,15 @@ fun rememberDragSurface( ?.let { currentMove(it.localToRoot(change.position)) } } } finally { - // Also reached when the pointer node is disposed mid-drag (the - // page swapping out under the finger) — that is a cancel, not a - // drop, and must never write. + // Also reached when the pointer node is disposed mid-drag — + // a cancel, not a drop, and must never write. if (dropped) currentDrop() else currentCancel() } } } } -/** - * The app's first haptics: a lift on pickup, then a tick every time the drop - * target snaps to a different slot, so the granularity is felt rather than read. - */ +/** A lift on pickup, then a tick every time the drop target snaps to a new slot. */ @Composable fun DragSnapHaptics(slot: Any?) { val haptics = LocalHapticFeedback.current 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 20daab9..6bdfc90 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 @@ -32,12 +32,7 @@ import java.util.Locale /** How long the confirmation chip stays up, matching a short snackbar. */ private const val CHIP_MILLIS = 4_000L -/** - * How long an *undone* move stays up. Shorter than everything else the chip - * says: it offers nothing to act on, and it confirms a change the user has just - * asked for and can see on the grid behind it, so the full dwell is only the - * chip outstaying what it had to say. - */ +/** How long an *undone* move stays up — shorter, since it offers nothing to act on. */ private const val UNDONE_CHIP_MILLIS = 1_600L /** What the chip currently reads, kept past the outcome it was built from. */ @@ -48,12 +43,9 @@ private val FAB_BAND = 88.dp /** * The two surfaces a drag-and-drop reschedule needs on top of the calendar: the - * recurring-scope prompt, and the confirmation chip carrying Undo. - * - * None of the four calendar screens sets a `snackbarHost` on its Scaffold, so - * this hosts its own confirmation — a pill on the FAB's own band at the bottom - * start rather than a full-width bar, so the calendar it confirms a change to - * stays visible behind it. + * recurring-scope prompt, and the confirmation chip carrying Undo. None of the + * calendar screens sets a `snackbarHost`, so this hosts its own confirmation as + * a pill on the FAB's band, leaving the calendar visible behind it. */ @Composable fun EventMoveHost(viewModel: RescheduleViewModel, modifier: Modifier = Modifier) { @@ -86,9 +78,8 @@ fun EventMoveHost(viewModel: RescheduleViewModel, modifier: Modifier = Modifier) } // Held past the outcome being consumed so the chip has something to draw - // while it springs back out. Updated *in composition* rather than from an - // effect: an effect lands a frame late, so the chip would open carrying the - // previous message and grow into this one while it was still animating in. + // while it springs back out. Updated in composition rather than from an + // effect, which would land a frame late and open the chip on stale text. val shown = remember { mutableStateOf(ChipContent("", null)) } if (message != null && (shown.value.message != message || shown.value.undo != moved?.undo)) { shown.value = ChipContent(message, moved?.undo) @@ -102,9 +93,8 @@ fun EventMoveHost(viewModel: RescheduleViewModel, modifier: Modifier = Modifier) BoxWithConstraints(modifier = modifier.fillMaxSize()) { val chipMaxWidth = maxWidth - FAB_BAND - // A FAB-height band anchored at the bottom start with the FAB's own - // margin; centring the chip in it lines it up beside the bottom-end FAB - // at exactly its height, rather than sitting a touch above it. + // A FAB-height band at the bottom start with the FAB's own margin, so + // the chip lines up beside the bottom-end FAB. Box( modifier = Modifier .align(Alignment.BottomStart) 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 62149c1..a1e04fe 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 @@ -19,13 +19,8 @@ import kotlinx.coroutines.flow.StateFlow /** * Drag-to-reschedule wiring (#68), provided once at `CalendarHost` and read by - * whichever event block is being composed. A composition local rather than six - * layers of parameters: every calendar surface needs the same three things, and - * the blocks that need them sit deep inside private composables. - * - * Null means moving is off entirely (no host provided it) — a block then - * registers no drag gesture at all, so a long press keeps its old meaning of - * nothing happening. + * whichever event block is being composed. Null means moving is off entirely and + * blocks register no drag gesture at all. */ @Immutable class EventMoveScope( @@ -36,10 +31,9 @@ class EventMoveScope( val movableCalendarIds: Set, val move: (MoveRequest) -> Unit, /** - * 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 - * object across a move: it is a composition local every visible block reads, - * and replacing it would recompose all of them twice per drop. + * 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`. */ @@ -68,8 +62,7 @@ fun moveInFlight(): Boolean { /** * Runs [onUndo] when an undo write begins, and never for one that began before - * this composable came on screen — a view switched to *after* an undo has - * nothing left to carry back. + * this composable came on screen. */ @Composable fun OnUndoStarted(onUndo: () -> Unit) { @@ -88,10 +81,8 @@ const val GHOST_ALPHA: Float = 0.3f /** * Opacity for a block whose copy is in flight: ghosted from the lift until the - * copy is handed back, then animated up rather than switched. The ghost keeps - * its place through the write and *travels* to the new slot when the grid - * re-reads it, arriving under the copy as that fades — so what the eye follows - * is one block moving, not one vanishing and another appearing. + * copy is handed back, then animated up rather than switched, so the block + * appears to travel to its new slot instead of vanishing and reappearing. */ @Composable fun ghostAlpha(lifted: Boolean): Float = animateFloatAsState( @@ -101,8 +92,7 @@ fun ghostAlpha(lifted: Boolean): Float = animateFloatAsState( /** * A TalkBack action that opens [event] in the edit form, so rescheduling isn't - * pointer-only. Null when this event can't be moved — the block then carries no - * action and registers no drag either. + * pointer-only. Null when this event can't be moved. */ @Composable fun eventMoveAction(event: EventInstance): CustomAccessibilityAction? { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/HourGutter.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/HourGutter.kt index d92721c..2a4beec 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/HourGutter.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/HourGutter.kt @@ -56,8 +56,7 @@ fun HourGutter( ) { val use24Hour = LocalUse24HourFormat.current val locale = currentLocale() - // Derived, not read directly: the drag is rewritten every frame, while its - // snapped start only changes once per slot — which is all the gutter shows. + // Derived: the drag is rewritten every frame, its snapped start once a slot. val dragStartMin by remember(dragController) { derivedStateOf { dragController.drag?.startMin } } @@ -103,8 +102,7 @@ fun HourGutter( .height(BADGE_HEIGHT) .background(MaterialTheme.colorScheme.primary, CircleShape) .padding(horizontal = 4.dp) - // The dragged block behind it already carries this time; a - // second copy would only duplicate the announcement. + // The dragged block already announces this time. .clearAndSetSemantics { }, contentAlignment = Alignment.Center, ) { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RecurringScopeDialog.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RecurringScopeDialog.kt index 2765c67..1c8d95f 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RecurringScopeDialog.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RecurringScopeDialog.kt @@ -15,17 +15,13 @@ import de.jeanlucmakiola.floret.components.OptionCard /** * How far a write to a recurring event should reach: this occurrence, it and - * everything after (a series split), or the whole series. - * - * One of the two carve-outs from the full-screen picker rule — a two-or-three - * option decision taken mid-action reads better as a popup than as a near-empty - * screen. Shared by the edit screen's save and a drag-and-drop reschedule, which - * want the same three options for the same reasons. + * everything after (a series split), or the whole series. Shared by the edit + * screen's save and a drag-and-drop reschedule; one of the two carve-outs from + * the full-screen picker rule. * * [allowOccurrence] drops "only this event" (an exception row can't carry its own - * rule, so a changed recurrence rules it out); [allowSeries] drops the two wider - * options, for a rule whose days can't be recalculated from one moved - * occurrence — [reason] then says why. + * rule); [allowSeries] drops the two wider options, for a rule whose days can't + * be recalculated from one moved occurrence — [reason] then says why. */ @Composable fun RecurringScopeDialog( 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 1300752..a474ddc 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 @@ -47,18 +47,15 @@ sealed interface MoveTarget { /** * A whole-day shift, keeping the time of day — a month-grid or all-day drag. - * A delta rather than a target date on purpose: the grid already knows how - * many columns the finger crossed, and re-deriving that from a date would - * mean the screen and this view model each resolving the event's first day - * in their own zone, which can disagree by a day. + * A delta rather than a target date, so the screen and this view model can't + * disagree by a day over which zone the event's first day is resolved in. */ data class ByDays(val days: Int) : MoveTarget } /** * One dropped event. [beginMillis]/[endMillis] are the dragged *occurrence's* - * own times (`Instances.BEGIN`/`END`), exactly as the detail and edit screens - * pass them, so a recurring series resolves the right occurrence. + * own times (`Instances.BEGIN`/`END`), as the detail and edit screens pass them. */ data class MoveRequest( val eventId: Long, @@ -69,9 +66,8 @@ data class MoveRequest( /** * A recurring drop waiting for the user to pick how far it reaches. - * [occurrenceOnly] means the rule names days that a single moved occurrence - * can't re-derive (`BYDAY=MO,WE`, `2TH`, …), so the only honest option left is - * this one occurrence — see [realignRecurrence]. + * [occurrenceOnly] means the rule names days a single moved occurrence can't + * re-derive (`BYDAY=MO,WE`, `2TH`, …) — see [realignRecurrence]. */ data class MoveScopePrompt(val occurrenceOnly: Boolean) @@ -113,11 +109,9 @@ sealed interface MoveOutcome { * Writes a drag-and-drop reschedule (#68) through the same repository calls the * edit screen's save uses, so recurring writes, reminder reconciliation and * attendee preservation behave identically. Hosted at `CalendarHost` so one - * instance serves every calendar view and survives view switches. - * - * The full prefilled form is carried through the write — never a stripped one — - * because the occurrence-exception path reconciles reminders and attendees onto - * the new row, and a partial form would wipe them. + * instance serves every calendar view and survives view switches. The full + * prefilled form is carried through the write — a stripped one would wipe the + * reminders and attendees the occurrence-exception path reconciles. */ @HiltViewModel class RescheduleViewModel @Inject constructor( @@ -137,27 +131,23 @@ class RescheduleViewModel @Inject constructor( /** * True from the moment a drop is accepted until its write settles — the - * window the dropped block holds its landing position for, rather than - * snapping back to where it came from until the grid re-reads it. + * window the dropped block holds its landing position for. */ val inFlight: StateFlow = _inFlight.asStateFlow() private val _undoStarted = MutableStateFlow(0) /** - * Ticks the moment an undo write begins — before the provider has anything to - * re-read. An undo moves an event exactly as a drop does, so the view that - * drew the drop takes this as its cue to carry the chip back rather than let - * it reappear on the old day. A counter rather than the undo itself: what a - * view needs is the *timing*, and it already knows what it moved. + * 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. */ val undoStarted: StateFlow = _undoStarted.asStateFlow() /** * Set from the moment a drop is accepted until its write settles. Two drops - * of the same recurring event landing inside that window would each compute - * their shift from the same pre-move occurrence, and the data layer applies - * both to the re-read anchor — so the shifts would compound. + * of the same recurring event inside that window would compute their shift + * from the same pre-move occurrence, and the shifts would compound. */ private var busy = false set(value) { @@ -167,8 +157,7 @@ class RescheduleViewModel @Inject constructor( /** * The calendars whose events may be dragged. Nothing below the UI guards - * this — the repository and data source attempt any write handed to them — - * so a block outside this set registers no drag gesture at all. + * this, so a block outside this set registers no drag gesture at all. */ val movableCalendarIds: StateFlow> = repository.calendars() .map { calendars -> calendars.filter { it.allowsEventMove }.map { it.id }.toSet() } @@ -187,10 +176,7 @@ class RescheduleViewModel @Inject constructor( val updated: EventForm, /** True for a series master with a rule — an exception row is not one. */ val isRecurring: Boolean, - /** - * False when the rule names days this move can't re-derive, so anything - * wider than the single occurrence would leave rule and anchor disagreeing. - */ + /** False when the rule names days this move can't re-derive. */ val canRealign: Boolean, /** The series row's DTSTART date after the move — what its `UNTIL` must clear. */ val newAnchorDate: LocalDate, @@ -219,8 +205,7 @@ class RescheduleViewModel @Inject constructor( /** Answer the scope dialog. */ fun moveWithScope(scope: RecurringWriteScope) { val prepared = pending ?: return - // Belt and braces against the dialog ever offering a scope the rule - // can't carry: writing it would leave anchor and rule disagreeing. + // Guard against the dialog ever offering a scope the rule can't carry. if (!prepared.canRealign && scope != RecurringWriteScope.ThisEvent) return pending = null _scopePrompt.value = null @@ -238,10 +223,9 @@ class RescheduleViewModel @Inject constructor( } /** - * Put a completed move back where it came from. The outcome deliberately - * stands until the inverse write reports back: clearing it first would drop - * the confirmation chip and open a second one a moment later, rather than - * letting the one chip change what it says. + * 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. */ fun undo(undo: MoveUndo) { if (busy) return @@ -288,13 +272,11 @@ class RescheduleViewModel @Inject constructor( is MoveTarget.Start -> original.shiftedTo(target.instant, zone) is MoveTarget.ByDays -> original.shiftedByDays(target.days, zone) } - // A zero-distance drop is not a write. Matches the edit form's own - // pristine-form no-op, and keeps a mis-aimed long press harmless. + // A zero-distance drop is not a write. if (shifted == original) return null - // The UNTIL check is deferred to the write: how far the move reaches - // decides which date has to clear it, and "only this event" writes an - // exception row that no UNTIL constrains at all. + // 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() - EventFormProblem.RecurrenceEndsBeforeStart).isNotEmpty()) { _outcome.value = MoveOutcome.Failed return null @@ -305,10 +287,7 @@ class RescheduleViewModel @Inject constructor( val movedDay = shifted.start.date != original.start.date // The series anchor moves by the same *wall-clock* shift as the dragged // occurrence, so only a whole-day shift moves it by a predictable number - // of days — a drag that also changes the time of day would carry some - // anchor times of day across an extra midnight and leave the rule naming - // the wrong weekday. All-day forms shift as bare dates, so they always - // qualify. + // of days; anything else can cross an extra midnight at the anchor. val wholeDayShift = original.isAllDay || shifted.start.time == original.start.time val realigned = if (isRecurring && movedDay) { if (wholeDayShift) { @@ -332,8 +311,7 @@ class RescheduleViewModel @Inject constructor( isRecurring = isRecurring, canRealign = !isRecurring || !movedDay || realigned != null, // Where the series row's own DTSTART lands, given the anchor moves by - // the same shift. Only meaningful under [wholeDayShift], which is the - // only case a wider-than-one-occurrence write is offered in. + // the same shift — only meaningful under wholeDayShift. newAnchorDate = anchorDate(detail, original, zone) .plus(shifted.start.date.toEpochDays() - original.start.date.toEpochDays(), DateTimeUnit.DAY), ) @@ -382,14 +360,10 @@ class RescheduleViewModel @Inject constructor( /** * Whether this write would leave a rule whose `UNTIL` precedes the first day - * it now applies to — the provider then generates nothing and the event - * silently disappears from every view. - * - * Which date has to clear `UNTIL` depends on how far the write reaches: a - * whole-series move carries the series *anchor*, a split starts a new series - * at the moved occurrence, and a single occurrence becomes an exception row - * that no `UNTIL` constrains. Testing the occurrence in every case would - * refuse the perfectly ordinary drag of a bounded series' last occurrence. + * it now applies to — the provider then generates nothing at all. Which date + * has to clear `UNTIL` depends on the scope: a whole-series move carries the + * series *anchor*, a split starts at the moved occurrence, and a single + * occurrence becomes an exception row that no `UNTIL` constrains. */ private fun endsBeforeItStarts(prepared: PreparedMove, scope: RecurringWriteScope): Boolean { if (!prepared.isRecurring || scope == RecurringWriteScope.ThisEvent) return false @@ -415,8 +389,7 @@ class RescheduleViewModel @Inject constructor( /** * Undo is offered only where the inverse is one symmetric write: a - * non-recurring event (absolute DTSTART/DTEND) and a whole-series move (the - * −Δ wall-clock shift lands on the re-read anchor). "This event" leaves an + * non-recurring event, and a whole-series move. "This event" leaves an * exception row behind and "this and following" splits the series with an * UNTIL truncation — neither is undone by shifting back. */ @@ -433,8 +406,7 @@ class RescheduleViewModel @Inject constructor( /** * Where the dragged occurrence now begins, in the same anchoring the views - * read dates back in: a UTC midnight for an all-day event, the real instant - * for a timed one. + * read dates back in: UTC midnight for an all-day event. */ private fun movedStartMillis(prepared: PreparedMove): Long { val form = prepared.updated diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimeFormat.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimeFormat.kt index 8fbb23a..fb6262a 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimeFormat.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimeFormat.kt @@ -45,8 +45,8 @@ fun formatMinuteOfDay(minutes: Int, is24Hour: Boolean, locale: Locale): String = /** * The time shown in the timeline gutter while a block is dragged: 24h → - * "09:15", 12h → "9:15". The meridiem is dropped on purpose — the hour labels - * around it already carry it, and the gutter is too narrow to hold it. + * "09:15", 12h → "9:15". The meridiem is dropped — the hour labels around it + * already carry it, and the gutter is too narrow. */ fun formatGutterTime(minutes: Int, is24Hour: Boolean, locale: Locale): String { val clamped = minutes.coerceIn(0, MINUTES_PER_DAY - 1) 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 f4686b8..6cb8a46 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 @@ -79,10 +79,9 @@ data class TimelineDrag( data class TimelineDrop(val event: EventInstance, val date: LocalDate, val startMin: Int) /** - * The timeline's live geometry, republished on every layout. Held in plain - * fields rather than snapshot state on purpose: it changes on every scroll - * frame, and recomposing the screen that often would cost far more than the - * drag loop's own per-frame read of it. + * The timeline's live geometry, republished on every layout. Plain fields rather + * than snapshot state: it changes on every scroll frame, and the drag loop reads + * it per frame anyway. */ class TimelineGeometry { /** The day-columns row — scrolling *content*, so its root position folds in the scroll. */ @@ -102,18 +101,15 @@ class TimelineGeometry { /** * Whether the columns are laid out right-to-left. Pointer coordinates are - * never mirrored, but the grid is, so the leftmost column is the *last* day - * in Arabic — the mapping has to flip with it. + * never mirrored but the grid is, so the mapping has to flip with it. */ var isRtl: Boolean = false } /** * Hoisted drag state for one timeline (#68). It lives above the per-page - * `AnimatedContent` — a page change mid-drag would otherwise strand a ghost — - * and the block it renders is drawn in an overlay: a `Card` column ends its - * modifier chain with a clip, so a block offset toward the neighbouring column - * would simply be cut off in place. + * `AnimatedContent`, so a page change mid-drag can't strand a ghost, and the + * block it renders is drawn in an overlay, clear of the day column's clip. */ @Stable class TimelineDragController { @@ -123,19 +119,16 @@ class TimelineDragController { private set /** - * A dropped block, held at the slot it landed on while the write runs. The - * grid behind it still shows the old time until the provider notifies and - * the query re-reads, so releasing the copy at drop time would snap the - * event back to where it came from for the length of the write. + * A dropped block, held at the slot it landed on while the write runs — the + * grid behind it still shows the old time until the query re-reads. */ var settling: TimelineDrag? by mutableStateOf(null) private set /** * Which block is lifted, and whether anything is. Separate snapshot state - * from [drag] on purpose: [drag] changes on every frame, and the blocks that - * only need to know "am I the ghost" must not recompose that often. Stays - * set through [settling], so the source never reappears under the copy. + * from [drag], which changes every frame, so blocks that only need "am I the + * ghost" don't recompose that often. Stays set through [settling]. */ var liftedInstanceId: Long? by mutableStateOf(null) private set @@ -143,10 +136,8 @@ class TimelineDragController { /** * Where the settled drop came from, and which event row it belongs to. The * instance id alone can't identify the source block for the length of the - * write: the provider regenerates `Instances` rows, so a re-read that still - * carries the *old* time can arrive under a new instance id — and a source - * matched by instance id would stop being the ghost and flash back to full - * opacity in the slot the event is about to leave. + * write: the provider regenerates `Instances` rows, so a re-read carrying the + * *old* time can arrive under a new instance id. */ private var settledOrigin: Triple? by mutableStateOf(null) @@ -156,9 +147,8 @@ class TimelineDragController { /** * Whether the grid itself now draws the settled drop at its landing slot. - * The copy may only be handed back once this is true: releasing on a timer - * puts the source ghost back at full opacity in its *old* slot for whatever - * is left of the re-read — a flicker of the event where it no longer is. + * The copy may only be handed back once this is true, or the source ghost + * flashes back to full opacity in its old slot. */ var settledOnGrid: Boolean by mutableStateOf(false) private set @@ -168,10 +158,8 @@ class TimelineDragController { private var pointer = Offset.Zero /** - * The slot the block already occupied when it was picked up. A long press - * that never moves must write nothing — and because the target snaps to the - * grid, "nothing moved" is not the same as "the start is unchanged": an - * event at 09:07 resolves to 09:00 the instant it lifts. + * The slot the block already occupied when it was picked up — not the same + * as its start, since the target snaps to the grid (09:07 lifts to 09:00). */ private var originSlot: Pair? = null @@ -205,9 +193,8 @@ class TimelineDragController { /** * Whether [block] is the one whose copy is in flight, and so must stay a - * ghost. While the finger holds it that is the instance it picked up; once - * dropped it is also whatever now sits in the slot it left, however the - * provider has renumbered it in the meantime. + * ghost: the instance the finger picked up, or once dropped whatever now + * sits in the slot it left. */ fun ghosts(block: TimedBlock, date: LocalDate): Boolean { if (liftedInstanceId == null) return false @@ -221,8 +208,7 @@ class TimelineDragController { /** * 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 - * or its title: a single-occurrence move writes an exception row with a new - * `eventId`, which nothing else about the drop can predict. + * or its title, since a single-occurrence move writes a new `eventId`. */ fun noteGrid(date: LocalDate, blocks: List) { val landed = settling ?: return @@ -254,9 +240,8 @@ class TimelineDragController { /** * Stop ghosting the source — the grid draws the drop itself by now — while - * the copy is still on screen dissolving. Un-ghosting only at [release] - * would leave the block under the fading copy dim, and brighten it once the - * copy was gone: a dip the eye reads as the event flickering. + * the copy is still dissolving. Un-ghosting at [release] instead would leave + * the block dim under the fading copy and then brighten it: a visible dip. */ fun handOver() { liftedInstanceId = null @@ -286,8 +271,7 @@ class TimelineDragController { val origin = grid.positionInRoot() val rawMinutes = (pointer.y - grab.y - origin.y) / hourPx * 60f val snapped = (rawMinutes / DRAG_SNAP_MINUTES).roundToInt() * DRAG_SNAP_MINUTES - // Clamp the start into the target day; a tail running past midnight is - // fine and stays visible on the next day. + // 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 span = block.endMin - block.startMin // The column the finger is over on screen, and the day that column shows. @@ -312,8 +296,8 @@ class TimelineDragController { /** * Hold the scroll for the whole drag and nudge it once per frame while the - * finger sits near an edge. One `scroll` call, not a loop of - * `animateScrollBy`: each of those re-acquires the mutex and cancels the last. + * finger sits near an edge. One `scroll` call, since a loop of + * `animateScrollBy` would re-acquire the mutex and cancel the last. */ suspend fun autoScroll() { val scroll = geometry.scroll ?: return @@ -374,17 +358,15 @@ fun TimelineDrop.startInstant(zone: TimeZone): Instant = const val SETTLE_GRACE_MILLIS: Long = 60L /** - * How long to wait for a grid that never confirms the drop — the event landed on - * a day this timeline doesn't show, or the write failed and nothing changed. The - * copy has to go either way. + * How long to wait for a grid that never confirms the drop — it landed on a day + * this timeline doesn't show, or the write failed. The copy has to go either way. */ private const val SETTLE_TIMEOUT_MILLIS = 900L /** * The hand-over: the copy dissolves over this while the grid's own block slides - * in under it, so the two overlap rather than one replacing the other. Kept - * short, because a full-width copy sits over any neighbour it now shares a lane - * with until it is gone. + * in under it. Kept short, because a full-width copy sits over any neighbour it + * now shares a lane with until it is gone. */ const val SETTLE_FADE_MILLIS: Int = 250 @@ -409,9 +391,9 @@ fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = LaunchedEffect(controller.isDragging) { if (controller.isDragging) controller.autoScroll() } - // Hold the landed copy until the write is done — including the whole time a + // Hold the landed copy until the write is done — including the time a // recurring drop's scope dialog is up — and then until the grid draws the - // drop itself, so the copy dissolves onto a block that is already there. + // drop itself. var handingOver by remember(controller.settling) { mutableStateOf(false) } LaunchedEffect(controller.settling, moveInFlight, controller.settledOnGrid) { if (controller.settling == null || moveInFlight) return@LaunchedEffect @@ -425,14 +407,13 @@ fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = Box( modifier = modifier .fillMaxSize() - // A copy of a block that is still in the tree behind it; announcing - // it again would just duplicate the event for the drag's duration. + // A copy of a block still in the tree behind it; announcing it again + // would duplicate the event for the drag's duration. .clearAndSetSemantics { } .onGloballyPositioned { origin = it.positionInRoot() }, ) { val drag = controller.drag ?: controller.settling ?: return@Box - // Landed: the copy sinks back to the grid's own plane while the write - // runs, so the release reads as the block settling rather than vanishing. + // Landed: the copy sinks back to the grid's own plane while the write runs. val landed = controller.drag == null val lift by animateFloatAsState( targetValue = if (landed || reduceMotion) 0f else 1f, diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/day/DayScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/day/DayScreen.kt index 366a7da..5d98f13 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/day/DayScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/day/DayScreen.kt @@ -649,8 +649,7 @@ private fun DayColumnCard( val minEventHeight = hourHeight * MIN_EVENT_FRACTION // Keyed by event, so a block that changes time or lane is the *same* // composable afterwards and tweens there. The ordinal disambiguates - // the rare column holding two occurrences of one series, which would - // otherwise be two blocks under one key. + // a column holding two occurrences of one series. val ordinals = mutableMapOf() blocks.forEach { block -> val ordinal = ordinals.merge(block.event.eventId, 1, Int::plus)!! - 1 @@ -723,8 +722,8 @@ private fun EventBlock( val fill = eventFill(block.event.color, dark, soften) val zone = remember { TimeZone.currentSystemDefault() } val moveAction = eventMoveAction(block.event) - // A block clipped at the top continues from the previous day: its top edge is - // midnight, not the event's start, so dragging it would invent a time. + // A block clipped at the top continues from the previous day: its top edge + // is midnight, not the event's start, so dragging it would invent a time. val draggable = moveAction != null && block.beginsOn(date, zone) val dragModifier = rememberEventDragSource( enabled = draggable, @@ -738,8 +737,7 @@ private fun EventBlock( val ghost = ghostAlpha(lifted) Box( modifier = modifier - // The source stays put as a ghost while its floating copy travels, - // then fades out as the copy settles on its new slot. + // The source stays put as a ghost while its floating copy travels. .then(if (ghost < 1f) Modifier.alpha(ghost) else Modifier) .background(fill, RoundedCornerShape(4.dp)) .clickable(onClick = onClick) 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 ee42ad3..750df47 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 @@ -28,8 +28,7 @@ class MonthRowGeometry( val laneCount: Int, /** * Whether the columns are laid out right-to-left. Pointer coordinates are - * never mirrored, but the grid is, so the leftmost column is the *last* day - * in Arabic. + * never mirrored but the grid is, so the mapping has to flip with it. */ val isRtl: Boolean, /** What this row currently draws in [lane] of column `col`, or null. */ @@ -57,10 +56,9 @@ class MonthRowGeometry( } /** - * Whether [chip] is the moved event [moved] as the grid now holds it. Neither - * id alone will do: the provider hands re-read instances new instance ids, and a - * single-occurrence move writes an exception row with a new event id — the title - * is what survives both. + * Whether [chip] is the moved event [moved] as the grid now holds it. Neither id + * alone will do: re-read instances get new instance ids, and a single-occurrence + * move writes a new event id — the title survives both. */ private fun isSameEvent(chip: EventInstance, moved: EventInstance): Boolean = chip.eventId == moved.eventId || chip.title == moved.title @@ -83,14 +81,10 @@ data class MonthChipDrop( ) /** - * Hoisted drag state for the month grid (#68). - * - * Every visible week row registers itself, so a chip can be carried across rows: - * the pointer node that took the press keeps receiving events after the finger - * has left its own bounds, and the target is then resolved against whichever - * registered row the finger is actually over. Rows are keyed by an identity token - * rather than by their date, because the continuous style can show the same week - * twice — once in each adjoining month. + * Hoisted drag state for the month grid (#68). Every visible week row registers + * itself so a chip can be carried across rows, the target resolved against + * whichever registered row the finger is over. Rows are keyed by an identity + * token rather than a date: the continuous style can show the same week twice. */ @Stable class MonthDragController { @@ -100,19 +94,16 @@ class MonthDragController { private set /** - * A dropped chip, held on the day it landed on while the write runs. The grid - * behind it still shows the old day until the provider notifies and the query - * re-reads, so releasing at drop time would snap the event back for the - * length of the write. + * A dropped chip, held on the day it landed on while the write runs — the + * grid behind it still shows the old day until the query re-reads. */ var settling: MonthChipDrag? by mutableStateOf(null) private set /** * Which chip is lifted, and whether anything is. Separate snapshot state from - * [drag] on purpose: [drag] changes on every frame, while the chips and rows - * that only need "am I the ghost" / "must I stop clipping" must not. Stays - * set through [settling], so the source never reappears under the copy. + * [drag], which changes every frame, so the chips and rows that only need + * "am I the ghost" don't recompose that often. Stays set through [settling]. */ var liftedInstanceId: Long? by mutableStateOf(null) private set @@ -120,20 +111,16 @@ class MonthDragController { /** * Where the grid itself now draws the drop, in root coordinates — published * by whichever week row ends up holding it. The copy is dropped wherever the - * finger was, but the grid seats it in a lane; without this the copy waits - * out a flat hold beside the chip the grid has already drawn and then - * cross-fades across the gap, which reads as the event taking its time to - * arrive. With it the copy glides onto its seat and hands over there. + * finger was while the grid seats it in a lane, so without this it would + * cross-fade across the gap instead of gliding onto its seat. */ var settledInRoot: Offset? by mutableStateOf(null) private set /** * Whether the drop's own chip is still standing in for the copy. Cleared at - * [handOver] rather than at [release], for the same reason [liftedInstanceId] - * is: held to the end it would keep the chip dim under a copy that has - * already faded, and brighten it afterwards — a dip the eye reads as the - * event flickering. + * [handOver] rather than [release], or the chip stays dim under a copy that + * has already faded and brightens afterwards: a visible dip. */ private var settledGhost: Boolean by mutableStateOf(false) @@ -143,8 +130,7 @@ class MonthDragController { /** * The last drop this controller made, kept past [release] for [beginUndo] — - * the confirmation chip carries Undo for four seconds after the copy is long - * gone. Not snapshot state: nothing draws from it. + * the confirmation chip carries Undo long after the copy is gone. */ private var undoable: MonthChipDrag? = null @@ -199,10 +185,6 @@ class MonthDragController { /** * 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. - * Matched on the landing day plus either the event row or its title: a - * single-occurrence move writes an exception row with a new `eventId`, and - * the provider hands re-read instances new ids, so neither alone identifies - * the chip the drop became. */ fun isSettledChip(event: EventInstance, days: List?): Boolean { val landed = settling?.takeIf { settledGhost } ?: return false @@ -211,9 +193,8 @@ class MonthDragController { } /** - * Ask the row keyed [token] whether it now seats the settled chip, and take - * its answer. The continuous style can show the same week twice, once in each - * adjoining month, so both copies of the landing row will answer: the seat + * Ask the row keyed [token] whether it now seats the settled chip. Both + * copies of a week the continuous style shows twice will answer; the seat * nearest where the chip was let go is the one the finger was over. */ fun noteSettled(token: Any) { @@ -227,15 +208,10 @@ class MonthDragController { } /** - * Send a copy back for an undo, which moves the event exactly as the drop did - * and so should read the same way rather than teleporting the chip. Nothing - * here writes anything: it puts a chip on the journey the inverse write is - * about to make, and the settle that follows is the drop's own. - * - * False when there is nothing to carry — no drop of this controller's to undo - * (it happened in another view, or has already been undone), or the grid does - * not seat the moved event where it would have to start from. The chip then - * simply reappears on the day it came from, as it always did. + * Put a chip on the journey an undo's inverse write is about to make, so it + * travels back rather than teleporting. Writes nothing itself. False when + * 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 { val last = undoable ?: return false @@ -251,8 +227,8 @@ class MonthDragController { /** * End the drag. A real drop keeps its chip 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 rather than wherever the finger happened to be. + * until [release], slid over to that day's column so it lands where the grid + * is about to draw it. */ fun finish(): MonthChipDrop? { val landed = drag @@ -270,8 +246,7 @@ class MonthDragController { /** * Stop ghosting the source while the copy is still dissolving — un-ghosting - * only at [release] leaves the chip under it dim and then brightens it, a - * dip the eye reads as the event flickering. + * at [release] instead leaves the chip dim and then brightens it. */ fun handOver() { liftedInstanceId = null @@ -312,8 +287,8 @@ class MonthDragController { drag = MonthChipDrag( event = event, grabDate = grabbed, - // Off the grid entirely (the gutter, the header): keep the last cell - // it was over, so a wobble past the edge doesn't drop the target. + // Off the grid entirely: keep the last cell it was over, so a + // wobble past the edge doesn't drop the target. targetDate = resolved ?: drag?.targetDate, topLeftInRoot = pointer - grab, sizePx = sizePx, 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 59fae40..be5ec50 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 @@ -415,12 +415,10 @@ fun MonthScreen( }, ) { innerPadding -> // Hoisted above every style's grid: a dragged chip is drawn in an - // overlay so it is free of the week row's clip and, in the scrolling - // styles, of the list viewport's. + // overlay, clear of the week row's and the list viewport's clips. val chipDrag = rememberMonthDragController() - // Undo moves the event back, so the chip travels back too rather than - // reappearing on its old day. The signal comes from the write, not - // from the chip that offers it: that lives above every view. + // 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() } Box( modifier = Modifier @@ -477,8 +475,7 @@ fun MonthScreen( /** * How long to wait for a grid that never seats the drop — it landed in a day's - * "+N" overflow, or on a day this month doesn't show, or the write failed and - * nothing changed. The copy has to go either way. + * "+N" overflow or on a day this month doesn't show, or the write failed. */ private const val MONTH_SETTLE_TIMEOUT_MILLIS = 450L @@ -493,11 +490,10 @@ private fun MonthDragOverlay(controller: MonthDragController) { // 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) - // Hold the landed chip until the write is done — including the whole time a + // Hold the landed chip until the write is done — including the time a // recurring drop's scope dialog is up — and then, unlike the timeline, carry - // it to its seat: a chip is dropped at whatever height the finger was at, - // while the grid seats it in a lane, so the copy has a gap to close before - // the two are the same chip and the hand-over can be invisible. + // it to its seat: it was dropped at whatever height the finger was at, while + // the grid seats it in a lane. var handingOver by remember(controller.settling) { mutableStateOf(false) } var gliding by remember(controller.settling) { mutableStateOf(false) } val glide = remember { Animatable(Offset.Zero, Offset.VectorConverter) } @@ -522,14 +518,13 @@ private fun MonthDragOverlay(controller: MonthDragController) { Box( modifier = Modifier .fillMaxSize() - // A copy of a chip that is still in the tree behind it; announcing it - // again would just duplicate the event for the drag's duration. + // A copy of a chip still in the tree behind it; announcing it again + // would duplicate the event for the drag's duration. .clearAndSetSemantics { } .onGloballyPositioned { origin = it.positionInRoot() }, ) { val drag = controller.drag ?: controller.settling ?: return@Box - // Landed: the copy sinks back to the grid's own plane while the write - // runs, so the release reads as the chip settling rather than vanishing. + // Landed: the copy sinks back to the grid's own plane while the write runs. val lift by animateFloatAsState( targetValue = if (controller.drag == null || reduceMotion) 0f else 1f, label = "chip-lift", @@ -1858,10 +1853,9 @@ private fun MonthWeekRow( val shownLanes = laneCount.coerceAtMost(MAX_EVENT_ROWS) val morphing = morphInFlight() - // Drag to reschedule (#68). The chips can take no pointer input of their own — - // the full-bleed tap layer below sits on top of them, and Compose stops - // sibling hit-testing at the topmost hit — so one detector on the row's day- - // column box hit-tests them geometrically instead. + // Drag to reschedule (#68). The chips can take no pointer input of their own + // — the full-bleed tap layer sits on top of them — so one detector on the + // row's day-column box hit-tests them geometrically instead. val moveScope = LocalEventMove.current val dragController = LocalMonthDrag.current val rowToken = remember { Any() } @@ -1874,10 +1868,9 @@ private fun MonthWeekRow( DisposableEffect(rowToken, dragController) { onDispose { dragController?.removeRow(rowToken) } } - // Republished on layout *and* on every recomposition: the coordinates only - // change on the former, but what the row draws — which the controller reads - // to find a moved chip's seat — changes on the latter, and a re-read that - // moves an event doesn't move the row it lands in. + // Republished on layout *and* on every recomposition: the coordinates change + // only on the former, but what the row draws — which the controller reads to + // find a moved chip's seat — changes on the latter. val publish = { val cell = cellCoordinates[0]?.takeIf { it.isAttached } if (dragController != null && cell != null) { @@ -1898,9 +1891,8 @@ private fun MonthWeekRow( } SideEffect { publish() } // 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 - // because that is what changes when the re-read lands: until then the row - // still draws the event on the day it came from and answers nothing. + // 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) { dragController?.noteSettled(rowToken) } @@ -2003,8 +1995,8 @@ private fun MonthWeekRow( bandCoordinates[0] = it publish() } - // A dragged chip travels to another row, so the clip has - // to yield for it exactly as it does for a morph. + // A dragged chip travels to another row, so the clip + // yields for it as it does for a morph. .then(if (morphing || dragging) Modifier else Modifier.clipToBounds()), ) { // Spanning bars on their shared lanes. @@ -2167,10 +2159,9 @@ private fun MonthWeekRow( } /** - * The row-level pickup for month chips: it resolves which chip the press landed - * on from the geometry the row just laid out, and abandons the gesture when the - * press was on empty space (or on an event whose calendar can't be moved), so - * tapping a day still opens it. + * The row-level pickup for month chips: resolves which chip the press landed on + * from the geometry the row just laid out, and abandons the gesture on empty + * space (or an unmovable calendar) so tapping a day still opens it. */ @Composable private fun monthChipDragModifier( @@ -2216,8 +2207,7 @@ private fun monthChipDragModifier( onDrop = { controller?.finish()?.let { drop -> // How many columns the finger crossed — grabbing the middle of a - // multi-day bar shifts the event by what the finger 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() if (delta != 0) { moveScope?.move( @@ -2328,10 +2318,8 @@ private fun MonthBar( modifier: Modifier = Modifier, /** * The days this chip covers in its row, for the drag (#68). A drop's own chip - * has to ghost like the source it came from until the copy has landed on it, - * and the instance id can't find it: the provider hands the re-read instance a - * new one, so the chip the drop became would otherwise sit at full opacity - * under the copy still travelling towards it — the event drawn twice. + * has to ghost until the copy lands on it, and the instance id can't find it: + * the provider hands the re-read instance a new one. */ days: List? = null, ) { @@ -2341,8 +2329,7 @@ private fun MonthBar( val soften = LocalSoftenColors.current val fill = eventFill(event.color, dark, soften) val moveAction = eventMoveAction(event) - // The source stays put as a ghost while its floating copy travels, then - // fades out as the copy settles on its new day. + // 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 diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthUiState.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthUiState.kt index bb9c629..56624d4 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthUiState.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthUiState.kt @@ -69,9 +69,6 @@ fun MonthWeek.laneEvents(col: Int, day: LocalDate, laneCap: Int): List val gap = with(density) { COLUMN_GAP.toPx() } dragController.geometry.let { @@ -797,8 +796,7 @@ private fun DayColumnCard( val minEventHeight = hourHeight * MIN_EVENT_FRACTION // Keyed by event, so a block that changes time or lane is the *same* // composable afterwards and tweens there. The ordinal disambiguates - // the rare column holding two occurrences of one series, which would - // otherwise be two blocks under one key. + // a column holding two occurrences of one series. val ordinals = mutableMapOf() blocks.forEach { block -> val ordinal = ordinals.merge(block.event.eventId, 1, Int::plus)!! - 1 @@ -892,8 +890,8 @@ private fun EventBlock( val fill = eventFill(block.event.color, dark, soften) val zone = remember { TimeZone.currentSystemDefault() } val moveAction = eventMoveAction(block.event) - // A block clipped at the top continues from the previous day: its top edge is - // midnight, not the event's start, so dragging it would invent a time. + // A block clipped at the top continues from the previous day: its top edge + // is midnight, not the event's start, so dragging it would invent a time. val draggable = moveAction != null && block.beginsOn(date, zone) val dragModifier = rememberEventDragSource( enabled = draggable, @@ -907,8 +905,7 @@ private fun EventBlock( val ghost = ghostAlpha(lifted) Box( modifier = (if (dimmed) modifier.alpha(EventDimAlpha) else modifier) - // The source stays put as a ghost while its floating copy travels, - // then fades out as the copy settles on its new slot. + // The source stays put as a ghost while its floating copy travels. .then(if (ghost < 1f) Modifier.alpha(ghost) else Modifier) .background(fill, RoundedCornerShape(4.dp)) .clickable(onClick = onClick) diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapperTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapperTest.kt index 0d96dd5..e8f4a75 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapperTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapperTest.kt @@ -288,9 +288,8 @@ class EventWriteMapperTest { @Test fun `a weekday move must carry the rule with the anchor, or the series stays put`() { - // The anchor moves by the same shift as the occurrence, so a Monday - // series lands on a Wednesday — while RRULE is written verbatim. Without - // realignRecurrence the rule would still say Monday and nothing moves. + // The anchor moves by the same shift as the occurrence, while RRULE is + // written verbatim: without realignRecurrence nothing moves. val series = instantAt("2026-01-05T09:00", "Europe/Berlin") val original = form( start = LocalDateTime(LocalDate(2026, 6, 8), LocalTime(9, 0)), diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/CalendarRowStateTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/CalendarRowStateTest.kt index 33b9ed4..b557dc2 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/domain/CalendarRowStateTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/CalendarRowStateTest.kt @@ -108,8 +108,7 @@ class CalendarRowStateTest { @Test fun `a switched-off calendar's events stay movable, unlike a new-event target`() { - // Nothing renders them, so the question is moot — but the predicate is - // deliberately not isEventTarget, which would also exclude them. + // The predicate is deliberately not isEventTarget. val hidden = cal().copy(isVisibleInSystem = false) assertThat(hidden.isEventTarget).isFalse() assertThat(hidden.allowsEventMove).isTrue() diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/EventShiftTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/EventShiftTest.kt index 4b2ee71..53c96ca 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/domain/EventShiftTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/EventShiftTest.kt @@ -67,8 +67,8 @@ class EventShiftTest { val moved = original.shiftedTo(target, berlin) - // Two real hours from 01:00 CET lands at 04:00 CEST, not 03:00: the wall - // clock stretches because the hour in between does not exist. + // Two real hours from 01:00 CET lands at 04:00 CEST: the hour in + // between does not exist. assertThat(moved.start).isEqualTo(LocalDateTime(LocalDate(2026, 3, 29), LocalTime(1, 0))) assertThat(moved.end).isEqualTo(LocalDateTime(LocalDate(2026, 3, 29), LocalTime(4, 0))) assertThat(moved.end.toInstant(berlin) - moved.start.toInstant(berlin)) @@ -131,9 +131,8 @@ class EventShiftTest { @Test fun `a timed shift onto a DST changeover keeps the real length, not the wall clock`() { - // 22:00 Sat -> 04:00 Sun is six real hours; the target Sunday is the one - // Berlin springs forward on. Keeping wall clock would write a five-hour - // DURATION for the whole series. + // 22:00 Sat -> 04:00 Sun is six real hours, onto the Sunday Berlin + // springs forward on. Keeping wall clock would write five. val overnight = form( start = LocalDateTime(LocalDate(2026, 3, 21), LocalTime(22, 0)), end = LocalDateTime(LocalDate(2026, 3, 22), LocalTime(4, 0)), diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/RecurrenceRealignTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/RecurrenceRealignTest.kt index 6832bbd..5b9b590 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/domain/RecurrenceRealignTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/RecurrenceRealignTest.kt @@ -26,8 +26,7 @@ class RecurrenceRealignTest { fun `a day-of-month rule is refused, because the anchor moves by days not dates`() { // BYMONTHDAY=28 with a January anchor, occurrence Feb 28 dragged to Mar 1: // the rebuilt rule would say the 1st while the anchor became Jan 29 — a - // DTSTART that is not an instance of its own rule. Weekday arithmetic is - // uniform mod 7 and survives the same shift; day-of-month is not. + // DTSTART that is not an instance of its own rule. assertThat(realignRecurrence("FREQ=MONTHLY;BYMONTHDAY=8", monday, wednesday)).isNull() assertThat( realignRecurrence("FREQ=YEARLY;BYMONTH=6;BYMONTHDAY=8", monday, LocalDate(2026, 7, 20)), @@ -36,8 +35,7 @@ class RecurrenceRealignTest { @Test fun `BYDAY on a non-weekly rule is refused`() { - // "every Monday of the month" is a shape this has not been reasoned about, - // and parseSimpleRecurrence can't read it either — so the UNTIL guard + // parseSimpleRecurrence can't read it either, so the UNTIL guard // downstream would be blind to it. assertThat(realignRecurrence("FREQ=MONTHLY;BYDAY=MO", monday, wednesday)).isNull() } @@ -86,8 +84,7 @@ class RecurrenceRealignTest { @Test fun `everything realignable is also a rule the UNTIL guard can read`() { // problems() checks UNTIL through parseSimpleRecurrence; a rule this - // realigns but that parser rejects would move a series past its own end - // unchecked. Accepting only weekly BYDAY keeps the two in step. + // realigns but that parser rejects would go unchecked. val realignable = listOf( "FREQ=WEEKLY;BYDAY=MO", "FREQ=WEEKLY;INTERVAL=2;BYDAY=MO;UNTIL=20261231T225959Z", 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 8675914..68632e5 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 @@ -45,10 +45,8 @@ class RescheduleViewModelTest { @BeforeEach fun setUp() = Dispatchers.setMain(dispatcher) @AfterEach fun tearDown() = Dispatchers.resetMain() - // Monday 2026-06-08, midday in whatever zone the test JVM runs in — resolved - // in the device zone rather than UTC, because that is the zone the drop path - // reads dates back in, and at UTC+13 a UTC midday is already Tuesday. The - // BYDAY assertions below depend on this really being a Monday. + // Monday 2026-06-08, midday in the device zone — the zone the drop path + // reads dates back in. The BYDAY assertions depend on it being a Monday. private val monday = LocalDate(2026, 6, 8) private val beginMillis = LocalDateTime(monday, LocalTime(12, 0)) .toInstant(TimeZone.currentSystemDefault()) @@ -282,8 +280,7 @@ class RescheduleViewModelTest { vm.move(toWednesday()) advanceUntilIdle() - // The block belongs to the write, not to the drop: an exception row is - // constrained by no UNTIL, so this scope must still be on offer. + // An exception row is constrained by no UNTIL, so the scope stays on offer. assertThat(vm.scopePrompt.value).isNotNull() vm.moveWithScope(RecurringWriteScope.ThisEvent) @@ -302,9 +299,8 @@ class RescheduleViewModelTest { } val vm = viewModel(tempDir, fake) - // Onto the next day *and* two hours later: the anchor moves by the same - // wall-clock shift, so an anchor late enough in the day would cross two - // midnights and land on a weekday the rebuilt rule doesn't name. + // Onto the next day *and* two hours later: a late enough anchor would + // cross two midnights under the same wall-clock shift. vm.move( MoveRequest( eventId = 42L,