From 463811056ded88dd3cefbeb47cc670a33916024e Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 20 Jul 2026 15:20:37 +0200 Subject: [PATCH] refactor(ui): one selection check and one motion spec across the pickers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three inconsistencies the recurrence-picker review surfaced, all of the same kind: the redesigned picker had quietly diverged from the family. SelectedCheck existed as five copies app-side plus a private one in the kit, and they had drifted — the kit drew Icons.Rounded.Check, every app copy drew Icons.Default.Check. Delete all five against the kit's now public primitive (floret-kit!2), which settles the glyph on Rounded. The recurrence picker was the only full-screen picker whose selected row carried no check, relying on the tonal highlight alone. Add it to all six rows, matching OptionPicker, reminder, agenda range, timezone, calendar and Settings. Its two new AnimatedVisibility blocks were the only ones in the app using Compose's bare defaults rather than expandEnter()/collapseExit(). Those helpers honour rememberReduceMotion(), so the weekday and count cards were ignoring the system "remove animations" setting — an accessibility regression, not just a style drift. Bumps the floret-kit pointer; needs floret-kit!2 merged first. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/common/CalendarPickerGroups.kt | 10 +---- .../calendula/ui/common/Picker.kt | 10 +---- .../calendula/ui/common/TimeZonePicker.kt | 10 +---- .../calendula/ui/edit/EventEditScreen.kt | 43 ++++++++++++++++++- .../calendula/ui/settings/SettingsScreen.kt | 18 ++------ floret-kit | 2 +- 6 files changed, 49 insertions(+), 44 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/CalendarPickerGroups.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/CalendarPickerGroups.kt index d46ad50..8a25102 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/CalendarPickerGroups.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/CalendarPickerGroups.kt @@ -11,7 +11,6 @@ import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.size import androidx.compose.foundation.shape.CircleShape import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.Check import androidx.compose.material.icons.filled.Cloud import androidx.compose.material.icons.filled.PhoneAndroid import androidx.compose.material3.Icon @@ -33,6 +32,7 @@ import de.jeanlucmakiola.calendula.R import de.jeanlucmakiola.calendula.domain.CalendarSource import de.jeanlucmakiola.floret.components.GroupedRow import de.jeanlucmakiola.floret.components.Position +import de.jeanlucmakiola.floret.components.SelectedCheck /** * The app's single "which calendar" selection list, shared by the event editor @@ -99,13 +99,7 @@ private fun CalendarPickerGroup( selected = isSelected, leading = { CalendarColorChip(calendar.color) }, trailing = if (isSelected) { - { - Icon( - imageVector = Icons.Default.Check, - contentDescription = null, - tint = MaterialTheme.colorScheme.primary, - ) - } + { SelectedCheck() } } else { null }, 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 1aefd87..c038dee 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 @@ -5,7 +5,6 @@ import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.Check import androidx.compose.material3.Checkbox import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme @@ -26,6 +25,7 @@ import de.jeanlucmakiola.floret.components.CustomAmountEditor import de.jeanlucmakiola.floret.components.FullScreenPicker import de.jeanlucmakiola.floret.components.GroupedRow import de.jeanlucmakiola.floret.components.Position +import de.jeanlucmakiola.floret.components.SelectedCheck import de.jeanlucmakiola.floret.components.positionOf import de.jeanlucmakiola.floret.identity.collapseExit import de.jeanlucmakiola.floret.identity.expandEnter @@ -207,14 +207,6 @@ private fun PickerDescription(text: String) { ) } -@Composable -private fun SelectedCheck() { - Icon( - imageVector = Icons.Default.Check, - contentDescription = null, - tint = MaterialTheme.colorScheme.primary, - ) -} /** * Agenda-range picker, full-screen. Two grouped lists — the calendar-aligned 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 fc5d472..cf20da7 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 @@ -9,7 +9,6 @@ import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons -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.Search @@ -41,6 +40,7 @@ import de.jeanlucmakiola.floret.components.FullScreenPicker import de.jeanlucmakiola.floret.components.GroupedRow import de.jeanlucmakiola.floret.components.InlineTextField import de.jeanlucmakiola.floret.components.Position +import de.jeanlucmakiola.floret.components.SelectedCheck import de.jeanlucmakiola.floret.components.positionOf import de.jeanlucmakiola.floret.locale.currentLocale @@ -223,14 +223,6 @@ private fun SectionHeader(text: String) { ) } -@Composable -private fun SelectedCheck() { - Icon( - imageVector = Icons.Default.Check, - contentDescription = null, - tint = MaterialTheme.colorScheme.primary, - ) -} @Composable private fun ZoneRow( 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 13dd8ed..019c830 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 @@ -145,6 +145,7 @@ import de.jeanlucmakiola.floret.components.InlineTextField import de.jeanlucmakiola.floret.components.OptionCard import de.jeanlucmakiola.floret.components.OptionPicker import de.jeanlucmakiola.floret.components.Position +import de.jeanlucmakiola.floret.components.SelectedCheck import de.jeanlucmakiola.floret.components.groupedShape as floretGroupedShape import de.jeanlucmakiola.floret.components.positionOf import de.jeanlucmakiola.calendula.ui.common.REMINDER_PRESETS @@ -1379,6 +1380,11 @@ private fun RecurrencePickerDialog( title = stringResource(R.string.event_edit_recurrence_none), position = positionOf(0, rowCount), selected = current == null, + trailing = if (current == null) { + { SelectedCheck() } + } else { + null + }, onClick = { onSelect(null) }, ) RecurrenceFreq.entries.forEachIndexed { index, entry -> @@ -1386,6 +1392,11 @@ private fun RecurrencePickerDialog( title = stringResource(recurrencePresetLabel(entry)), position = positionOf(index + 1, rowCount), selected = isPlainPreset && parsed?.freq == entry, + trailing = if (isPlainPreset && parsed?.freq == entry) { + { SelectedCheck() } + } else { + null + }, onClick = { onSelect(SimpleRecurrence(entry).toRRule()) }, ) } @@ -1393,6 +1404,11 @@ private fun RecurrencePickerDialog( title = stringResource(R.string.event_edit_recurrence_custom), position = positionOf(rowCount - 1, rowCount), selected = current != null && !isPlainPreset, + trailing = if (current != null && !isPlainPreset) { + { SelectedCheck() } + } else { + null + }, onClick = { customMode = true }, ) } else { @@ -1477,7 +1493,11 @@ private fun RecurrencePickerDialog( // Weekday picks — only meaningful on a weekly rule. The 16dp gap // sits inside the animated block so it grows/shrinks with the card. - AnimatedVisibility(visible = freq == RecurrenceFreq.Weekly) { + AnimatedVisibility( + visible = freq == RecurrenceFreq.Weekly, + enter = expandEnter(), + exit = collapseExit(), + ) { GroupedSurface( position = Position.Alone, modifier = Modifier @@ -1507,6 +1527,11 @@ private fun RecurrencePickerDialog( title = stringResource(R.string.event_edit_recurrence_end_never), position = Position.Top, selected = endMode == RecurrenceEndMode.Never, + trailing = if (endMode == RecurrenceEndMode.Never) { + { SelectedCheck() } + } else { + null + }, onClick = { endMode = RecurrenceEndMode.Never }, ) GroupedRow( @@ -1514,6 +1539,11 @@ private fun RecurrencePickerDialog( summary = untilSummary, position = Position.Middle, selected = endMode == RecurrenceEndMode.Until, + trailing = if (endMode == RecurrenceEndMode.Until) { + { SelectedCheck() } + } else { + null + }, onClick = { endMode = RecurrenceEndMode.Until showUntilPicker = true @@ -1527,12 +1557,21 @@ private fun RecurrencePickerDialog( Position.Bottom }, selected = endMode == RecurrenceEndMode.Count, + trailing = if (endMode == RecurrenceEndMode.Count) { + { SelectedCheck() } + } else { + null + }, onClick = { endMode = RecurrenceEndMode.Count }, ) // Animated for the same reason the weekday card is: this is the // picker's other collapsible block, and a bare `if` here made // the ends group snap while the card above it glided. - AnimatedVisibility(visible = endMode == RecurrenceEndMode.Count) { + AnimatedVisibility( + visible = endMode == RecurrenceEndMode.Count, + enter = expandEnter(), + exit = collapseExit(), + ) { GroupedSurface( position = Position.Bottom, modifier = Modifier.padding(horizontal = 16.dp), diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsScreen.kt index 7339a75..712635b 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsScreen.kt @@ -43,7 +43,6 @@ import androidx.compose.material.icons.filled.BugReport import androidx.compose.material.icons.filled.Cake import androidx.compose.material.icons.filled.CalendarMonth import androidx.compose.material.icons.filled.Dashboard -import androidx.compose.material.icons.filled.Check import androidx.compose.material.icons.filled.DragHandle import androidx.compose.material.icons.filled.SwapVert import androidx.compose.material.icons.filled.ExpandLess @@ -131,6 +130,7 @@ import de.jeanlucmakiola.floret.components.Position import de.jeanlucmakiola.calendula.ui.common.REMINDER_PRESETS import de.jeanlucmakiola.calendula.ui.common.ReminderDefaultPicker import de.jeanlucmakiola.calendula.ui.common.TimePickerAlert +import de.jeanlucmakiola.floret.components.SelectedCheck import de.jeanlucmakiola.floret.components.positionOf import de.jeanlucmakiola.calendula.ui.common.reminderLeadTimeLabel import de.jeanlucmakiola.calendula.ui.common.SnoozeDurationPicker @@ -1093,13 +1093,7 @@ private fun NotificationsScreen( }, position = Position.Top, trailing = if (batteryExempt) { - { - Icon( - imageVector = Icons.Default.Check, - contentDescription = null, - tint = MaterialTheme.colorScheme.primary, - ) - } + { SelectedCheck() } } else { null }, @@ -1845,13 +1839,7 @@ private fun FontOptionRow( } }, trailing = if (selected) { - { - Icon( - imageVector = Icons.Default.Check, - contentDescription = null, - tint = MaterialTheme.colorScheme.primary, - ) - } + { SelectedCheck() } } else { null }, diff --git a/floret-kit b/floret-kit index df8bdba..b495471 160000 --- a/floret-kit +++ b/floret-kit @@ -1 +1 @@ -Subproject commit df8bdbaf73380a354b7d2ee28e08875e155c89cd +Subproject commit b4954713ff2a476b8b4e4c1f93d9267547a757e4