fix(edit): use the family's text input, and show both times when zones differ

Two review fixes.

The zone picker's search box was a raw Material OutlinedTextField — the
only one left in the app, and against the convention DialogControls
states outright ("the family's InlineTextField over a tonal surface, not
Material's outlined field"). Rebuild it on InlineTextField over a tonal
surface, with the clear button inside the surface since the picker's top
bar is the title rather than a search field.

Showing a pinned event only in its own zone answered "what was it set
to?" while dropping "when is it for me?" — the user had to do the offset
arithmetic. Show both whenever they differ:

- the edit form keeps editing the event in its own zone (that's the time
  it was set at) and captions it with the local equivalent;
- the detail screen keeps local times primary and now leads the zone card
  with the original ("8:00 AM – 9:00 AM in New York") instead of naming
  the zone and nothing else.

EventForm.timesIn is pure, so the conversion — including crossing the
date line and each zone's own DST, which don't move together — is a
plain JUnit test rather than something only reviewable on a phone.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-17 15:54:33 +02:00
parent ab3df639b9
commit 0588609c75
6 changed files with 214 additions and 12 deletions

View File

@@ -4,6 +4,7 @@ import kotlinx.datetime.LocalDate
import kotlinx.datetime.LocalDateTime import kotlinx.datetime.LocalDateTime
import kotlinx.datetime.LocalTime import kotlinx.datetime.LocalTime
import kotlinx.datetime.TimeZone import kotlinx.datetime.TimeZone
import kotlinx.datetime.toInstant
import kotlinx.datetime.toLocalDateTime import kotlinx.datetime.toLocalDateTime
import kotlin.time.Instant import kotlin.time.Instant
@@ -219,6 +220,23 @@ fun EventDetail.toEditSnapshot(beginMillis: Long, endMillis: Long, zone: TimeZon
rowEnd = instance.end, rowEnd = instance.end,
) )
/**
* The form's times as they land in [target] — what a pinned event's wall-clock
* actually means where the user is standing. Null when there's nothing to
* disambiguate: an unpinned event (already in [target]), one pinned to [target]
* itself, an all-day event (no zone), or an unparseable pinned zone.
*
* The form edits a pinned event in its own zone, so this is what lets the UI
* show the other side of the pair rather than making the user do the arithmetic.
*/
fun EventForm.timesIn(target: TimeZone): Pair<LocalDateTime, LocalDateTime>? {
if (isAllDay) return null
val pinned = timezone?.let { runCatching { TimeZone.of(it) }.getOrNull() } ?: return null
if (pinned.id == target.id) return null
return start.toInstant(pinned).toLocalDateTime(target) to
end.toInstant(pinned).toLocalDateTime(target)
}
/** /**
* The optional sections that hold a value in [form] — when editing, these * The optional sections that hold a value in [form] — when editing, these
* must be visible regardless of the user's default-fields setting, or the * must be visible regardless of the user's default-fields setting, or the

View File

@@ -2,17 +2,21 @@ package de.jeanlucmakiola.calendula.ui.common
import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.lazy.itemsIndexed
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.material.icons.Icons import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.filled.Check import androidx.compose.material.icons.filled.Check
import androidx.compose.material.icons.filled.Close
import androidx.compose.material.icons.filled.Public import androidx.compose.material.icons.filled.Public
import androidx.compose.material.icons.filled.Search import androidx.compose.material.icons.filled.Search
import androidx.compose.material3.Icon import androidx.compose.material3.Icon
import androidx.compose.material3.IconButton
import androidx.compose.material3.MaterialTheme import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Surface
import androidx.compose.material3.Text 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
@@ -20,8 +24,12 @@ import androidx.compose.runtime.mutableStateOf
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
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalSoftwareKeyboardController
import androidx.compose.ui.res.stringResource import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.input.ImeAction
import androidx.compose.ui.text.input.KeyboardCapitalization
import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.dp
import de.jeanlucmakiola.calendula.R import de.jeanlucmakiola.calendula.R
@@ -31,6 +39,7 @@ import de.jeanlucmakiola.calendula.domain.formatGmtOffset
import de.jeanlucmakiola.calendula.domain.timeZoneOptions import de.jeanlucmakiola.calendula.domain.timeZoneOptions
import de.jeanlucmakiola.floret.components.FullScreenPicker import de.jeanlucmakiola.floret.components.FullScreenPicker
import de.jeanlucmakiola.floret.components.GroupedRow import de.jeanlucmakiola.floret.components.GroupedRow
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
@@ -75,15 +84,10 @@ fun TimeZonePickerDialog(
onDismiss = onDismiss, onDismiss = onDismiss,
scrollable = false, scrollable = false,
) { ) {
OutlinedTextField( SearchField(
value = query, query = query,
onValueChange = { query = it }, onQueryChange = { query = it },
singleLine = true, modifier = Modifier.padding(horizontal = 16.dp, vertical = 8.dp),
leadingIcon = { Icon(Icons.Default.Search, contentDescription = null) },
placeholder = { Text(stringResource(R.string.event_edit_timezone_search)) },
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 16.dp, vertical = 8.dp),
) )
LazyColumn(contentPadding = PaddingValues(top = 8.dp, bottom = 24.dp)) { LazyColumn(contentPadding = PaddingValues(top = 8.dp, bottom = 24.dp)) {
if (!searching) { if (!searching) {
@@ -155,6 +159,59 @@ fun TimeZonePickerDialog(
} }
} }
/**
* The query box: the family's borderless [InlineTextField] over a tonal surface,
* so it reads like the rest of the app's inputs rather than a boxed Material
* field. The clear button rides inside the surface (unlike the search screen,
* which has a top bar to put it in) because the picker's bar is the title.
*/
@Composable
private fun SearchField(
query: String,
onQueryChange: (String) -> Unit,
modifier: Modifier = Modifier,
) {
val keyboard = LocalSoftwareKeyboardController.current
// surfaceContainerHighest — the picker sits on surfaceContainerHigh, so
// anything lower vanishes into it.
Surface(
color = MaterialTheme.colorScheme.surfaceContainerHighest,
shape = RoundedCornerShape(28.dp),
modifier = modifier.fillMaxWidth(),
) {
Row(
verticalAlignment = Alignment.CenterVertically,
modifier = Modifier.padding(horizontal = 16.dp),
) {
Icon(
imageVector = Icons.Default.Search,
contentDescription = null,
tint = MaterialTheme.colorScheme.onSurfaceVariant,
)
InlineTextField(
value = query,
onValueChange = onQueryChange,
placeholder = stringResource(R.string.event_edit_timezone_search),
capitalization = KeyboardCapitalization.None,
imeAction = ImeAction.Search,
onImeAction = { keyboard?.hide() },
modifier = Modifier
.weight(1f)
.padding(horizontal = 12.dp, vertical = 14.dp),
)
if (query.isNotEmpty()) {
IconButton(onClick = { onQueryChange("") }) {
Icon(
imageVector = Icons.Default.Close,
contentDescription = stringResource(R.string.search_clear),
tint = MaterialTheme.colorScheme.onSurfaceVariant,
)
}
}
}
}
}
/** A small primary-coloured group label, matching the settings screens. */ /** A small primary-coloured group label, matching the settings screens. */
@Composable @Composable
private fun SectionHeader(text: String) { private fun SectionHeader(text: String) {

View File

@@ -453,14 +453,41 @@ private fun EventDetailContent(state: EventDetailUiState.Success, modifier: Modi
} }
// Time zone — only when the event is timed and pinned to a zone other // Time zone — only when the event is timed and pinned to a zone other
// than the device's, so cross-zone events read unambiguously. // than the device's, so cross-zone events read unambiguously. The card
// above already answers "when is this for me"; this one answers "when
// was it set", which the zone's name alone never did — an 8 AM New York
// call showing as 2 PM here should still say 8 AM somewhere.
foreignTimeZoneLabel(detail.eventTimezone, instance.isAllDay, locale)?.let { tzLabel -> foreignTimeZoneLabel(detail.eventTimezone, instance.isAllDay, locale)?.let { tzLabel ->
Spacer(Modifier.height(gap)) Spacer(Modifier.height(gap))
DetailCard( DetailCard(
icon = Icons.Default.Public, icon = Icons.Default.Public,
iconContentDescription = stringResource(R.string.event_detail_timezone), 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)
Text(
text = stringResource(
R.string.event_detail_timezone_original,
secondary ?: primary,
originalZone.id.substringAfterLast('/').replace('_', ' '),
),
style = MaterialTheme.typography.titleMedium,
)
Spacer(Modifier.height(2.dp))
}
Text(
text = tzLabel,
style = MaterialTheme.typography.bodyMedium,
color = MaterialTheme.colorScheme.onSurfaceVariant,
)
} }
} }

