diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/RecurrenceOccurrences.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/RecurrenceOccurrences.kt index 0095c85..ca434cc 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/RecurrenceOccurrences.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/RecurrenceOccurrences.kt @@ -10,31 +10,18 @@ import kotlinx.datetime.plus /** * The first [limit] dates a [SimpleRecurrence] fires on, starting at [start] - * (the event's own start date, i.e. DTSTART), for showing a rule as dates - * instead of as words. + * (DTSTART), for previewing a rule as dates instead of as words. A preview only, + * kept to the shapes the picker can build; the provider stays the authority. * - * A recurrence rule is the one place in the editor where a correct-sounding - * phrase can still mean something the user didn't intend — "every 2 weeks on - * Mon, Fri", "monthly" on the 31st — and the only honest answer is the dates - * themselves. This is a *preview*, deliberately kept to the shapes the picker - * can build; the provider remains the authority on what actually gets stored - * and expanded. + * Mirrors RFC 5545: [start] is always the first occurrence (§3.8.5.3), even when + * the rule's own picks miss it; a monthly or yearly rule *skips* a period the + * start day doesn't exist in rather than clamping; a weekly rule repeats in + * blocks of `interval` weeks beginning on Monday (the default WKST, since + * [toRRule] never writes one); [RecurrenceEnd.Count] counts real occurrences and + * [RecurrenceEnd.Until] is inclusive. * - * The rules it mirrors, all from RFC 5545 (the spec the platform's recurrence - * engine implements): - * - a monthly or yearly rule **skips** a period the start day doesn't exist in, - * rather than sliding to the month's last day: the 31st recurring monthly - * lands only in 31-day months, 29 February yearly only in leap years. - * - a weekly rule with weekday picks repeats in blocks of `interval` weeks, - * with the week beginning on **Monday** — the RFC's default WKST, and the one - * that applies here because [toRRule] never writes a WKST part. The - * display-only "first day of week" preference deliberately doesn't enter into - * it, or the preview would disagree with the stored rule. - * - [RecurrenceEnd.Count] counts real occurrences (skipped periods don't - * consume one), [RecurrenceEnd.Until] is inclusive of its date. - * - * Returns fewer than [limit] dates when the series ends first, and an empty - * list when the rule yields nothing at all (an UNTIL before the start). + * Returns fewer than [limit] dates when the series ends first, and an empty list + * only when the rule yields nothing at all (an UNTIL before [start]). */ fun SimpleRecurrence.upcomingOccurrences(start: LocalDate, limit: Int): List { if (limit <= 0) return emptyList() @@ -42,15 +29,17 @@ fun SimpleRecurrence.upcomingOccurrences(start: LocalDate, limit: Int): List until) return emptyList() - val result = mutableListOf() + // DTSTART is in the recurrence set whatever the rule picks, so seed with it + // and let the walk skip anything landing on or before it. + val result = mutableListOf(start) var period = 0 - // Periods that produce nothing (a skipped 31st, a weekday block whose picks - // all fall before the start) must not stall the walk, so the cap is on - // periods examined rather than on dates found. + // Periods can yield nothing (a skipped 31st), so the cap counts periods + // examined rather than dates found. while (result.size < wanted && period < MAX_PERIODS) { for (date in occurrencesInPeriod(period, start)) { - if (date < start) continue + if (date <= start) continue if (until != null && date > until) return result result += date if (result.size == wanted) return result @@ -67,8 +56,8 @@ private fun SimpleRecurrence.occurrencesInPeriod(period: Int, start: LocalDate): RecurrenceFreq.Weekly -> weeklyOccurrences(period, start) RecurrenceFreq.Monthly -> { val month = start.plus(period * interval, DateTimeUnit.MONTH) - // plus() clamps the day into the shorter month; the rule instead - // skips such a period, so a clamped date means "not this month". + // plus() clamps into the shorter month, but the rule skips such a + // period — so a clamped date means "not this month". listOfNotNull(dateOrNull(month.year, month.month.number, start.day)) } RecurrenceFreq.Yearly -> @@ -96,13 +85,9 @@ private fun SimpleRecurrence.weeklyOccurrences(period: Int, start: LocalDate): L } /** - * Whether a run of [occurrences] starting at [start] leaves its starting year — - * i.e. whether showing them without a year would be ambiguous. - * - * Day and month alone are enough for a rule that stays inside one year, and - * dropping the year keeps the line short. They are actively misleading - * otherwise: a yearly rule reads as the same date three times over, and a - * monthly one rolling past December gives no hint that it has. + * Whether a run of [occurrences] starting at [start] leaves its starting year, + * i.e. whether showing them without a year would be ambiguous — a yearly rule + * would otherwise read as the same date repeated. */ fun occurrencesSpanYears(occurrences: List, start: LocalDate): Boolean = occurrences.any { it.year != start.year } || occurrences.map { it.year }.distinct().size > 1 @@ -114,9 +99,8 @@ private fun dateOrNull(year: Int, month: Int, day: Int): LocalDate? = private const val DAYS_PER_WEEK = 7 /** - * How many repetitions to examine before giving up. Generous enough for the - * sparsest rule the picker can build (29 February yearly with a 999-year - * interval is nonsense; 31st monthly needs at most a handful), and bounded so a - * rule whose occurrences all fall outside its own UNTIL can't spin. + * How many repetitions to examine before giving up: generous enough for the + * sparsest rule the picker can build, bounded so a rule whose occurrences all + * fall outside its own UNTIL can't spin. */ private const val MAX_PERIODS = 2_000 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 53f006d..41f3f21 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHost.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHost.kt @@ -149,9 +149,8 @@ fun CalendarHost( // over Settings and survives view switches. var showCalendars by rememberSaveable { mutableStateOf(false) } - // Backup & restore (#69) — like the manager, driven by the calendar list - // rather than by preferences, so it is hoisted here instead of living as a - // Settings sub-section. Reached from Settings and from the manager. + // Backup & restore (#69) — hoisted like the manager, being driven by the + // calendar list rather than by preferences. Reached from both. var showBackup by rememberSaveable { mutableStateOf(false) } // Event form (v1.2 create) — same held-key pattern as the detail screen: @@ -486,10 +485,8 @@ fun CalendarHost( ) } - // Calendar manager — declared last so it covers every overlay that can - // open it: Settings, both event forms, and the .ics import picker (#76). - // Coming back from it leaves the caller exactly as it was, with the - // calendar list already refreshed by the provider's notification. + // Declared last so it covers every overlay that can open it: Settings, + // both event forms, and the .ics import picker (#76). AnimatedVisibility( visible = showCalendars, enter = slideInHorizontally(slideSpec) { it } + fadeIn(), @@ -509,12 +506,15 @@ fun CalendarHost( ) { BackupScreen( onBack = { showBackup = false }, - // Restoring runs the normal .ics import, and that way round this - // screen has to step aside: declared above the import overlays, - // it would otherwise cover the screen it just asked for. Closing - // it hands the user back to whatever opened Backup once the - // import is done. - onImport = { importUri = it; importForceMany = true; showBackup = false }, + // Restore runs the normal .ics import, and both this screen and + // the manager that can have opened it are declared above the + // import overlays — so both have to step aside. + onImport = { + importUri = it + importForceMany = true + showBackup = false + showCalendars = false + }, ) } } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/agenda/AgendaRange.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/agenda/AgendaRange.kt index 474f48d..a8e5f39 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/agenda/AgendaRange.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/agenda/AgendaRange.kt @@ -92,20 +92,27 @@ fun parseAgendaRange(stored: String?, default: AgendaRange): AgendaRange = when * - [AgendaRange.ThisMonth] → month and year ("June 2026") * - everything else → "start – end" ("27 Jun – 3 Jul 2026"), with the start's * year shown too only when it differs from the end's. + * + * [monthAsSpan] puts [AgendaRange.ThisMonth] on the "start – end" form as well. + * The agenda's own header browses the whole month, so the month name is right + * there; the range picker previews the window an option opens *today*, which for + * "This month" is only the rest of it. */ fun agendaRangeWindowSummary( range: AgendaRange, start: LocalDate, end: LocalDate, locale: Locale, + monthAsSpan: Boolean = false, ): String { val javaStart = java.time.LocalDate.of(start.year, start.month.ordinal + 1, start.day) val javaEnd = java.time.LocalDate.of(end.year, end.month.ordinal + 1, end.day) val dayMonth = localizedDateFormatter(locale, "dMMM") val dayMonthYear = localizedDateFormatter(locale, "dMMMy") - return when (range) { - AgendaRange.Day -> dayMonthYear.format(javaStart) - AgendaRange.ThisMonth -> localizedDateFormatter(locale, "LLLLy").format(javaStart) + return when { + range == AgendaRange.Day -> dayMonthYear.format(javaStart) + range == AgendaRange.ThisMonth && !monthAsSpan -> + localizedDateFormatter(locale, "LLLLy").format(javaStart) else -> { val startFmt = if (start.year == end.year) dayMonth else dayMonthYear "${startFmt.format(javaStart)} – ${dayMonthYear.format(javaEnd)}" diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/Picker.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/Picker.kt index d0fe640..44ab0b1 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/Picker.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/Picker.kt @@ -59,10 +59,9 @@ import kotlin.time.Clock * lead time to the set. Changes apply live via [onSelect]; the user leaves via * back. * - * [leadTimeSummary] adds a second line to each lead-time row. It exists for the - * all-day pickers, where the lead time alone doesn't say when anything happens: - * the hour comes from the separate "show all-day reminders at" setting, so - * "1 day before" is only half the answer until the row spells out the time. + * [leadTimeSummary] adds a second line to each lead-time row, for the all-day + * pickers: the hour comes from the separate "show all-day reminders at" setting, + * so the lead time alone doesn't say when anything happens. */ @Composable fun ReminderDefaultPicker( @@ -233,10 +232,8 @@ internal fun PickerDescription(text: String) { * Mirrors [ReminderDefaultPicker]'s custom-expand pattern. * * Every option carries the dates it resolves to today as its summary, computed - * through the same [dayCount] the agenda windows by (hence [weekStart], which - * "This week" depends on). Without it the two groups are near-indistinguishable - * — "This week" and "Next 7 days" name different windows in the same words, and - * only the concrete span tells them apart. + * through the same [dayCount] the agenda windows by (hence [weekStart]) — only + * the concrete span tells "This week" and "Next 7 days" apart. */ @Composable fun AgendaRangePicker( @@ -260,11 +257,11 @@ fun AgendaRangePicker( val locale = currentLocale() val zone = remember { TimeZone.currentSystemDefault() } val today = remember(zone) { Clock.System.now().toLocalDateTime(zone).date } - // The window each option would open right now — the agenda's own end-day - // arithmetic (anchor + dayCount - 1), so the dates match what it will show. + // The agenda's own end-day arithmetic (anchor + dayCount - 1), so the dates + // match what it will show. val windowSummary: (AgendaRange) -> String = { range -> val end = today.plus(range.dayCount(today, weekStart) - 1, DateTimeUnit.DAY) - agendaRangeWindowSummary(range, today, end, locale) + agendaRangeWindowSummary(range, today, end, locale, monthAsSpan = true) } val rangeRow: @Composable (AgendaRange, Position) -> Unit = { option, position -> @@ -306,8 +303,8 @@ fun AgendaRangePicker( } else { stringResource(R.string.agenda_range_custom) }, - // Only a chosen custom window has dates to show; an unset one has no - // day count yet, so the row stays single-line until it does. + // An unset custom window has no day count yet, so the row stays + // single-line until it does. summary = if (customSelected) windowSummary(selected) else null, position = if (customExpanded) Position.Top else positionOf(rolling.size, rollingRowCount), selected = customSelected, diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/PastEventsPicker.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/PastEventsPicker.kt index 049ed11..4b34acb 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/PastEventsPicker.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/PastEventsPicker.kt @@ -4,6 +4,7 @@ import androidx.compose.animation.Crossfade import androidx.compose.animation.core.snap import androidx.compose.animation.core.tween import androidx.compose.foundation.background +import androidx.compose.foundation.gestures.detectTapGestures import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.fillMaxWidth @@ -17,7 +18,6 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.draw.clipToBounds -import androidx.compose.ui.input.pointer.PointerEventPass import androidx.compose.ui.input.pointer.pointerInput import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.clearAndSetSemantics @@ -41,19 +41,9 @@ import kotlinx.datetime.toLocalDateTime import kotlin.time.Clock /** - * The "past events" chooser for the Agenda (#69 follow-up). - * - * "Show / dim / hide" is the case where the word cannot carry the choice — - * *how far* does dimmed fade, and what is left behind when finished events are - * hidden? So a stand-in agenda day sits above the options and re-renders as you - * pick one, using the very rows the Agenda draws: dimming is - * [AgendaEventRow]'s own `dimmed` flag, and hiding drops the row exactly as - * [de.jeanlucmakiola.calendula.ui.agenda.AgendaScreen] filters it — so the - * preview cannot claim an effect the screen doesn't have. - * - * Like the Month style and App name pickers, selecting applies immediately and - * leaves the picker open: closing would hide the thing the screen is for. Back - * exits. + * The "past events" chooser for the Agenda (#69 follow-up). A stand-in agenda + * day sits above the options and re-renders as you pick one, using the Agenda's + * own rows and filtering. A preview picker, so it stays open on selection. */ @Composable internal fun PastEventsPicker( @@ -102,7 +92,6 @@ internal fun PastEventsPicker( } else { null }, - // Applies straight away; the preview above is the confirmation. onClick = { onSelect(mode) }, ) } @@ -111,9 +100,8 @@ internal fun PastEventsPicker( /** * A stand-in agenda day in a given [mode]: three events on today, the first two - * already finished. The times are fixed rather than derived from the clock, so - * the preview reads the same at any hour — what varies is only what [mode] does - * to the two finished rows. + * finished. Times are fixed rather than clock-derived, so it reads the same at + * any hour. */ @Composable private fun PastEventsPreview(mode: PastEventDisplay, modifier: Modifier = Modifier) { @@ -125,24 +113,17 @@ private fun PastEventsPreview(mode: PastEventDisplay, modifier: Modifier = Modif stringResource(R.string.settings_past_events_sample_evening), ) val sample = remember(today, zone, titles) { samplePastDay(today, zone, titles) } - // Hiding drops the finished rows outright; showing and dimming keep them, - // and only dimming fades them — the same three-way split AgendaContent makes. + // The same three-way split AgendaContent makes. val visible = if (mode == PastEventDisplay.HIDE) sample.filterNot { it.hasPassed } else sample Column( modifier = modifier .fillMaxWidth() .clipToBounds() - // A preview is a picture, not a control: swallow touches before the - // rows' own clickables see them, and keep it out of the reading order — - // the options below already say what is selected. - .pointerInput(Unit) { - awaitPointerEventScope { - while (true) { - awaitPointerEvent(PointerEventPass.Initial).changes.forEach { it.consume() } - } - } - } + // A picture, not a control: take the taps the fake rows would + // otherwise handle, and keep it out of the reading order. Taps only — + // consuming drags would stop the picker scrolling from here. + .pointerInput(Unit) { detectTapGestures { } } .clearAndSetSemantics { }, ) { AgendaDayHeader(date = today, today = today, onOpenDay = {}) @@ -163,11 +144,8 @@ private fun PastEventsPreview(mode: PastEventDisplay, modifier: Modifier = Modif private class SampleRow(val event: EventInstance, val hasPassed: Boolean) /** - * Today's stand-in events, [titles] in order: two finished (morning, midday) and - * one still to come. - * - * Colours are raw ARGB on purpose — that is what the provider hands out for an - * event, so a theme token here would misrepresent what the agenda renders. + * Today's stand-in events, [titles] in order: two finished and one to come. + * Colours are raw ARGB, as the provider hands them out. */ private fun samplePastDay(today: LocalDate, zone: TimeZone, titles: List): List = listOf( diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/RecurrenceOccurrencesTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/RecurrenceOccurrencesTest.kt index 50a6729..eff2c34 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/domain/RecurrenceOccurrencesTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/RecurrenceOccurrencesTest.kt @@ -53,13 +53,29 @@ class RecurrenceOccurrencesTest { byDays = setOf(DayOfWeek.FRIDAY, DayOfWeek.MONDAY), ).upcomingOccurrences(date(2026, 7, 30), limit = 4) - // Thursday start: the Monday of that week is already past, so the first - // hit is Friday 31 Jul, then Mon/Fri of the following weeks. + // The Thursday start is an occurrence in its own right (RFC 5545 puts + // DTSTART in the set), then Friday 31 Jul and Mon/Fri of the next week. assertThat(occurrences).containsExactly( + date(2026, 7, 30), date(2026, 7, 31), date(2026, 8, 3), date(2026, 8, 7), - date(2026, 8, 10), + ).inOrder() + } + + @Test + fun `the start counts once even when it is also one of the picks`() { + // Thursday start with Thursday picked: the seeded DTSTART and the rule's + // own hit are the same date and must not both be listed. + val occurrences = SimpleRecurrence( + RecurrenceFreq.Weekly, + byDays = setOf(DayOfWeek.THURSDAY), + ).upcomingOccurrences(date(2026, 7, 30), limit = 3) + + assertThat(occurrences).containsExactly( + date(2026, 7, 30), + date(2026, 8, 6), + date(2026, 8, 13), ).inOrder() }