diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/TimeZoneCatalog.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/TimeZoneCatalog.kt index ca08fc0..b67af5d 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/TimeZoneCatalog.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/TimeZoneCatalog.kt @@ -3,20 +3,31 @@ package de.jeanlucmakiola.calendula.domain import java.text.Normalizer import java.time.Instant import java.time.ZoneId +import java.time.ZonedDateTime +import java.time.format.DateTimeFormatter import java.time.format.TextStyle import java.util.Locale /** - * One selectable zone, resolved for display: [id] is the IANA id we store in - * `EVENT_TIMEZONE`, [displayName] its localized name, and [offsetMinutes] its - * offset *at a given instant* — zones shift with DST, so an offset is only - * meaningful next to the moment it was resolved for. + * One selectable zone, resolved for display at a given instant. [id] is the IANA + * id we store in `EVENT_TIMEZONE` and show as the primary label (via [label]); + * [shortName] its abbreviation ("CET", "WET", "UTC", or a "GMT+05:30" fallback); + * [offsetMinutes] its offset. Both the abbreviation and the offset shift with + * DST, so they're only meaningful next to the moment they were resolved for. + * + * [displayName] is the long localized name ("Central European Time"). It's kept + * for search only — matching on it lets someone type "pacific" — and is *not* + * displayed: spelled out next to the id it made the field too wide to fit. */ data class TimeZoneOption( val id: String, val displayName: String, + val shortName: String, val offsetMinutes: Int, ) { + /** The id as the primary label, underscores undone ("America/New York"). */ + val label: String get() = id.replace('_', ' ') + /** The trailing segment of the id ("Europe/Berlin" -> "Berlin"), underscores undone. */ val city: String get() = id.substringAfterLast('/').replace('_', ' ') @@ -24,10 +35,20 @@ data class TimeZoneOption( val region: String get() = id.substringBeforeLast('/', missingDelimiterValue = "") } +private fun optionFor(zone: ZoneId, locale: Locale, at: Instant) = TimeZoneOption( + id = zone.id, + displayName = zone.getDisplayName(TextStyle.FULL, locale), + // "zzz" resolves the DST-correct abbreviation at [at] (CET vs CEST); zones + // with no named abbreviation fall back to a "GMT+05:30" form, which is + // exactly what we'd want to show them as anyway. + shortName = ZonedDateTime.ofInstant(at, zone) + .format(DateTimeFormatter.ofPattern("zzz", locale)), + offsetMinutes = zone.rules.getOffset(at).totalSeconds / 60, +) + /** - * Every zone the JVM knows, localized and resolved at [at]. This is ~600 - * entries, so build it once and filter the result rather than rebuilding per - * keystroke. + * Every zone the JVM knows, resolved at [at]. This is ~600 entries, so build it + * once and filter the result rather than rebuilding per keystroke. * * Bare three-letter ids ("EST", "CST6CDT") and the legacy SystemV tree are * dropped: they're aliases the tz database keeps for compatibility, they'd @@ -40,14 +61,7 @@ fun timeZoneOptions( ): List = ZoneId.getAvailableZoneIds() .asSequence() .filter { it.contains('/') && !it.startsWith("SystemV/") } - .map { id -> - val zone = ZoneId.of(id) - TimeZoneOption( - id = id, - displayName = zone.getDisplayName(TextStyle.FULL, locale), - offsetMinutes = zone.rules.getOffset(at).totalSeconds / 60, - ) - } + .map { optionFor(ZoneId.of(it), locale, at) } .sortedWith(compareBy({ it.region }, { it.city })) .toList() @@ -62,11 +76,18 @@ fun timeZoneOptionOf( at: Instant = Instant.now(), ): TimeZoneOption? { val zone = runCatching { ZoneId.of(id) }.getOrNull() ?: return null - return TimeZoneOption( - id = id, - displayName = zone.getDisplayName(TextStyle.FULL, locale), - offsetMinutes = zone.rules.getOffset(at).totalSeconds / 60, - ) + return optionFor(zone, locale, at) +} + +/** + * The short descriptor shown beneath the id, e.g. "CET · GMT+01:00". When the + * abbreviation is itself offset-shaped ("UTC", "GMT+05:30") the offset would + * only repeat it, so the abbreviation stands alone. + */ +fun zoneDescriptor(option: TimeZoneOption): String { + val abbrev = option.shortName + val offsetShaped = abbrev == "UTC" || abbrev.startsWith("GMT") + return if (offsetShaped) abbrev else "$abbrev · ${formatGmtOffset(option.offsetMinutes)}" } /** diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimeZonePicker.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimeZonePicker.kt index 4e1579d..ed74451 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimeZonePicker.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimeZonePicker.kt @@ -35,8 +35,8 @@ import androidx.compose.ui.unit.dp import de.jeanlucmakiola.calendula.R import de.jeanlucmakiola.calendula.domain.TimeZoneOption import de.jeanlucmakiola.calendula.domain.filterTimeZones -import de.jeanlucmakiola.calendula.domain.formatGmtOffset import de.jeanlucmakiola.calendula.domain.timeZoneOptions +import de.jeanlucmakiola.calendula.domain.zoneDescriptor import de.jeanlucmakiola.floret.components.FullScreenPicker import de.jeanlucmakiola.floret.components.GroupedRow import de.jeanlucmakiola.floret.components.InlineTextField @@ -240,8 +240,8 @@ private fun ZoneRow( onClick: () -> Unit, ) { GroupedRow( - title = zone.city, - summary = "${zone.displayName} · ${formatGmtOffset(zone.offsetMinutes)}", + title = zone.label, + summary = zoneDescriptor(zone), position = position, selected = selected, trailing = if (selected) { @@ -253,8 +253,8 @@ private fun ZoneRow( ) } -/** "Central European Time · GMT+02:00" for [zoneId], or the bare id if unknown. */ +/** "CET · GMT+01:00" for [zoneId], or the bare id if the catalogue lacks it. */ private fun zoneSummary(zoneId: String, allZones: List): String = allZones.firstOrNull { it.id == zoneId } - ?.let { "${it.displayName} · ${formatGmtOffset(it.offsetMinutes)}" } + ?.let { zoneDescriptor(it) } ?: zoneId diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/detail/EventDetailScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/detail/EventDetailScreen.kt index 61fad9f..83c7032 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/detail/EventDetailScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/detail/EventDetailScreen.kt @@ -93,6 +93,9 @@ import de.jeanlucmakiola.calendula.domain.EventInstance import de.jeanlucmakiola.calendula.domain.EventStatus import de.jeanlucmakiola.calendula.domain.RecurringWriteScope import de.jeanlucmakiola.calendula.domain.Reminder +import de.jeanlucmakiola.calendula.domain.TimeZoneOption +import de.jeanlucmakiola.calendula.domain.timeZoneOptionOf +import de.jeanlucmakiola.calendula.domain.zoneDescriptor import de.jeanlucmakiola.floret.identity.predictiveBack import de.jeanlucmakiola.calendula.ui.common.CalendarFailure import de.jeanlucmakiola.calendula.ui.common.LocalSoftenColors @@ -108,7 +111,6 @@ import kotlinx.datetime.TimeZone import java.time.ZoneId import java.time.format.DateTimeFormatter import java.time.format.FormatStyle -import java.time.format.TextStyle as JavaTextStyle import java.util.Locale import kotlin.time.Duration.Companion.seconds import kotlin.time.Instant @@ -460,32 +462,37 @@ private fun EventDetailContent(state: EventDetailUiState.Success, modifier: Modi // 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 // acts on, so the original must stay quieter than it, not compete. - foreignTimeZoneLabel(detail.eventTimezone, instance.isAllDay, locale)?.let { tzLabel -> + val foreignZone = remember(detail.eventTimezone, instance.isAllDay, locale) { + foreignTimeZone(detail.eventTimezone, instance.isAllDay, locale) + } + foreignZone?.let { zoneOption -> Spacer(Modifier.height(gap)) DetailCard( icon = Icons.Default.Public, iconContentDescription = stringResource(R.string.event_detail_timezone), ) { - Text(text = tzLabel, style = MaterialTheme.typography.titleMedium) - val originalZone = detail.eventTimezone?.let { tz -> - runCatching { TimeZone.of(tz) }.getOrNull() - } - if (originalZone != null) { - // formatWhen puts the time range in the secondary half only - // for a same-day event; one spanning midnight — which a - // cross-zone event easily does — carries the whole span in - // the primary instead, so fall back to it rather than - // dropping the original time entirely. - val (primary, secondary) = formatWhen(instance, originalZone, locale) - Spacer(Modifier.height(2.dp)) - Text( - // The label above already names the zone, so the range - // needs no "in New York" tail to be unambiguous here. - text = secondary ?: primary, - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - } + // The id is the primary label ("Europe/Berlin"); the long + // localized name spelled next to it made the field too wide. + Text(text = zoneOption.label, style = MaterialTheme.typography.titleMedium) + // Beneath, small: the abbreviation plus the event's own-zone time, + // e.g. "CET · 8:00 AM – 9:00 AM". formatWhen puts the range in the + // secondary half only for a same-day event; one spanning midnight + // — which a cross-zone event easily does — carries the whole span + // in the primary instead, so fall back to it. If the zone can't be + // read back at all, drop to just the abbreviation + offset. + val originalTime = runCatching { TimeZone.of(zoneOption.id) }.getOrNull() + ?.let { formatWhen(instance, it, locale) } + ?.let { (primary, secondary) -> secondary ?: primary } + Spacer(Modifier.height(2.dp)) + Text( + text = if (originalTime != null) { + "${zoneOption.shortName} · $originalTime" + } else { + zoneDescriptor(zoneOption) + }, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) } } @@ -777,21 +784,15 @@ private fun attendeeRoleLabel(attendee: Attendee): Int? = when { private fun reminderLeadText(reminder: Reminder): String = reminderLeadTimeLabel(reminder.minutes) /** - * A localized label for [tz] (e.g. "Central European Time (Europe/Berlin)"), - * but only when the event is timed and pinned to a zone different from the - * device's. Returns null when there's nothing worth showing. + * [tz] resolved as a [TimeZoneOption], but only when the event is timed and + * 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 + * tz database doesn't know). */ -private fun foreignTimeZoneLabel(tz: String?, isAllDay: Boolean, locale: Locale): String? { +private fun foreignTimeZone(tz: String?, isAllDay: Boolean, locale: Locale): TimeZoneOption? { if (isAllDay || tz.isNullOrBlank()) return null - val deviceZone = ZoneId.systemDefault().id - if (tz == deviceZone) return null - return try { - val zone = ZoneId.of(tz) - val name = zone.getDisplayName(JavaTextStyle.FULL, locale) - if (name == tz) tz else "$name ($tz)" - } catch (e: Exception) { - tz - } + if (tz == ZoneId.systemDefault().id) return null + return timeZoneOptionOf(tz, locale) } /** Wrap http(s) URLs in [text] as tappable links tinted [linkColor]. */ diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt index 97993d2..e2f3a33 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt @@ -112,7 +112,7 @@ import de.jeanlucmakiola.calendula.domain.EventAttendee import de.jeanlucmakiola.calendula.domain.EventColorOption import de.jeanlucmakiola.calendula.domain.EventForm import de.jeanlucmakiola.calendula.domain.EventFormField -import de.jeanlucmakiola.calendula.domain.formatGmtOffset +import de.jeanlucmakiola.calendula.domain.zoneDescriptor import de.jeanlucmakiola.calendula.domain.timeZoneOptionOf import de.jeanlucmakiola.calendula.domain.timesIn import de.jeanlucmakiola.calendula.domain.EventFormProblem @@ -745,13 +745,12 @@ private fun EventEditContent( } Column(modifier = Modifier.weight(1f)) { Text( - text = pinned?.city + text = pinned?.label ?: stringResource(R.string.event_edit_timezone_device), style = MaterialTheme.typography.titleMedium, ) Text( - text = pinned - ?.let { "${it.displayName} · ${formatGmtOffset(it.offsetMinutes)}" } + text = pinned?.let { zoneDescriptor(it) } ?: stringResource(R.string.event_edit_timezone_device_summary), style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/TimeZoneCatalogTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/TimeZoneCatalogTest.kt index fbecfc5..1ef2512 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/domain/TimeZoneCatalogTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/TimeZoneCatalogTest.kt @@ -30,6 +30,45 @@ class TimeZoneCatalogTest { assertThat(ny.region).isEqualTo("America") } + @Test + fun `label is the id with underscores undone`() { + assertThat(zones.first { it.id == "America/New_York" }.label) + .isEqualTo("America/New York") + assertThat(zones.first { it.id == "Europe/Berlin" }.label).isEqualTo("Europe/Berlin") + } + + @Test + fun `resolved abbreviation is compact, not the long name`() { + // Whatever CLDR gives ("CET"/"CEST" or a "GMT+.." fallback), it must be + // short and space-free — that's the whole point of showing it instead of + // "Central European Time". + val berlin = zones.first { it.id == "Europe/Berlin" } + assertThat(berlin.shortName).doesNotContain(" ") + assertThat(berlin.shortName.length).isLessThan(berlin.displayName.length) + } + + @Test + fun `descriptor pairs a named abbreviation with the offset`() { + val option = TimeZoneOption( + id = "Europe/Berlin", + displayName = "Central European Time", + shortName = "CET", + offsetMinutes = 60, + ) + assertThat(zoneDescriptor(option)).isEqualTo("CET · GMT+01:00") + } + + @Test + fun `descriptor drops the offset when the abbreviation already is one`() { + // Repeating the offset after an offset-shaped abbreviation would just say + // the same thing twice. + fun descriptorFor(shortName: String, offset: Int) = zoneDescriptor( + TimeZoneOption(id = "X/Y", displayName = "", shortName = shortName, offsetMinutes = offset), + ) + assertThat(descriptorFor("UTC", 0)).isEqualTo("UTC") + assertThat(descriptorFor("GMT+05:30", 330)).isEqualTo("GMT+05:30") + } + @Test fun `offset is resolved at the given instant, not the current one`() { val berlin = zones.first { it.id == "Europe/Berlin" }