View File

@@ -114,6 +114,7 @@ import de.jeanlucmakiola.calendula.domain.EventForm
import de.jeanlucmakiola.calendula.domain.EventFormField import de.jeanlucmakiola.calendula.domain.EventFormField
import de.jeanlucmakiola.calendula.domain.formatGmtOffset import de.jeanlucmakiola.calendula.domain.formatGmtOffset
import de.jeanlucmakiola.calendula.domain.timeZoneOptionOf import de.jeanlucmakiola.calendula.domain.timeZoneOptionOf
import de.jeanlucmakiola.calendula.domain.timesIn
import de.jeanlucmakiola.calendula.domain.EventFormProblem import de.jeanlucmakiola.calendula.domain.EventFormProblem
import de.jeanlucmakiola.calendula.domain.RecurrenceEnd import de.jeanlucmakiola.calendula.domain.RecurrenceEnd
import de.jeanlucmakiola.calendula.domain.RecurrenceFreq import de.jeanlucmakiola.calendula.domain.RecurrenceFreq
@@ -676,6 +677,22 @@ private fun EventEditContent(
color = MaterialTheme.colorScheme.error, color = MaterialTheme.colorScheme.error,
) )
} }
// The rows above edit a pinned event in *its own* zone, which is the
// time it was set at — but that says nothing about when it lands for
// whoever is reading it. Spell the local equivalent out rather than
// leaving the user to do the offset arithmetic. Absent (null) unless
// the event is pinned somewhere other than here.
form.timesIn(TimeZone.currentSystemDefault())?.let { (localStart, localEnd) ->
Spacer(Modifier.height(2.dp))
Text(
text = stringResource(
R.string.event_edit_timezone_local_time,
formatTimeRange(localStart, localEnd, locale),
),
style = MaterialTheme.typography.bodySmall,
color = MaterialTheme.colorScheme.onSurfaceVariant,
)
}
} }
Spacer(Modifier.height(gap)) Spacer(Modifier.height(gap))
@@ -1986,6 +2003,21 @@ private fun EditCard(
} }
} }
/**
* "2:00 PM 3:00 PM", honouring the user's 12/24-hour setting. Collapses to one
* time when both land on the same minute (an instant event shouldn't read as a
* range against itself). Dates are deliberately absent — the rows above carry
* them.
*/
@Composable
private fun formatTimeRange(start: LocalDateTime, end: LocalDateTime, locale: Locale): String {
val use24Hour = LocalUse24HourFormat.current
val timeFormat = remember(locale, use24Hour) { timeOfDayFormatter(use24Hour, locale) }
val from = timeFormat.format(start.time.toJavaLocalTime())
val to = timeFormat.format(end.time.toJavaLocalTime())
return if (from == to) from else "$from $to"
}
/** /**
* Borderless text input used inside the cards (and as the headline title). * Borderless text input used inside the cards (and as the headline title).
* Thin wrapper over the shared [InlineTextField] so the form and the rest of * Thin wrapper over the shared [InlineTextField] so the form and the rest of

View File

@@ -107,6 +107,13 @@
<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> <string name="event_edit_timezone_clear">Use device zone</string>
<!-- 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
range, e.g. "2:00 PM 3:00 PM". -->
<string name="event_edit_timezone_local_time">%1$s your time</string>
<!-- The event's own zone times on the detail screen. %1$s is a time range,
%2$s the zone's city, e.g. "8:00 AM 9:00 AM in New York". -->
<string name="event_detail_timezone_original">%1$s in %2$s</string>
<!-- Event form — per-event color --> <!-- Event form — per-event color -->
<string name="event_edit_color">Color</string> <string name="event_edit_color">Color</string>

View File

@@ -209,6 +209,67 @@ class EventFormTest {
assertThat(prefilled.start).isEqualTo(LocalDateTime(LocalDate(2026, 6, 11), LocalTime(10, 0))) assertThat(prefilled.start).isEqualTo(LocalDateTime(LocalDate(2026, 6, 11), LocalTime(10, 0)))
} }
@Test
fun `timesIn converts a pinned event into the target zone`() {
val form = EventForm(
calendarId = 1L,
start = LocalDateTime(LocalDate(2026, 7, 17), LocalTime(8, 0)),
end = LocalDateTime(LocalDate(2026, 7, 17), LocalTime(9, 0)),
timezone = "America/New_York",
)
val (start, end) = form.timesIn(berlin)!!
// 08:00 New York (EDT, UTC-4) is 14:00 Berlin (CEST, UTC+2).
assertThat(start).isEqualTo(LocalDateTime(LocalDate(2026, 7, 17), LocalTime(14, 0)))
assertThat(end).isEqualTo(LocalDateTime(LocalDate(2026, 7, 17), LocalTime(15, 0)))
}
@Test
fun `timesIn crosses the date line when the offset pushes past midnight`() {
val form = EventForm(
calendarId = 1L,
start = LocalDateTime(LocalDate(2026, 7, 17), LocalTime(20, 0)),
end = LocalDateTime(LocalDate(2026, 7, 17), LocalTime(21, 0)),
timezone = "America/New_York",
)
val (start, _) = form.timesIn(berlin)!!
// 20:00 New York is 02:00 the NEXT day in Berlin — the date has to move
// with it, not just the clock.
assertThat(start).isEqualTo(LocalDateTime(LocalDate(2026, 7, 18), LocalTime(2, 0)))
}
@Test
fun `timesIn tracks each zone's own DST rather than a fixed offset`() {
// In January both are on standard time: 08:00 EST is 14:00 CET — the
// same six hours as July, but only because both shifted. Late March,
// when the US has sprung forward and Europe hasn't, the gap is five.
val march = EventForm(
calendarId = 1L,
start = LocalDateTime(LocalDate(2026, 3, 20), LocalTime(8, 0)),
end = LocalDateTime(LocalDate(2026, 3, 20), LocalTime(9, 0)),
timezone = "America/New_York",
)
assertThat(march.timesIn(berlin)!!.first)
.isEqualTo(LocalDateTime(LocalDate(2026, 3, 20), LocalTime(13, 0)))
}
@Test
fun `timesIn returns null when there is nothing to disambiguate`() {
val base = EventForm(
calendarId = 1L,
start = LocalDateTime(LocalDate(2026, 7, 17), LocalTime(8, 0)),
end = LocalDateTime(LocalDate(2026, 7, 17), LocalTime(9, 0)),
)
// Unpinned: the form's times already are the local times.
assertThat(base.timesIn(berlin)).isNull()
// Pinned to the target itself: same thing.
assertThat(base.copy(timezone = "Europe/Berlin").timesIn(berlin)).isNull()
// All-day: date-anchored, so there's no zone conversion to show.
assertThat(base.copy(isAllDay = true, timezone = "America/New_York").timesIn(berlin))
.isNull()
// Unparseable: can't convert, mustn't throw.
assertThat(base.copy(timezone = "Mars/Olympus_Mons").timesIn(berlin)).isNull()
}
@Test @Test
fun `toEditForm turns the exclusive all-day end into the last covered day`() { fun `toEditForm turns the exclusive all-day end into the last covered day`() {
// 11th..13th = UTC midnights of the 11th and the (exclusive) 14th. // 11th..13th = UTC midnights of the 11th and the (exclusive) 14th.