fix(agenda): share the day-aware time label with the widget
Code-review follow-up. - The multi-day expansion in groupAgendaDays is shared with the agenda widget, but only the screen's summary was made day-aware — so the widget rendered the raw "start – end" on every spanned day, the very bug the screen fix cured. Hoist a pure agendaTimeLabel(event, day, zone) into the shared agenda layer and resolve strings from it in both the screen and the widget, so they label identically. (findings 1, 2) - groupAgendaDays could silently drop an instance whose clamped span was empty (firstDay > lastDay); floor lastDay at firstDay so a returned instance always surfaces on at least its first visible day. (finding 3) - agendaTimeLabel resolves the span days once instead of the summary recomputing them 2–3× per row. (finding 5) Finding 4 (within-day sort) needs no change: sorting by absolute start already places a still-running multi-day event at the top of each day it continues into, which is chronologically correct (it is ongoing from that day's midnight), and at its real start time on its first day. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -569,23 +569,17 @@ private fun agendaDayLabel(date: LocalDate, today: LocalDate): String {
|
||||
*/
|
||||
@Composable
|
||||
private fun agendaTimeSummary(event: EventInstance, day: LocalDate): String {
|
||||
val allDayLabel = stringResource(R.string.event_detail_all_day)
|
||||
val is24Hour = LocalUse24HourFormat.current
|
||||
val locale = currentLocale()
|
||||
|
||||
val time = if (event.spansMultipleDays(zone)) {
|
||||
val isFirstDay = day <= event.spanFirstDay(zone)
|
||||
val isLastDay = day >= event.spanLastDay(zone)
|
||||
when {
|
||||
event.isAllDay -> allDayLabel
|
||||
isFirstDay -> stringResource(R.string.agenda_span_starts, formatTime(event.start, is24Hour, locale))
|
||||
isLastDay -> stringResource(R.string.agenda_span_ends, formatTime(event.end, is24Hour, locale))
|
||||
else -> allDayLabel // a full middle day
|
||||
}
|
||||
} else if (event.isAllDay) {
|
||||
allDayLabel
|
||||
} else {
|
||||
"${formatTime(event.start, is24Hour, locale)} – ${formatTime(event.end, is24Hour, locale)}"
|
||||
val time = when (val label = agendaTimeLabel(event, day, zone)) {
|
||||
AgendaTimeLabel.AllDay -> stringResource(R.string.event_detail_all_day)
|
||||
is AgendaTimeLabel.Starts ->
|
||||
stringResource(R.string.agenda_span_starts, formatTime(label.start, is24Hour, locale))
|
||||
is AgendaTimeLabel.Ends ->
|
||||
stringResource(R.string.agenda_span_ends, formatTime(label.end, is24Hour, locale))
|
||||
is AgendaTimeLabel.Range ->
|
||||
"${formatTime(label.start, is24Hour, locale)} – ${formatTime(label.end, is24Hour, locale)}"
|
||||
}
|
||||
|
||||
val location = event.location?.takeIf { it.isNotBlank() }
|
||||
|
||||
@@ -8,6 +8,7 @@ import kotlinx.datetime.TimeZone
|
||||
import kotlinx.datetime.plus
|
||||
import kotlinx.datetime.toLocalDateTime
|
||||
import kotlin.time.Duration.Companion.milliseconds
|
||||
import kotlin.time.Instant
|
||||
|
||||
/**
|
||||
* The zone the event's calendar dates live in. Timed events are resolved in the
|
||||
@@ -36,6 +37,36 @@ fun EventInstance.spanLastDay(zone: TimeZone): LocalDate {
|
||||
fun EventInstance.spansMultipleDays(zone: TimeZone): Boolean =
|
||||
spanFirstDay(zone) != spanLastDay(zone)
|
||||
|
||||
/**
|
||||
* What an agenda row's time line should convey for an event on a given day —
|
||||
* the part of a multi-day span that [day] falls in. Pure and shared so the
|
||||
* agenda screen and the agenda widget label multi-day events identically; each
|
||||
* surface only formats the instants into its own locale/24h string.
|
||||
*/
|
||||
sealed interface AgendaTimeLabel {
|
||||
/** An all-day event, or a whole in-between day of a multi-day span. */
|
||||
data object AllDay : AgendaTimeLabel
|
||||
/** The first day of a multi-day timed event: when it begins. */
|
||||
data class Starts(val start: Instant) : AgendaTimeLabel
|
||||
/** The last day of a multi-day timed event: when it ends. */
|
||||
data class Ends(val end: Instant) : AgendaTimeLabel
|
||||
/** A single-day timed event: its start–end range. */
|
||||
data class Range(val start: Instant, val end: Instant) : AgendaTimeLabel
|
||||
}
|
||||
|
||||
/** The [AgendaTimeLabel] for [event] as it appears on [day], resolved in [zone]. */
|
||||
fun agendaTimeLabel(event: EventInstance, day: LocalDate, zone: TimeZone): AgendaTimeLabel {
|
||||
if (event.isAllDay) return AgendaTimeLabel.AllDay
|
||||
val firstDay = event.spanFirstDay(zone)
|
||||
val lastDay = event.spanLastDay(zone)
|
||||
return when {
|
||||
firstDay == lastDay -> AgendaTimeLabel.Range(event.start, event.end)
|
||||
day <= firstDay -> AgendaTimeLabel.Starts(event.start)
|
||||
day >= lastDay -> AgendaTimeLabel.Ends(event.end)
|
||||
else -> AgendaTimeLabel.AllDay // a full in-between day
|
||||
}
|
||||
}
|
||||
|
||||
/** One calendar day with at least one event, for the agenda list. */
|
||||
data class AgendaDay(
|
||||
val date: LocalDate,
|
||||
@@ -63,7 +94,10 @@ fun groupAgendaDays(
|
||||
val byDay = sortedMapOf<LocalDate, MutableList<EventInstance>>()
|
||||
for (instance in instances) {
|
||||
val firstDay = instance.spanFirstDay(zone).coerceAtLeast(anchor)
|
||||
val lastDay = instance.spanLastDay(zone).coerceAtMost(windowEnd)
|
||||
// Never below firstDay: an instance the query returned always surfaces on
|
||||
// at least its first visible day, even if its end resolves earlier (e.g. a
|
||||
// zero-length or boundary instant) — otherwise the loop would drop it.
|
||||
val lastDay = instance.spanLastDay(zone).coerceAtMost(windowEnd).coerceAtLeast(firstDay)
|
||||
var day = firstDay
|
||||
while (day <= lastDay) {
|
||||
byDay.getOrPut(day) { mutableListOf() }.add(instance)
|
||||
|
||||
@@ -49,6 +49,8 @@ import de.jeanlucmakiola.calendula.data.prefs.parsePastEventDisplay
|
||||
import de.jeanlucmakiola.calendula.domain.EventInstance
|
||||
import de.jeanlucmakiola.calendula.domain.hasEnded
|
||||
import de.jeanlucmakiola.calendula.ui.agenda.AgendaRange
|
||||
import de.jeanlucmakiola.calendula.ui.agenda.AgendaTimeLabel
|
||||
import de.jeanlucmakiola.calendula.ui.agenda.agendaTimeLabel
|
||||
import de.jeanlucmakiola.calendula.ui.agenda.anchorTodayIfMissing
|
||||
import de.jeanlucmakiola.calendula.ui.agenda.dayCount
|
||||
import de.jeanlucmakiola.calendula.ui.agenda.parseAgendaRange
|
||||
@@ -127,7 +129,7 @@ class RefreshAgendaAction : ActionCallback {
|
||||
/** Flat row model so the [LazyColumn] can mix day headers and events. */
|
||||
private sealed interface AgendaRow {
|
||||
data class Header(val date: LocalDate, val today: LocalDate) : AgendaRow
|
||||
data class Event(val event: EventInstance) : AgendaRow
|
||||
data class Event(val date: LocalDate, val event: EventInstance) : AgendaRow
|
||||
/** "Nothing left today" line under an anchored, event-less today (#35). */
|
||||
data class Placeholder(val date: LocalDate) : AgendaRow
|
||||
}
|
||||
@@ -185,7 +187,7 @@ private fun AgendaWidgetBody(data: AgendaWidgetData, dark: Boolean) {
|
||||
if (day.events.isEmpty()) {
|
||||
add(AgendaRow.Placeholder(day.date))
|
||||
} else {
|
||||
day.events.forEach { add(AgendaRow.Event(it)) }
|
||||
day.events.forEach { add(AgendaRow.Event(day.date, it)) }
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -196,6 +198,7 @@ private fun AgendaWidgetBody(data: AgendaWidgetData, dark: Boolean) {
|
||||
is AgendaRow.Placeholder -> PlaceholderRow(row.date)
|
||||
is AgendaRow.Event -> EventRow(
|
||||
event = row.event,
|
||||
day = row.date,
|
||||
dark = dark,
|
||||
soften = data.soften,
|
||||
is24Hour = data.is24Hour,
|
||||
@@ -312,6 +315,7 @@ private fun PlaceholderRow(date: LocalDate) {
|
||||
@Composable
|
||||
private fun EventRow(
|
||||
event: EventInstance,
|
||||
day: LocalDate,
|
||||
dark: Boolean,
|
||||
soften: Boolean,
|
||||
is24Hour: Boolean,
|
||||
@@ -357,7 +361,7 @@ private fun EventRow(
|
||||
style = TextStyle(color = titleColor, fontSize = 14.sp),
|
||||
)
|
||||
Text(
|
||||
text = eventTimeSummary(context, event, is24Hour),
|
||||
text = eventTimeSummary(context, event, day, is24Hour),
|
||||
maxLines = 1,
|
||||
style = TextStyle(color = GlanceTheme.colors.onSurfaceVariant, fontSize = 12.sp),
|
||||
)
|
||||
@@ -396,11 +400,20 @@ private fun agendaDayLabel(context: Context, date: LocalDate, today: LocalDate):
|
||||
return if (relative != null) "$relative · $formatted" else formatted
|
||||
}
|
||||
|
||||
private fun eventTimeSummary(context: Context, event: EventInstance, is24Hour: Boolean): String {
|
||||
val time = if (event.isAllDay) {
|
||||
context.getString(R.string.event_detail_all_day)
|
||||
} else {
|
||||
"${formatTime(event.start, is24Hour)} – ${formatTime(event.end, is24Hour)}"
|
||||
private fun eventTimeSummary(
|
||||
context: Context,
|
||||
event: EventInstance,
|
||||
day: LocalDate,
|
||||
is24Hour: Boolean,
|
||||
): String {
|
||||
val time = when (val label = agendaTimeLabel(event, day, zone())) {
|
||||
AgendaTimeLabel.AllDay -> context.getString(R.string.event_detail_all_day)
|
||||
is AgendaTimeLabel.Starts ->
|
||||
context.getString(R.string.agenda_span_starts, formatTime(label.start, is24Hour))
|
||||
is AgendaTimeLabel.Ends ->
|
||||
context.getString(R.string.agenda_span_ends, formatTime(label.end, is24Hour))
|
||||
is AgendaTimeLabel.Range ->
|
||||
"${formatTime(label.start, is24Hour)} – ${formatTime(label.end, is24Hour)}"
|
||||
}
|
||||
val location = event.location?.takeIf { it.isNotBlank() }
|
||||
return if (location != null) "$time · $location" else time
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
package de.jeanlucmakiola.calendula.ui.agenda
|
||||
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import de.jeanlucmakiola.calendula.domain.EventInstance
|
||||
import kotlinx.datetime.LocalDate
|
||||
import kotlinx.datetime.LocalDateTime
|
||||
import kotlinx.datetime.TimeZone
|
||||
import kotlinx.datetime.toInstant
|
||||
import kotlin.time.Instant
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
class AgendaTimeLabelTest {
|
||||
|
||||
private val zone = TimeZone.UTC
|
||||
|
||||
private fun at(y: Int, mo: Int, d: Int, h: Int = 0, min: Int = 0): Instant =
|
||||
LocalDateTime(y, mo, d, h, min).toInstant(zone)
|
||||
|
||||
private fun event(start: Instant, end: Instant, isAllDay: Boolean = false) = EventInstance(
|
||||
instanceId = 1,
|
||||
eventId = 1,
|
||||
calendarId = 1,
|
||||
title = "e",
|
||||
start = start,
|
||||
end = end,
|
||||
isAllDay = isAllDay,
|
||||
color = 0,
|
||||
location = null,
|
||||
)
|
||||
|
||||
private fun labelOn(y: Int, mo: Int, d: Int, event: EventInstance) =
|
||||
agendaTimeLabel(event, LocalDate(y, mo, d), zone)
|
||||
|
||||
@Test
|
||||
fun `a single-day timed event is a start-end range`() {
|
||||
val e = event(at(2026, 7, 2, 12, 0), at(2026, 7, 2, 13, 0))
|
||||
|
||||
assertThat(labelOn(2026, 7, 2, e))
|
||||
.isEqualTo(AgendaTimeLabel.Range(e.start, e.end))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a single-day all-day event is all-day`() {
|
||||
val e = event(at(2026, 7, 2), at(2026, 7, 3), isAllDay = true)
|
||||
|
||||
assertThat(labelOn(2026, 7, 2, e)).isEqualTo(AgendaTimeLabel.AllDay)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a multi-day timed event names the start, middle, and end days`() {
|
||||
val e = event(at(2026, 7, 1, 14, 0), at(2026, 7, 4, 10, 0))
|
||||
|
||||
assertThat(labelOn(2026, 7, 1, e)).isEqualTo(AgendaTimeLabel.Starts(e.start))
|
||||
assertThat(labelOn(2026, 7, 2, e)).isEqualTo(AgendaTimeLabel.AllDay)
|
||||
assertThat(labelOn(2026, 7, 3, e)).isEqualTo(AgendaTimeLabel.AllDay)
|
||||
assertThat(labelOn(2026, 7, 4, e)).isEqualTo(AgendaTimeLabel.Ends(e.end))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a multi-day all-day event is all-day on every day`() {
|
||||
val e = event(at(2026, 7, 2), at(2026, 7, 5), isAllDay = true)
|
||||
|
||||
assertThat(labelOn(2026, 7, 2, e)).isEqualTo(AgendaTimeLabel.AllDay)
|
||||
assertThat(labelOn(2026, 7, 3, e)).isEqualTo(AgendaTimeLabel.AllDay)
|
||||
assertThat(labelOn(2026, 7, 4, e)).isEqualTo(AgendaTimeLabel.AllDay)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an event begun before the shown day is not labelled as starting`() {
|
||||
// Runs 29 Jun 09:00 → 2 Jul 09:00; on 1 Jul it is mid-span, on 2 Jul it ends.
|
||||
val e = event(at(2026, 6, 29, 9, 0), at(2026, 7, 2, 9, 0))
|
||||
|
||||
assertThat(labelOn(2026, 7, 1, e)).isEqualTo(AgendaTimeLabel.AllDay)
|
||||
assertThat(labelOn(2026, 7, 2, e)).isEqualTo(AgendaTimeLabel.Ends(e.end))
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user