fix: address code-review findings on the 2.16.0 branch #87

Merged
makiolaj merged 1 commits from fix/code-review-2.16.0 into release/v2.16.0 2026-07-20 11:52:21 +00:00
13 changed files with 324 additions and 64 deletions
Showing only changes of commit 94355bf340 - Show all commits

View File

@@ -6,9 +6,11 @@ import de.jeanlucmakiola.calendula.domain.Availability
import de.jeanlucmakiola.calendula.domain.EventForm import de.jeanlucmakiola.calendula.domain.EventForm
import kotlinx.datetime.toJavaLocalDate import kotlinx.datetime.toJavaLocalDate
import kotlinx.datetime.toJavaLocalDateTime import kotlinx.datetime.toJavaLocalDateTime
import java.time.Duration
import java.time.Instant import java.time.Instant
import java.time.ZoneId import java.time.ZoneId
import java.time.ZoneOffset import java.time.ZoneOffset
import java.time.LocalDateTime as JavaLocalDateTime
/** Provider-ready DTSTART / DTEND / EVENT_TIMEZONE for an event write. */ /** Provider-ready DTSTART / DTEND / EVENT_TIMEZONE for an event write. */
internal data class EventWriteTimes( internal data class EventWriteTimes(
@@ -35,7 +37,7 @@ internal fun EventForm.toWriteTimes(zone: ZoneId): EventWriteTimes = if (isAllDa
timezone = "UTC", timezone = "UTC",
) )
} else { } else {
val writeZone = timezone?.let { runCatching { ZoneId.of(it) }.getOrNull() } ?: zone val writeZone = writeZone(zone)
EventWriteTimes( EventWriteTimes(
dtStartMillis = start.toJavaLocalDateTime().atZone(writeZone).toInstant().toEpochMilli(), dtStartMillis = start.toJavaLocalDateTime().atZone(writeZone).toInstant().toEpochMilli(),
dtEndMillis = end.toJavaLocalDateTime().atZone(writeZone).toInstant().toEpochMilli(), dtEndMillis = end.toJavaLocalDateTime().atZone(writeZone).toInstant().toEpochMilli(),
@@ -43,6 +45,30 @@ internal fun EventForm.toWriteTimes(zone: ZoneId): EventWriteTimes = if (isAllDa
) )
} }
/**
* The zone this form's DTSTART is expressed in: UTC for an all-day event (the
* provider's date anchor), otherwise the form's own pinned zone, falling back to
* [deviceZone] when it doesn't pin one or pins something the tz database can't
* parse.
*/
private fun EventForm.writeZone(deviceZone: ZoneId): ZoneId = if (isAllDay) {
ZoneOffset.UTC
} else {
timezone?.let { runCatching { ZoneId.of(it) }.getOrNull() } ?: deviceZone
}
/**
* The form's start as a bare wall-clock value — what the user sees on the form,
* stripped of any zone. All-day events use their date's midnight rather than
* [EventForm.start]'s placeholder time-of-day, which exists only so switching the
* event back to timed has something to show.
*/
private fun EventForm.anchorLocal(): JavaLocalDateTime = if (isAllDay) {
start.date.toJavaLocalDate().atStartOfDay()
} else {
start.toJavaLocalDateTime()
}
/** /**
* RFC 2445 duration for a recurring event's row (the provider requires * RFC 2445 duration for a recurring event's row (the provider requires
* DURATION instead of DTEND when an RRULE is set): whole days for all-day * DURATION instead of DTEND when an RRULE is set): whole days for all-day
@@ -109,10 +135,12 @@ internal fun buildEventInsertValues(
* Time fields travel together (the provider validates them as a unit): * Time fields travel together (the provider validates them as a unit):
* - unchanged times, all-day flag and rrule → no time columns at all; * - unchanged times, all-day flag and rrule → no time columns at all;
* - non-recurring result → DTSTART/DTEND, DURATION and RRULE cleared; * - non-recurring result → DTSTART/DTEND, DURATION and RRULE cleared;
* - recurring result → the *series* DTSTART moves by the same delta the user * - recurring result → the *series* DTSTART moves by the same **wall-clock**
* applied to the displayed occurrence ([seriesDtStartMillis] is the row's * shift the user applied to the displayed occurrence and is re-resolved in the
* current DTSTART), DURATION replaces DTEND, RRULE is written. This keeps * event's zone ([seriesDtStartMillis] is the row's current DTSTART), DURATION
* past occurrences intact when someone edits a later occurrence's time. * replaces DTEND, RRULE is written. This keeps past occurrences intact when
* someone edits a later occurrence's time, and keeps the anchor's time-of-day
* stable across a DST boundary or a zone change between the two.
*/ */
internal fun buildEventUpdateValues( internal fun buildEventUpdateValues(
original: EventForm, original: EventForm,
@@ -158,8 +186,23 @@ internal fun buildEventUpdateValues(
put(CalendarContract.Events.RRULE, null) put(CalendarContract.Events.RRULE, null)
put(CalendarContract.Events.DURATION, null) put(CalendarContract.Events.DURATION, null)
} else { } else {
val startDelta = newTimes.dtStartMillis - original.toWriteTimes(zone).dtStartMillis // Move the series anchor by the *wall-clock* shift the user applied to the
put(CalendarContract.Events.DTSTART, seriesDtStartMillis + startDelta) // displayed occurrence, then re-resolve it in the event's (possibly new)
// zone — never by a millisecond delta. An instant delta silently bakes in
// the offset that happened to apply on the edited occurrence's date, which
// is a different offset from the series anchor's whenever a DST boundary
// sits between them, or whenever the zone itself changed. Working in wall
// clock keeps "09:00" meaning 09:00 at both ends.
val seriesLocal = Instant.ofEpochMilli(seriesDtStartMillis)
.atZone(original.writeZone(zone)).toLocalDateTime()
val wallClockShift = Duration.between(original.anchorLocal(), updated.anchorLocal())
val shifted = seriesLocal.plus(wallClockShift)
// An all-day series anchor must sit on a UTC midnight. A pure day move
// already lands there (both ends are midnights), but *switching* a
// recurring event to all-day shifts by a time-of-day too, so snap.
val newSeriesLocal = if (updated.isAllDay) shifted.toLocalDate().atStartOfDay() else shifted
val newSeriesStart = newSeriesLocal.atZone(updated.writeZone(zone))
put(CalendarContract.Events.DTSTART, newSeriesStart.toInstant().toEpochMilli())
put(CalendarContract.Events.DTEND, null) put(CalendarContract.Events.DTEND, null)
put(CalendarContract.Events.RRULE, updated.rrule) put(CalendarContract.Events.RRULE, updated.rrule)
put(CalendarContract.Events.DURATION, newTimes.toRfc2445Duration(updated.isAllDay)) put(CalendarContract.Events.DURATION, newTimes.toRfc2445Duration(updated.isAllDay))

View File

@@ -31,8 +31,33 @@ data class TimeZoneOption(
/** The leading segment ("Europe/Berlin" -> "Europe"); empty for bare ids like "UTC". */ /** The leading segment ("Europe/Berlin" -> "Europe"); empty for bare ids like "UTC". */
val region: String get() = id.substringBeforeLast('/', missingDelimiterValue = "") val region: String get() = id.substringBeforeLast('/', missingDelimiterValue = "")
/**
* The four fields [filterTimeZones] matches on, normalized once here rather
* than per query. Normalizing is not cheap — NFD decomposition plus a combining-
* mark strip — and a search re-examines every option on every keystroke, so
* doing it at construction turns ~2400 normalizations per character into a few
* hundred plain `startsWith`/`contains` calls.
*
* Declared in the class body, so it stays out of `equals`/`hashCode`/`copy`:
* it is derived state, and two options with the same id are the same option.
*/
internal val searchKeys: TimeZoneSearchKeys = TimeZoneSearchKeys(
city = city.normalizeForSearch(),
id = id.normalizeForSearch(),
displayName = displayName.normalizeForSearch(),
shortName = shortName.normalizeForSearch(),
)
} }
/** Pre-normalized match targets for one [TimeZoneOption]. */
internal data class TimeZoneSearchKeys(
val city: String,
val id: String,
val displayName: String,
val shortName: String,
)
private fun optionFor( private fun optionFor(
zone: ZoneId, zone: ZoneId,
locale: Locale, locale: Locale,
@@ -87,9 +112,13 @@ private fun resolveAbbreviation(
private fun String.looksLikeOffset(): Boolean = this == "UTC" || startsWith("GMT") private fun String.looksLikeOffset(): Boolean = this == "UTC" || startsWith("GMT")
/** /**
* Every zone the JVM knows, resolved at [at]. This is ~600 entries, so build it * Every zone the JVM knows, resolved at [at]. [regionOf] (see
* once and filter the result rather than rebuilding per keystroke. [regionOf] * [resolveAbbreviation]) supplies the zone's region for the abbreviation.
* (see [resolveAbbreviation]) supplies the zone's region for the abbreviation. *
* This is ~600 entries, each costing a localized display name plus up to two ICU
* short-name lookups, so it is **not** cheap enough for the main thread — build
* it off-thread once and filter the result with [filterTimeZones] rather than
* rebuilding per keystroke.
* *
* Bare three-letter ids ("EST", "CST6CDT") and the legacy SystemV tree are * Bare three-letter ids ("EST", "CST6CDT") and the legacy SystemV tree are
* dropped: they're aliases the tz database keeps for compatibility, they'd * dropped: they're aliases the tz database keeps for compatibility, they'd
@@ -154,18 +183,15 @@ fun filterTimeZones(options: List<TimeZoneOption>, query: String): List<TimeZone
if (needle.isEmpty()) return options if (needle.isEmpty()) return options
return options return options
.mapNotNull { option -> .mapNotNull { option ->
val city = option.city.normalizeForSearch() val keys = option.searchKeys
val id = option.id.normalizeForSearch()
val name = option.displayName.normalizeForSearch()
val abbrev = option.shortName.normalizeForSearch()
val rank = when { val rank = when {
city.startsWith(needle) -> 0 keys.city.startsWith(needle) -> 0
abbrev == needle -> 1 keys.shortName == needle -> 1
name.startsWith(needle) -> 2 keys.displayName.startsWith(needle) -> 2
abbrev.startsWith(needle) -> 3 keys.shortName.startsWith(needle) -> 3
city.contains(needle) -> 4 keys.city.contains(needle) -> 4
id.contains(needle) -> 5 keys.id.contains(needle) -> 5
name.contains(needle) -> 6 keys.displayName.contains(needle) -> 6
else -> return@mapNotNull null else -> return@mapNotNull null
} }
rank to option rank to option
@@ -174,6 +200,9 @@ fun filterTimeZones(options: List<TimeZoneOption>, query: String): List<TimeZone
.map { it.second } .map { it.second }
} }
/** Unicode combining marks — what NFD decomposition leaves an accent as. */
private val COMBINING_MARKS = Regex("\\p{Mn}+")
/** /**
* Lowercased, accent-stripped, underscores and slashes flattened to spaces, so * Lowercased, accent-stripped, underscores and slashes flattened to spaces, so
* a query types the way a place is spoken rather than the way the tz database * a query types the way a place is spoken rather than the way the tz database
@@ -181,7 +210,7 @@ fun filterTimeZones(options: List<TimeZoneOption>, query: String): List<TimeZone
*/ */
private fun String.normalizeForSearch(): String = private fun String.normalizeForSearch(): String =
Normalizer.normalize(this, Normalizer.Form.NFD) Normalizer.normalize(this, Normalizer.Form.NFD)
.replace(Regex("\\p{Mn}+"), "") .replace(COMBINING_MARKS, "")
.replace('_', ' ') .replace('_', ' ')
.replace('/', ' ') .replace('/', ' ')
.lowercase(Locale.ROOT) .lowercase(Locale.ROOT)

View File

@@ -89,7 +89,9 @@ import kotlinx.datetime.toLocalDateTime
import kotlin.time.Instant import kotlin.time.Instant
import java.util.Locale import java.util.Locale
private val zone = TimeZone.currentSystemDefault() // No file-level zone constant here on purpose: it would be fixed for the process
// lifetime and drift from the zone AgendaViewModel groups in after a device
// time-zone change. The zone travels on AgendaUiState.Success instead.
@OptIn(ExperimentalMaterial3Api::class) @OptIn(ExperimentalMaterial3Api::class)
@Composable @Composable
@@ -332,6 +334,7 @@ private fun AgendaContent(
AgendaList( AgendaList(
days = days, days = days,
today = state.today, today = state.today,
zone = state.zone,
dimPast = pastDisplay == PastEventDisplay.DIM, dimPast = pastDisplay == PastEventDisplay.DIM,
now = now, now = now,
onEventClick = onEventClick, onEventClick = onEventClick,
@@ -348,6 +351,7 @@ private fun AgendaContent(
private fun AgendaList( private fun AgendaList(
days: List<AgendaDay>, days: List<AgendaDay>,
today: LocalDate, today: LocalDate,
zone: TimeZone,
dimPast: Boolean, dimPast: Boolean,
now: Instant, now: Instant,
onEventClick: (EventInstance) -> Unit, onEventClick: (EventInstance) -> Unit,
@@ -379,6 +383,7 @@ private fun AgendaList(
AgendaEventRow( AgendaEventRow(
event = event, event = event,
day = day.date, day = day.date,
zone = zone,
position = positionOf(index, day.events.size), position = positionOf(index, day.events.size),
dimmed = dimPast && event.hasEnded(now), dimmed = dimPast && event.hasEnded(now),
modifier = animateItemMotion(), modifier = animateItemMotion(),
@@ -458,6 +463,7 @@ private fun AgendaEmptyDayRow(onClick: () -> Unit) {
private fun AgendaEventRow( private fun AgendaEventRow(
event: EventInstance, event: EventInstance,
day: LocalDate, day: LocalDate,
zone: TimeZone,
position: Position, position: Position,
dimmed: Boolean, dimmed: Boolean,
modifier: Modifier = Modifier, modifier: Modifier = Modifier,
@@ -469,7 +475,7 @@ private fun AgendaEventRow(
GroupedRow( GroupedRow(
modifier = if (dimmed) modifier.alpha(EventDimAlpha) else modifier, modifier = if (dimmed) modifier.alpha(EventDimAlpha) else modifier,
title = title, title = title,
summary = agendaTimeSummary(event, day), summary = agendaTimeSummary(event, day, zone),
position = position, position = position,
minHeight = 64.dp, minHeight = 64.dp,
leading = { leading = {
@@ -575,25 +581,34 @@ private fun agendaDayLabel(date: LocalDate, today: LocalDate): String {
* An all-day multi-day event is simply "All day" on every day it covers. * An all-day multi-day event is simply "All day" on every day it covers.
*/ */
@Composable @Composable
private fun agendaTimeSummary(event: EventInstance, day: LocalDate): String { private fun agendaTimeSummary(event: EventInstance, day: LocalDate, zone: TimeZone): String {
val is24Hour = LocalUse24HourFormat.current val is24Hour = LocalUse24HourFormat.current
val locale = currentLocale() val locale = currentLocale()
val time = when (val label = agendaTimeLabel(event, day, zone)) { val time = when (val label = agendaTimeLabel(event, day, zone)) {
AgendaTimeLabel.AllDay -> stringResource(R.string.event_detail_all_day) AgendaTimeLabel.AllDay -> stringResource(R.string.event_detail_all_day)
is AgendaTimeLabel.Starts -> is AgendaTimeLabel.Starts -> stringResource(
stringResource(R.string.agenda_span_starts, formatTime(label.start, is24Hour, locale)) R.string.agenda_span_starts,
is AgendaTimeLabel.Ends -> formatTime(label.start, zone, is24Hour, locale),
stringResource(R.string.agenda_span_ends, formatTime(label.end, is24Hour, locale)) )
is AgendaTimeLabel.Range -> is AgendaTimeLabel.Ends -> stringResource(
"${formatTime(label.start, is24Hour, locale)} ${formatTime(label.end, is24Hour, locale)}" R.string.agenda_span_ends,
formatTime(label.end, zone, is24Hour, locale),
)
is AgendaTimeLabel.Range -> "${formatTime(label.start, zone, is24Hour, locale)} " +
formatTime(label.end, zone, is24Hour, locale)
} }
val location = event.location?.takeIf { it.isNotBlank() } val location = event.location?.takeIf { it.isNotBlank() }
return if (location != null) "$time · $location" else time return if (location != null) "$time · $location" else time
} }
private fun formatTime(instant: Instant, is24Hour: Boolean, locale: Locale): String { private fun formatTime(
instant: Instant,
zone: TimeZone,
is24Hour: Boolean,
locale: Locale,
): String {
val t = instant.toLocalDateTime(zone).time val t = instant.toLocalDateTime(zone).time
return formatTimeOfDay(t.hour, t.minute, is24Hour, locale) return formatTimeOfDay(t.hour, t.minute, is24Hour, locale)
} }

View File

@@ -160,5 +160,12 @@ sealed interface AgendaUiState {
val rangeEnd: LocalDate, val rangeEnd: LocalDate,
/** Whether to show the top range bar — header + switcher (toggle, on by default). */ /** Whether to show the top range bar — header + switcher (toggle, on by default). */
val showRangeBar: Boolean, val showRangeBar: Boolean,
/**
* The zone [days] were grouped in. Carried in the state rather than
* re-read by the screen so labelling and grouping cannot disagree: an
* event's "Starts …/Ends …/All day" line is only correct relative to the
* same zone that decided which day it was filed under.
*/
val zone: TimeZone,
) : AgendaUiState ) : AgendaUiState
} }

View File

@@ -172,6 +172,7 @@ class AgendaViewModel @Inject constructor(
rangeIsOverride = params.rangeIsOverride, rangeIsOverride = params.rangeIsOverride,
rangeEnd = rangeEnd, rangeEnd = rangeEnd,
showRangeBar = params.showRangeBar, showRangeBar = params.showRangeBar,
zone = zone,
) )
} }
} }

View File

@@ -16,13 +16,19 @@ import java.util.Locale
* sits directly above a grid that already says which year it is, and the year's * sits directly above a grid that already says which year it is, and the year's
* *absence* is itself the signal that you're in the current one — it appears the * *absence* is itself the signal that you're in the current one — it appears the
* moment you page out of it, which is when it starts carrying information. * moment you page out of it, which is when it starts carrying information.
*
* [forceYear] overrides that for titles whose [date] does not tell the whole
* story — a week view's title names only the month its *first* day falls in, so
* a week straddling New Year must still show the year even though [date] is in
* [currentYear].
*/ */
fun formatCalendarTitle( fun formatCalendarTitle(
date: java.time.LocalDate, date: java.time.LocalDate,
locale: Locale, locale: Locale,
currentYear: Int, currentYear: Int,
skeleton: String, skeleton: String,
forceYear: Boolean = false,
): String { ): String {
val fields = if (date.year == currentYear) skeleton else skeleton + "y" val fields = if (date.year == currentYear && !forceYear) skeleton else skeleton + "y"
return localizedDateFormatter(locale, fields).format(date) return localizedDateFormatter(locale, fields).format(date)
} }

View File

@@ -21,6 +21,7 @@ import androidx.compose.material3.Text
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.produceState
import androidx.compose.runtime.remember import androidx.compose.runtime.remember
import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue import androidx.compose.runtime.setValue
@@ -35,6 +36,7 @@ import androidx.compose.ui.unit.dp
import de.jeanlucmakiola.calendula.R import de.jeanlucmakiola.calendula.R
import de.jeanlucmakiola.calendula.domain.TimeZoneOption import de.jeanlucmakiola.calendula.domain.TimeZoneOption
import de.jeanlucmakiola.calendula.domain.filterTimeZones import de.jeanlucmakiola.calendula.domain.filterTimeZones
import de.jeanlucmakiola.calendula.domain.timeZoneOptionOf
import de.jeanlucmakiola.calendula.domain.timeZoneOptions import de.jeanlucmakiola.calendula.domain.timeZoneOptions
import de.jeanlucmakiola.calendula.domain.zoneDescriptor import de.jeanlucmakiola.calendula.domain.zoneDescriptor
import de.jeanlucmakiola.floret.components.FullScreenPicker import de.jeanlucmakiola.floret.components.FullScreenPicker
@@ -43,6 +45,8 @@ import de.jeanlucmakiola.floret.components.InlineTextField
import de.jeanlucmakiola.floret.components.Position import de.jeanlucmakiola.floret.components.Position
import de.jeanlucmakiola.floret.components.positionOf import de.jeanlucmakiola.floret.components.positionOf
import de.jeanlucmakiola.floret.locale.currentLocale import de.jeanlucmakiola.floret.locale.currentLocale
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
/** /**
* Full-screen zone picker: a query field over every zone the JVM knows, with the * Full-screen zone picker: a query field over every zone the JVM knows, with the
@@ -64,14 +68,28 @@ fun TimeZonePickerDialog(
onDismiss: () -> Unit, onDismiss: () -> Unit,
) { ) {
var query by rememberSaveable { mutableStateOf("") } var query by rememberSaveable { mutableStateOf("") }
// ~600 zones, each resolving a localized name and a DST-aware offset: build
// once per open, then filter the result per keystroke.
val locale = currentLocale() val locale = currentLocale()
val allZones = remember(locale) { timeZoneOptions(locale, regionOf = ::icuTimeZoneRegion) } // ~600 zones, each resolving a localized name and up to two ICU short names:
// far too much to run inside composition, so build it on a worker and let the
// list fill in a frame later. Filtering the built catalogue is cheap (its
// search keys are pre-normalized), so that stays here.
val allZones by produceState(initialValue = emptyList<TimeZoneOption>(), locale) {
value = withContext(Dispatchers.Default) {
timeZoneOptions(locale, regionOf = ::icuTimeZoneRegion)
}
}
val matches = remember(allZones, query) { filterTimeZones(allZones, query) } val matches = remember(allZones, query) { filterTimeZones(allZones, query) }
val recentZones = remember(allZones, recents) { val recentZones = remember(allZones, recents) {
recents.mapNotNull { id -> allZones.firstOrNull { it.id == id } } recents.mapNotNull { id -> allZones.firstOrNull { it.id == id } }
} }
// Resolved on its own rather than looked up in the catalogue: it's one zone,
// so it costs nothing, and the device row can then render complete on the
// first frame instead of showing a bare id until the catalogue lands.
val deviceSummary = remember(deviceZoneId, locale) {
timeZoneOptionOf(deviceZoneId, locale, regionOf = ::icuTimeZoneRegion)
?.let { zoneDescriptor(it) }
?: deviceZoneId
}
val searching = query.isNotBlank() val searching = query.isNotBlank()
fun choose(zoneId: String?) { fun choose(zoneId: String?) {
@@ -97,7 +115,7 @@ fun TimeZonePickerDialog(
item(key = "device") { item(key = "device") {
GroupedRow( GroupedRow(
title = stringResource(R.string.event_edit_timezone_device), title = stringResource(R.string.event_edit_timezone_device),
summary = zoneSummary(deviceZoneId, allZones), summary = deviceSummary,
position = Position.Alone, position = Position.Alone,
selected = selected == null, selected = selected == null,
leading = { Icon(Icons.Default.Public, contentDescription = null) }, leading = { Icon(Icons.Default.Public, contentDescription = null) },
@@ -130,7 +148,10 @@ fun TimeZonePickerDialog(
} }
} }
if (searching && matches.isEmpty()) { // allZones.isNotEmpty() gates this: until the catalogue lands there is
// simply nothing to match yet, and claiming "no time zone matches"
// for that frame would be wrong.
if (searching && matches.isEmpty() && allZones.isNotEmpty()) {
item(key = "empty") { item(key = "empty") {
Text( Text(
text = stringResource(R.string.event_edit_timezone_none, query), text = stringResource(R.string.event_edit_timezone_none, query),
@@ -252,9 +273,3 @@ private fun ZoneRow(
onClick = onClick, onClick = onClick,
) )
} }
/** "CET · GMT+01:00" for [zoneId], or the bare id if the catalogue lacks it. */
private fun zoneSummary(zoneId: String, allZones: List<TimeZoneOption>): String =
allZones.firstOrNull { it.id == zoneId }
?.let { zoneDescriptor(it) }
?: zoneId

View File

@@ -109,6 +109,7 @@ import de.jeanlucmakiola.calendula.ui.common.recurrenceText
import de.jeanlucmakiola.calendula.ui.common.reminderLeadTimeLabel import de.jeanlucmakiola.calendula.ui.common.reminderLeadTimeLabel
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import kotlinx.datetime.TimeZone import kotlinx.datetime.TimeZone
import kotlin.time.toJavaInstant
import java.time.ZoneId import java.time.ZoneId
import java.time.format.DateTimeFormatter import java.time.format.DateTimeFormatter
import java.time.format.FormatStyle import java.time.format.FormatStyle
@@ -463,8 +464,8 @@ private fun EventDetailContent(state: EventDetailUiState.Success, modifier: Modi
// Same hierarchy as the When card above — the label carries the card, // Same hierarchy as the When card above — the label carries the card,
// the time sits small beneath it. The local time is the one the reader // the time sits small beneath it. The local time is the one the reader
// acts on, so the original must stay quieter than it, not compete. // acts on, so the original must stay quieter than it, not compete.
val foreignZone = remember(detail.eventTimezone, instance.isAllDay, locale) { val foreignZone = remember(detail.eventTimezone, instance.isAllDay, instance.start, locale) {
foreignTimeZone(detail.eventTimezone, instance.isAllDay, locale) foreignTimeZone(detail.eventTimezone, instance.isAllDay, locale, instance.start)
} }
foreignZone?.let { zoneOption -> foreignZone?.let { zoneOption ->
Spacer(Modifier.height(gap)) Spacer(Modifier.height(gap))
@@ -789,11 +790,26 @@ private fun reminderLeadText(reminder: Reminder): String = reminderLeadTimeLabel
* pinned to a zone different from the device's — the cases where showing it * pinned to a zone different from the device's — the cases where showing it
* removes ambiguity. Null otherwise (all-day, device-zone, blank, or an id the * removes ambiguity. Null otherwise (all-day, device-zone, blank, or an id the
* tz database doesn't know). * tz database doesn't know).
*
* Resolved *at [at]*, the event's own start, not at "now": the abbreviation and
* offset both move with DST, and the card prints them next to the event's time.
* Resolving at now would label a July event "CET · 10:00 AM" when read in
* January — an abbreviation that contradicts the time beside it.
*/ */
private fun foreignTimeZone(tz: String?, isAllDay: Boolean, locale: Locale): TimeZoneOption? { private fun foreignTimeZone(
tz: String?,
isAllDay: Boolean,
locale: Locale,
at: Instant,
): TimeZoneOption? {
if (isAllDay || tz.isNullOrBlank()) return null if (isAllDay || tz.isNullOrBlank()) return null
if (tz == ZoneId.systemDefault().id) return null if (tz == ZoneId.systemDefault().id) return null
return timeZoneOptionOf(tz, locale, regionOf = ::icuTimeZoneRegion) return timeZoneOptionOf(
tz,
locale,
at = at.toJavaInstant(),
regionOf = ::icuTimeZoneRegion,
)
} }
/** Wrap http(s) URLs in [text] as tappable links tinted [linkColor]. */ /** Wrap http(s) URLs in [text] as tappable links tinted [linkColor]. */

View File

@@ -47,7 +47,6 @@ import androidx.compose.material.icons.filled.Person
import androidx.compose.material.icons.filled.PersonAdd import androidx.compose.material.icons.filled.PersonAdd
import androidx.compose.material.icons.filled.Place import androidx.compose.material.icons.filled.Place
import androidx.compose.material.icons.filled.Public import androidx.compose.material.icons.filled.Public
import androidx.compose.material.icons.filled.Public
import androidx.compose.material.icons.filled.Repeat import androidx.compose.material.icons.filled.Repeat
import androidx.compose.material.icons.filled.Schedule import androidx.compose.material.icons.filled.Schedule
import androidx.compose.material.icons.filled.Tune import androidx.compose.material.icons.filled.Tune
@@ -164,8 +163,10 @@ import kotlinx.datetime.isoDayNumber
import kotlinx.datetime.toJavaDayOfWeek import kotlinx.datetime.toJavaDayOfWeek
import kotlinx.datetime.toJavaLocalDate import kotlinx.datetime.toJavaLocalDate
import kotlinx.datetime.toKotlinDayOfWeek import kotlinx.datetime.toKotlinDayOfWeek
import kotlinx.datetime.toInstant
import kotlinx.datetime.toJavaLocalTime import kotlinx.datetime.toJavaLocalTime
import kotlinx.datetime.toLocalDateTime import kotlinx.datetime.toLocalDateTime
import kotlin.time.toJavaInstant
import java.time.ZoneId import java.time.ZoneId
import java.time.format.DateTimeFormatter import java.time.format.DateTimeFormatter
import java.time.format.FormatStyle import java.time.format.FormatStyle
@@ -683,7 +684,13 @@ private fun EventEditContent(
// whoever is reading it. Spell the local equivalent out rather than // whoever is reading it. Spell the local equivalent out rather than
// leaving the user to do the offset arithmetic. Absent (null) unless // leaving the user to do the offset arithmetic. Absent (null) unless
// the event is pinned somewhere other than here. // the event is pinned somewhere other than here.
form.timesIn(TimeZone.currentSystemDefault())?.let { (localStart, localEnd) -> // Keyed rather than recomputed: this sits in the same Column as the
// title field, so it would otherwise re-parse the zone on every
// keystroke.
val localTimes = remember(form.timezone, form.start, form.end, form.isAllDay) {
form.timesIn(TimeZone.currentSystemDefault())
}
localTimes?.let { (localStart, localEnd) ->
Spacer(Modifier.height(2.dp)) Spacer(Modifier.height(2.dp))
Text( Text(
text = stringResource( text = stringResource(
@@ -741,8 +748,20 @@ private fun EventEditContent(
verticalAlignment = Alignment.CenterVertically, verticalAlignment = Alignment.CenterVertically,
modifier = Modifier.fillMaxWidth(), modifier = Modifier.fillMaxWidth(),
) { ) {
val pinned = remember(form.timezone, locale) { // Resolved at the event's own start, not at "now": the
form.timezone?.let { timeZoneOptionOf(it, locale, regionOf = ::icuTimeZoneRegion) } // abbreviation and offset shown ("CEST · GMT+02:00") must be
// the ones that apply when the event actually happens.
val pinned = remember(form.timezone, form.start, locale) {
form.timezone
?.let { id -> runCatching { id to TimeZone.of(id) }.getOrNull() }
?.let { (id, zone) ->
timeZoneOptionOf(
id,
locale,
at = form.start.toInstant(zone).toJavaInstant(),
regionOf = ::icuTimeZoneRegion,
)
}
} }
Column(modifier = Modifier.weight(1f)) { Column(modifier = Modifier.weight(1f)) {
Text( Text(

View File

@@ -213,10 +213,22 @@ class SettingsViewModel @Inject constructor(
* imperatively via [LauncherNameManager] and held here in its own flow — the * imperatively via [LauncherNameManager] and held here in its own flow — the
* main settings combine is already at its arity limit and this isn't a * main settings combine is already at its arity limit and this isn't a
* DataStore flow anyway. * DataStore flow anyway.
*
* Seeded with the manifest default and corrected off-thread rather than read
* in the initializer: `getComponentEnabledSetting` is a binder round-trip to
* system_server, and this ViewModel is constructed on the main thread while
* Settings is opening. The row it feeds is well below the fold, so the
* one-frame correction is never visible.
*/ */
private val _launcherName = MutableStateFlow(launcherNameManager.current()) private val _launcherName = MutableStateFlow(LauncherName.CALENDULA)
val launcherName: StateFlow<LauncherName> = _launcherName.asStateFlow() val launcherName: StateFlow<LauncherName> = _launcherName.asStateFlow()
init {
viewModelScope.launch {
_launcherName.value = withContext(io) { launcherNameManager.current() }
}
}
// Emitted when a picked font file couldn't be read as a font; the screen // Emitted when a picked font file couldn't be read as a font; the screen
// surfaces it and the previous selection stays put. // surfaces it and the previous selection stays put.
private val _fontImportFailed = MutableSharedFlow<Unit>(extraBufferCapacity = 1) private val _fontImportFailed = MutableSharedFlow<Unit>(extraBufferCapacity = 1)
@@ -484,11 +496,20 @@ class SettingsViewModel @Inject constructor(
viewModelScope.launch { prefs.setTodayButtonInToolbar(enabled) } viewModelScope.launch { prefs.setTodayButtonInToolbar(enabled) }
} }
/** Switch the launcher label between "Calendula" and "Calendar" (issue #44). */ /**
* Switch the launcher label between "Calendula" and "Calendar" (issue #44).
* The card highlights immediately, then settles on whatever the component
* state actually reports — the two `setComponentEnabledSetting` calls are
* binder round-trips, so they don't belong on the main thread either.
*/
fun setLauncherName(name: LauncherName) { fun setLauncherName(name: LauncherName) {
launcherNameManager.set(name) _launcherName.value = name
// Re-read so the UI reflects the actual component state, not an assumption. viewModelScope.launch {
_launcherName.value = launcherNameManager.current() _launcherName.value = withContext(io) {
launcherNameManager.set(name)
launcherNameManager.current()
}
}
} }
fun setPastEventDisplay(mode: PastEventDisplay) { fun setPastEventDisplay(mode: PastEventDisplay) {

View File

@@ -880,13 +880,21 @@ private fun minToHm(min: Int, is24Hour: Boolean, locale: java.util.Locale): Stri
* month of [weekStart] means a week straddling a boundary keeps the outgoing * month of [weekStart] means a week straddling a boundary keeps the outgoing
* month until it is fully gone — a week is seven contiguous days, so the earlier * month until it is fully gone — a week is seven contiguous days, so the earlier
* month has a day in it exactly while [weekStart] is still inside it. That also * month has a day in it exactly while [weekStart] is still inside it. That also
* makes the title depend on nothing but [weekStart], so it cannot drift with the * makes the *month* depend on nothing but [weekStart], so it cannot drift with
* direction you paged in from. * the direction you paged in from.
*
* The *year* is decided from the whole week, not just its start: a week running
* Dec 28 Jan 3 has four of its seven visible columns in the new year, so
* "December" with no year would be actively misleading while the reader is
* looking straight at January dates.
*/ */
private fun formatWeekTitle(weekStart: LocalDate, locale: Locale, currentYear: Int): String = private fun formatWeekTitle(weekStart: LocalDate, locale: Locale, currentYear: Int): String {
formatCalendarTitle( val weekEnd = weekStart.plus(6, kotlinx.datetime.DateTimeUnit.DAY)
return formatCalendarTitle(
date = java.time.LocalDate.of(weekStart.year, weekStart.month.ordinal + 1, 1), date = java.time.LocalDate.of(weekStart.year, weekStart.month.ordinal + 1, 1),
locale = locale, locale = locale,
currentYear = currentYear, currentYear = currentYear,
skeleton = "LLLL", skeleton = "LLLL",
forceYear = weekEnd.year != weekStart.year,
) )
}

View File

@@ -110,7 +110,6 @@
<string name="event_edit_timezone_recent">Recent</string> <string name="event_edit_timezone_recent">Recent</string>
<string name="event_edit_timezone_all">All time zones</string> <string name="event_edit_timezone_all">All time zones</string>
<string name="event_edit_timezone_none">No time zone matches “%1$s”</string> <string name="event_edit_timezone_none">No time zone matches “%1$s”</string>
<string name="event_edit_timezone_clear">Use device zone</string>
<!-- Shown under the time fields when the event is pinned to another zone: <!-- Shown under the time fields when the event is pinned to another zone:
the same event expressed where the user actually is. %1$s is a time the same event expressed where the user actually is. %1$s is a time
range, e.g. "2:00 PM 3:00 PM". --> range, e.g. "2:00 PM 3:00 PM". -->

View File

@@ -123,8 +123,18 @@ class EventWriteMapperTest {
private val seriesStart = 1_700_000_000_000L private val seriesStart = 1_700_000_000_000L
private fun update(original: EventForm, updated: EventForm): Map<String, Any?> = private fun update(
buildEventUpdateValues(original, updated, seriesStart, berlin) original: EventForm,
updated: EventForm,
series: Long = seriesStart,
): Map<String, Any?> = buildEventUpdateValues(original, updated, series, berlin)
/** The instant [local] names in [zoneId], as the provider would store it. */
private fun instantAt(local: String, zoneId: String): Long =
java.time.LocalDateTime.parse(local)
.atZone(java.time.ZoneId.of(zoneId))
.toInstant()
.toEpochMilli()
@Test @Test
fun `pristine form produces no values`() { fun `pristine form produces no values`() {
@@ -219,6 +229,77 @@ class EventWriteMapperTest {
assertThat(values[CalendarContract.Events.DURATION]).isEqualTo("P5400S") assertThat(values[CalendarContract.Events.DURATION]).isEqualTo("P5400S")
} }
@Test
fun `pinning a recurring event to another zone keeps the series wall clock`() {
// The regression this guards: the series DTSTART used to move by a
// millisecond delta measured at the *edited occurrence*. Here the series
// anchor sits in January (Berlin CET, +1) and the edited occurrence in
// July (Berlin CEST, +2), so the July delta is an hour off for January —
// the whole series would have drifted to 10:00 Tokyo.
val series = instantAt("2026-01-07T09:00", "Europe/Berlin")
val original = form(
start = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(9, 0)),
end = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(10, 0)),
).copy(rrule = "FREQ=WEEKLY")
val values = update(original, original.copy(timezone = "Asia/Tokyo"), series)
assertThat(values[CalendarContract.Events.EVENT_TIMEZONE]).isEqualTo("Asia/Tokyo")
// The anchor still reads 09:00 — now 09:00 in Tokyo, not 10:00.
assertThat(values[CalendarContract.Events.DTSTART])
.isEqualTo(instantAt("2026-01-07T09:00", "Asia/Tokyo"))
}
@Test
fun `a time edit moves the series anchor in wall clock across a DST boundary`() {
// Anchor in winter, edited occurrence in summer: pushing the occurrence
// one hour later must leave the anchor at 10:00 winter time, not at an
// instant that re-reads as 11:00 once the offset differs.
val series = instantAt("2026-01-07T09:00", "Europe/Berlin")
val original = form(
start = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(9, 0)),
end = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(10, 0)),
).copy(rrule = "FREQ=WEEKLY")
val moved = original.copy(
start = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(10, 0)),
end = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(11, 0)),
)
assertThat(update(original, moved, series)[CalendarContract.Events.DTSTART])
.isEqualTo(instantAt("2026-01-07T10:00", "Europe/Berlin"))
}
@Test
fun `moving a recurring occurrence to another day shifts the anchor by whole days`() {
val series = instantAt("2026-01-07T09:00", "Europe/Berlin")
val original = form(
start = LocalDateTime(LocalDate(2026, 1, 7), LocalTime(9, 0)),
end = LocalDateTime(LocalDate(2026, 1, 7), LocalTime(10, 0)),
).copy(rrule = "FREQ=WEEKLY")
val moved = original.copy(
start = LocalDateTime(LocalDate(2026, 1, 9), LocalTime(14, 30)),
end = LocalDateTime(LocalDate(2026, 1, 9), LocalTime(15, 30)),
)
assertThat(update(original, moved, series)[CalendarContract.Events.DTSTART])
.isEqualTo(instantAt("2026-01-09T14:30", "Europe/Berlin"))
}
@Test
fun `switching a recurring event to all-day anchors the series on a UTC midnight`() {
val series = instantAt("2026-01-07T09:00", "Europe/Berlin")
val original = form(
start = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(9, 0)),
end = LocalDateTime(LocalDate(2026, 7, 15), LocalTime(10, 0)),
).copy(rrule = "FREQ=WEEKLY")
val values = update(original, original.copy(isAllDay = true), series)
assertThat(values[CalendarContract.Events.EVENT_TIMEZONE]).isEqualTo("UTC")
val dtStart = values[CalendarContract.Events.DTSTART] as Long
assertThat(dtStart % (24L * 60 * 60 * 1000)).isEqualTo(0L)
}
@Test @Test
fun `adding a recurrence keeps the times and writes rule plus duration`() { fun `adding a recurrence keeps the times and writes rule plus duration`() {
val original = form() val original = form()