From 9bcd08a5bb151a1128fc1c8a0438952ba4987977 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 30 Jul 2026 21:57:15 +0200 Subject: [PATCH] docs: trim the code comments back to what the code doesn't say The v2.17.0 work left long prose comments explaining rationale that either repeats docs/ARCHITECTURE.md or restates the line below it. Cut ~520 comment lines across 54 files, keeping the short "why" notes for provider quirks and non-obvious flow behaviour. --- .../jeanlucmakiola/calendula/CalendulaApp.kt | 24 ++-- .../data/calendar/AllDayReminderEncoding.kt | 5 +- .../data/calendar/CalendarDataSource.kt | 27 ++-- .../calendula/data/calendar/CalendarMapper.kt | 4 +- .../data/calendar/CalendarRepository.kt | 12 +- .../data/calendar/CalendarRepositoryImpl.kt | 57 +++----- .../calendar/CalendarVisibilityReconciler.kt | 62 +++------ .../calendula/data/prefs/CalendarPrefs.kt | 27 ++-- .../data/prefs/ReminderStatePrefs.kt | 18 +-- .../data/reminders/ReminderActionReceiver.kt | 20 +-- .../data/reminders/ReminderAlarms.kt | 15 +-- .../calendula/data/reminders/ReminderAlert.kt | 11 +- .../data/reminders/ReminderInstanceSource.kt | 25 ++-- .../reminders/ReminderMaintenanceWorker.kt | 13 +- .../data/reminders/ReminderNotifier.kt | 15 +-- .../data/reminders/ReminderScanner.kt | 42 +++--- .../reminders/ReminderScheduleReceiver.kt | 28 ++-- .../data/reminders/ReminderSnoozeScheduler.kt | 20 ++- .../calendula/domain/CalendarRowState.kt | 38 ++---- .../domain/CalendarVisibilityPlan.kt | 28 ++-- .../jeanlucmakiola/calendula/domain/Models.kt | 28 ++-- .../domain/reminders/ReminderPlan.kt | 125 ++++-------------- .../jeanlucmakiola/calendula/ui/RootScreen.kt | 11 +- .../calendula/ui/calendars/BackupScreen.kt | 49 +++---- .../ui/calendars/CalendarVisibilityNotice.kt | 15 +-- .../calendula/ui/calendars/CalendarsScreen.kt | 37 ++---- .../ui/calendars/CalendarsViewModel.kt | 37 ++---- .../calendula/ui/common/AccountGroups.kt | 14 +- .../ui/common/CalendarPickerGroups.kt | 8 +- .../calendula/ui/common/RecurrenceText.kt | 18 +-- .../calendula/ui/edit/EventEditScreen.kt | 39 ++---- .../calendula/ui/edit/EventEditViewModel.kt | 8 +- .../calendula/ui/filter/FilterViewModel.kt | 5 +- .../calendula/ui/imports/ImportScreen.kt | 6 +- .../calendula/ui/imports/ImportViewModel.kt | 4 +- .../calendula/ui/month/MonthScreen.kt | 13 +- .../calendula/ui/month/MonthUiState.kt | 9 +- .../ui/permission/PermissionViewModel.kt | 4 +- .../permission/ReminderOnboardingViewModel.kt | 3 +- .../calendula/ui/search/SearchScreen.kt | 7 +- .../ui/settings/AppearanceSettings.kt | 80 +++-------- .../ui/settings/EventFormSettings.kt | 10 +- .../ui/settings/NotificationSettings.kt | 30 ++--- .../calendula/ui/settings/SettingsCommon.kt | 48 ++----- .../calendula/ui/settings/SettingsScreen.kt | 73 ++++------ .../ui/settings/SettingsViewModel.kt | 81 ++++-------- .../ui/settings/SpecialDatesSettings.kt | 13 +- .../calendula/ui/settings/ViewsSettings.kt | 24 +--- .../calendula/ui/settings/WeekStartPicker.kt | 19 +-- .../calendula/ui/settings/WidgetSettings.kt | 26 ++-- .../calendula/widget/WidgetSize.kt | 16 +-- .../calendula/widget/agenda/AgendaScale.kt | 35 ++--- .../calendula/widget/agenda/AgendaWidget.kt | 18 +-- .../calendar/CalendarRepositoryImplTest.kt | 8 +- 54 files changed, 431 insertions(+), 981 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt b/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt index 55c2595..c11701e 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/CalendulaApp.kt @@ -47,12 +47,9 @@ class CalendulaApp : Application() { } /** - * Bring reminder delivery up with the process (#75). The app plans and arms - * its own reminder alarms now, so launch is one of the moments that has to - * re-check them: a scan re-arms whatever the system dropped, posts anything - * a missed alarm still owes, and starts watching the provider so an edit - * re-plans without waiting for the next pass. The daily worker is the - * backstop for a device that drops the alarm with no reboot to announce it. + * Bring reminder delivery up with the process (#75): a scan re-arms whatever + * the system dropped and posts what a missed alarm still owes, then the + * provider watch keeps edits re-planned. The daily worker is the backstop. */ private fun startReminderDelivery() { val deps = EntryPointAccessors.fromApplication( @@ -65,12 +62,9 @@ class CalendulaApp : Application() { } /** - * Flush any calendar switch-off the app hasn't been allowed to write into - * the system's `Calendars.VISIBLE` yet — including the retired app-local - * "disabled calendars" set the upgrade inherits (#75). A no-op on a fresh - * install and in the steady state; a launch without the calendar permission - * leaves the set pending, and `RootScreen` runs it again once the app comes - * up holding it — whichever way it was granted. + * Flush any calendar switch-off not yet written to `Calendars.VISIBLE`, + * including the set inherited from the retired app-local model (#75). A + * no-op in the steady state; `RootScreen` re-runs it after a later grant. */ private fun reconcileCalendarVisibility() { val deps = EntryPointAccessors.fromApplication( @@ -101,10 +95,8 @@ class CalendulaApp : Application() { /** * Re-arm (or cancel) the daily special-dates reconcile from the saved - * settings on every launch — like [reconcileAutoBackup], this cancels - * orphaned work after the feature is turned off and re-schedules after a - * reinstall. The foreground trigger (RootScreen ON_RESUME) does the on-open - * refresh, so no immediate run is needed here. + * settings, like [reconcileAutoBackup]. The on-open refresh is RootScreen's + * ON_RESUME trigger, so no immediate run is needed here. */ private fun reconcileSpecialDates() { val deps = EntryPointAccessors.fromApplication(this, SpecialDatesSyncWorker.Deps::class.java) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/AllDayReminderEncoding.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/AllDayReminderEncoding.kt index adaeb0c..94021e6 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/AllDayReminderEncoding.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/AllDayReminderEncoding.kt @@ -73,9 +73,8 @@ internal fun nextYearlyOccurrence(month: Int, day: Int, today: LocalDate): Local /** * Recover the semantic whole-day lead time from a raw all-day reminder * [rawMinutes] — the inverse of [toProviderAllDayMinutes], for the form and the - * detail screen. Delegates to [allDayLeadDays] so what is displayed is the day - * the reminder actually fires on; see there for how an encoded row is told from - * a plain one written by another calendar app. + * detail screen. Delegates to [allDayLeadDays], so the day displayed is the day + * the reminder actually fires on. */ internal fun fromProviderAllDayMinutes( rawMinutes: Int, diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt index 17e62ee..63fff41 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt @@ -117,19 +117,15 @@ interface CalendarDataSource { /** * Show or hide the calendar device-wide by writing `Calendars.VISIBLE` — the - * app's one visibility model (#75). `VISIBLE` also gates the provider's own - * reminder scheduling, so switching a calendar off here is what actually - * stops its notifications; switching it on is what brings them back. - * Writable by a plain app (one of the three columns the platform documents - * as such) and device-local — no sync adapter pushes it anywhere. + * app's one visibility model (#75), which also gates its reminders. One of + * the three columns the platform documents as app-writable, and device-local. */ fun setCalendarVisible(id: Long, visible: Boolean) /** - * Whether one calendar is currently switched on at system level, without - * reading every row — for the reminder gate, which sees a calendar id and - * nothing else. Null when the answer can't be had: no row (the calendar was - * deleted) or no read permission. + * Whether one calendar is switched on at system level, without reading every + * row — for the reminder gate, which sees only a calendar id. Null when the + * answer can't be had: no row, or no read permission. */ fun isCalendarVisible(id: Long): Boolean? @@ -405,14 +401,11 @@ class AndroidCalendarDataSource @Inject constructor( } /** - * Addressed by appended id on the plain (non-sync-adapter) Calendars URI, - * one calendar per call. Both parts are load-bearing: - * `CalendarProvider2.updateInTransaction` short-circuits to a raw database - * update unless the selection is `_id=…`, skipping the dirty marking *and* - * the `checkNextAlarm()` reschedule — i.e. an `_id IN (…)` batch would write - * the flag but never re-arm the reminder alarms this write exists to - * trigger. The sync-adapter URI is avoided so the write also applies to - * synced calendars, which is where the bug bites. + * Addressed by appended id on the plain (non-sync-adapter) Calendars URI, one + * calendar per call. Both parts are load-bearing: + * `CalendarProvider2.updateInTransaction` skips the dirty marking and the + * `checkNextAlarm()` reschedule unless the selection is `_id=…`, and the + * plain URI is what makes the write apply to synced calendars too. */ override fun setCalendarVisible(id: Long, visible: Boolean) { val values = ContentValues().apply { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarMapper.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarMapper.kt index af2ac96..edfbd4a 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarMapper.kt @@ -31,9 +31,7 @@ internal fun ColumnReader.toCalendarSource(): CalendarSource { isManaged = isLocal && getString(CalendarProjection.IDX_MANAGED_MARKER) ?.startsWith(CalendarProjection.MANAGED_MARKER_PREFIX) == true, - // A provider that leaves the column NULL is treated as syncing — the - // harmless default, since this flag only ever holds the one-shot - // visibility migration back from switching a calendar on. + // NULL is treated as syncing — the harmless default. syncsEvents = isNull(CalendarProjection.IDX_SYNC_EVENTS) || getInt(CalendarProjection.IDX_SYNC_EVENTS) != 0, ) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepository.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepository.kt index 066d9af..f8c6212 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepository.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepository.kt @@ -39,16 +39,12 @@ interface CalendarRepository { suspend fun deleteCalendar(id: Long) /** - * Show or hide [ids] device-wide (`Calendars.VISIBLE`), which is also what - * turns the provider's reminder scheduling for them on or off — see - * [CalendarDataSource.setCalendarVisible]. Each calendar is written on its - * own, in order; a failure part-way leaves the earlier writes standing (the - * observer reports whatever actually landed). + * Show or hide [ids] device-wide (`Calendars.VISIBLE`), which also gates + * their reminders — see [CalendarDataSource.setCalendarVisible]. Written one + * at a time, in order; a failure part-way leaves the earlier writes standing. * * Without `WRITE_CALENDAR` the choice is kept app-side instead (see - * [de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs.pendingDisabledCalendarIds]), - * where it filters events and reminders just the same until it can be - * written. + * [de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs.pendingDisabledCalendarIds]). */ suspend fun setCalendarsVisible(ids: Collection, visible: Boolean) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt index 9d57346..8d64cab 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt @@ -68,23 +68,17 @@ class CalendarRepositoryImpl @Inject constructor( /** * Re-query signal for everything filtered by visibility: the provider's own - * notifications, plus every change to the pending switch-off set (an id - * leaves it as its `VISIBLE` write lands, which changes what is shown). - * [calendarsSnapshot] keeps the two in step. + * notifications, plus every change to the pending switch-off set. */ private fun visibilityTicks(): Flow = merge( ticks.onStart { emit(Unit) }, - // The current value is already covered by the tick above; only later - // changes re-query (the set is deduped, so an unrelated DataStore write - // doesn't). + // drop(1): the current value is already covered by the tick above. prefs.pendingDisabledCalendarIds.drop(1).map {}, ) - // A switch-off the app hasn't been allowed to write yet is folded into the - // flag itself, so every consumer — the Settings switch, the filter sheet, - // the form and import pickers, the widgets — reads one visibility and can't - // disagree with what the user just tapped. The reconciler reads the data - // source directly, because it needs the provider's own answer. + // A switch-off not yet written to the provider is folded into the flag + // itself, so every consumer reads one visibility (#75). The reconciler goes + // to the data source directly — it needs the provider's own answer. override fun calendars(): Flow> = visibilityTicks().reQuery { val calendars = calendarsSnapshot() @@ -94,19 +88,13 @@ class CalendarRepositoryImpl @Inject constructor( if (it.id in pendingDisabled) it.copy(isVisibleInSystem = false) else it } } - // Collapse re-emissions that carry an identical list (see - // [instances]). .distinctUntilChanged() .flowOn(io) - // Instances are filtered by the system's per-calendar VISIBLE flag ∪ the - // switch-offs still waiting to be written to it ∪ the app-side hidden set: - // an event is dropped when the user switched its calendar off in Settings → - // Calendars (which also stops the provider scheduling its reminders) *or* - // hid it in the filter sheet. Re-runs when the provider ticks — writing - // VISIBLE notifies, so switching a calendar updates every view — or when - // either set changes. [calendars] stays unfiltered so those screens can list - // and re-enable invisible calendars. + // Instances are filtered by the system's VISIBLE flag ∪ the switch-offs + // still waiting to be written to it ∪ the app-side hidden set from the + // filter sheet. [calendars] stays unfiltered so those screens can list and + // re-enable invisible calendars. override fun instances(range: ClosedRange): Flow> = combine( visibilityTicks().reQuery { @@ -127,10 +115,8 @@ class CalendarRepositoryImpl @Inject constructor( if (excluded.isEmpty()) queried.instances else queried.instances.filterNot { it.calendarId in excluded } } - // Any DataStore edit re-emits the hidden set even when it is - // unchanged (e.g. writing the last-used calendar), which would - // re-surface an identical list — collapse those so views don't - // re-render for them. + // Any DataStore edit re-emits the hidden set even when unchanged; + // collapse those so views don't re-render for them. .distinctUntilChanged() .flowOn(io) @@ -151,21 +137,14 @@ class CalendarRepositoryImpl @Inject constructor( private var cachedCalendars: List = emptyList() /** - * The calendar list for the current tick, queried once and shared. Every - * open view collects [calendars] *and* filters its instances by visibility, - * which used to cost one full `Calendars` query each per tick. Reusing a - * single read also keeps them consistent: within a tick, what a screen lists - * and what its events are filtered against can't come from two snapshots. + * The calendar list for the current tick, queried once and shared, so every + * open view doesn't pay for its own `Calendars` query and all of them see + * one snapshot. * - * An empty result is never cached — it is what a read without the calendar - * permission returns, and the grant itself doesn't notify the provider. - * - * The pending switch-off set keys the cache alongside the tick. An id leaves - * that set the moment its `VISIBLE` write lands, while the observer that - * would invalidate the snapshot is only dispatched through the main looper - * afterwards — so a snapshot taken while the id was still pending, read - * against the set that no longer holds it, would report the calendar as *on* - * again and re-admit exactly the events being hidden. + * An empty result is never cached — that is what a read without the calendar + * permission returns, and the grant itself doesn't notify the provider. The + * pending switch-off set keys the cache alongside the tick, because an id + * leaves it before the invalidating observer is dispatched. */ private suspend fun calendarsSnapshot(): List = calendarsLock.withLock { val current = generation.get() diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarVisibilityReconciler.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarVisibilityReconciler.kt index b79c039..6678eeb 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarVisibilityReconciler.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarVisibilityReconciler.kt @@ -21,25 +21,15 @@ import javax.inject.Inject import javax.inject.Singleton /** - * Keeps the app's pending "switched off" set (see - * [CalendarPrefs.pendingDisabledCalendarIds]) and the system's - * `Calendars.VISIBLE` in step — the fold-in of the retired app-local visibility - * model (#75), and the standing drain for switch-offs made without + * Keeps [CalendarPrefs.pendingDisabledCalendarIds] and the system's + * `Calendars.VISIBLE` in step (#75): the fold-in of the retired app-local + * visibility model, and the standing drain for switch-offs made without * `WRITE_CALENDAR`. * - * Runs on every launch, and again whenever the app comes up holding the calendar - * permission — a grant made on Android's own app-settings screen never reaches - * the permission screen's callback. It is a no-op whenever the pending set is - * empty and the notice has been settled, which - * is the steady state: each entry is written and dropped individually, so a run - * that dies part-way resumes exactly where it stopped and never re-applies a - * write the user has since undone by hand. - * - * The reconciliation only hides (see [calendarVisibilityPlan]). Calendars hidden - * at system level stay hidden, and on an *upgraded* install the first run that - * sees one arms the one-time notice explaining why Calendula no longer lists - * their events. A fresh install never had the old behaviour, so it retires that - * notice unshown — every other device ships with something hidden. + * Runs on every launch and on every calendar-permission grant; a no-op once the + * pending set is empty and the notice is settled. Only ever hides (see + * [calendarVisibilityPlan]); on an upgraded install the first run that sees a + * system-hidden calendar arms the one-time explanatory notice. */ @Singleton class CalendarVisibilityReconciler @Inject constructor( @@ -50,43 +40,30 @@ class CalendarVisibilityReconciler @Inject constructor( ) { suspend fun run() = withContext(io) { - // Everything, the DataStore reads included, sits inside the guard: this - // runs in a bare application-scope coroutine with no exception handler, - // so an IOException from a damaged preferences file would otherwise take - // the process down on every launch. + // The DataStore reads sit inside the guard too: this runs in a bare + // application-scope coroutine, so an IOException from a damaged + // preferences file would take the process down on every launch. try { - // A fresh install has no retired model behind it — nothing to - // migrate, and nothing to explain. Settled ahead of the permission - // gate so an update installed before the first grant can't make a - // first run look like an upgrade afterwards. + // Settled ahead of the permission gate, so an update installed + // before the first grant can't later look like an upgrade. if (!isUpgradeInstall()) settleNoticeOnce(pending = false) if (!hasPermission(Manifest.permission.READ_CALENDAR)) return@withContext val pending = prefs.pendingDisabledCalendarIds.first() val noticeSettled = prefs.visibilityNoticePending.first() != null - // The steady state, and every run after the first: nothing left to - // drain and nothing left to decide, so don't pay for the query. if (pending.isEmpty() && noticeSettled) return@withContext val calendars = dataSource.calendars() - // An empty read means "couldn't read", not "no calendars": the data - // source turns a null cursor — a provider momentarily unavailable — - // into an empty list. Both decisions below are one-way, so taking - // that reading as the truth would drop the whole pending set without - // ever writing VISIBLE = 0 (switching the user's calendars back on, - // events and reminders with them) and settle the notice as "nothing - // to explain". Leave both to the next run. + // Empty means "couldn't read" (null cursor), not "no calendars". + // Both decisions below are one-way, so leave them to the next run. if (calendars.isEmpty()) return@withContext settleNoticeOnce(hasSystemHiddenCalendars(calendars, pending)) if (pending.isEmpty() || !hasPermission(Manifest.permission.WRITE_CALENDAR)) { return@withContext } val plan = calendarVisibilityPlan(calendars, pending) - // Already off, or gone from the device — nothing to write, so let - // those ids leave the pending set with the rest. prefs.removePendingDisabledCalendarIds(plan.settled) // One calendar per write: the provider skips its reminder-alarm // reschedule for anything but a single-id update (see - // [CalendarDataSource.setCalendarVisible]). Dropping each id as it - // lands keeps a part-applied run resumable. + // [CalendarDataSource.setCalendarVisible]). for (id in plan.hide) { dataSource.setCalendarVisible(id, false) prefs.removePendingDisabledCalendarIds(setOf(id)) @@ -100,9 +77,7 @@ class CalendarVisibilityReconciler @Inject constructor( /** * Settle the one-time notice: [pending] arms it, false retires it unshown. - * Answered once, by whichever run can answer it first; the answer is stored - * either way, so the notice can't resurface later, when the same state would - * no longer be news to the user. + * Answered once and stored either way, so it can't resurface later. */ private suspend fun settleNoticeOnce(pending: Boolean) { if (prefs.visibilityNoticePending.first() != null) return @@ -111,10 +86,7 @@ class CalendarVisibilityReconciler @Inject constructor( /** * Whether this install has ever run an earlier version. The notice explains - * a change to behaviour the user has already seen, so a first install has - * nothing to announce — and hidden calendars are the *norm* on a fresh - * device (a second account's, "Holidays in …", a subscribed calendar), which - * would otherwise put a changelog dialog in front of a first-run user. + * a behaviour change, so a first install has nothing to announce. */ private fun isUpgradeInstall(): Boolean = try { @Suppress("DEPRECATION") diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/CalendarPrefs.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/CalendarPrefs.kt index e615803..b0bc1e7 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/CalendarPrefs.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/CalendarPrefs.kt @@ -29,10 +29,8 @@ class CalendarPrefs @Inject constructor( private val store: DataStore, ) { - // Both id sets are deduped: the store is shared with SettingsPrefs, so every - // unrelated write (a settings toggle, the last-used calendar) re-emits an - // identical set otherwise — and a change to the pending set now costs a - // fresh provider read in CalendarRepositoryImpl. + // Both id sets are deduped: the store is shared with SettingsPrefs, so any + // unrelated write would otherwise re-emit an identical set. val hiddenCalendarIds: Flow> = store.data .map { prefs -> prefs[HIDDEN_IDS_KEY].parseIds() } .distinctUntilChanged() @@ -42,17 +40,13 @@ class CalendarPrefs @Inject constructor( } /** - * Calendars switched off in Settings → Calendars that the provider does not - * know about yet. That switch writes the system's `Calendars.VISIBLE` (#75), - * which needs `WRITE_CALENDAR` — a user who granted read-only keeps their - * choice here instead, and so does everyone upgrading from the retired - * app-local model, whose set is read straight back out of the same key. + * Switch-offs the provider does not know about yet, because writing + * `Calendars.VISIBLE` needs `WRITE_CALENDAR` (#75). Also inherits the + * retired app-local model's set, from the same key. * - * Honoured as a display and reminder filter for as long as it is non-empty, - * so an un-flushable switch still does what the user asked. Not a second - * visibility model: `CalendarVisibilityReconciler` drains it into the - * provider entry by entry the moment the app may write, and nothing ever - * adds to it while it may. + * Honoured as a display and reminder filter while non-empty, but not a + * second visibility model: `CalendarVisibilityReconciler` drains it entry by + * entry as soon as the app may write, and nothing adds to it while it may. */ val pendingDisabledCalendarIds: Flow> = store.data .map { prefs -> prefs[DISABLED_IDS_KEY].parseIds() } @@ -62,9 +56,8 @@ class CalendarPrefs @Inject constructor( editPendingDisabled { it + ids } /** - * Drop [ids] from the pending set — one id at a time as the reconciler - * flushes it, so a run that fails part-way never re-applies what already - * landed (and can't undo a switch the user has since flipped by hand). + * Drop [ids] from the pending set, one at a time as the reconciler flushes + * them, so a run that fails part-way never re-applies what already landed. */ suspend fun removePendingDisabledCalendarIds(ids: Collection) = editPendingDisabled { it - ids.toSet() } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/ReminderStatePrefs.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/ReminderStatePrefs.kt index 053a2e5..7dc927a 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/ReminderStatePrefs.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/ReminderStatePrefs.kt @@ -10,19 +10,13 @@ import javax.inject.Inject import javax.inject.Singleton /** - * How far reminder delivery has got. One number, and it replaces everything the - * provider's `CalendarAlerts.STATE` used to do for us (#75). + * How far reminder delivery has got — the one number replacing the provider's + * `CalendarAlerts.STATE` (#75). A scan posts the reminders falling after this + * watermark and up to now, then moves it to now, so a scan running twice cannot + * post twice while a late one still catches up. * - * A scan posts the reminders whose moment falls after this watermark and up to - * now, then moves it to now. That single rule gives both halves of what the - * retired path got from the provider: a scan that runs twice cannot post the - * same reminder again, and a scan that runs *late* — after a reboot, an app - * update or a doze window swallowed the alarm — still posts everything the - * missed alarm would have. - * - * Unset means "never scanned". It is deliberately not treated as zero: the first - * scan after an install or an upgrade would otherwise consider every reminder - * since the epoch overdue and bury the user in notifications. + * Unset means "never scanned", not zero: the first scan after an install would + * otherwise treat every reminder since the epoch as overdue. */ @Singleton class ReminderStatePrefs @Inject constructor( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderActionReceiver.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderActionReceiver.kt index 20d5221..d62ee0d 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderActionReceiver.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderActionReceiver.kt @@ -93,13 +93,9 @@ class ReminderActionReceiver : BroadcastReceiver() { private const val EXTRA_ALL_DAY = "all_day" /** - * An explicit intent to this receiver carrying [alert] as extras. - * - * The data URI names the reminder and carries no information the extras - * don't. It is what actually keeps two reminders' `PendingIntent`s - * apart: `filterEquals` compares action, component, data, type and - * categories — never extras — so without it a request-code collision - * would have them share one alarm and one payload. + * An explicit intent to this receiver carrying [alert] as extras. The + * data URI duplicates no information but is what keeps two reminders' + * `PendingIntent`s apart — `filterEquals` never compares extras. */ fun intent(context: Context, action: String, alert: ReminderAlert): Intent = Intent(context, ReminderActionReceiver::class.java).apply { @@ -116,13 +112,9 @@ class ReminderActionReceiver : BroadcastReceiver() { } /** - * A stable request code per (alert, action) so one notification's - * PendingIntents stay distinct and don't clobber each other. - * - * The key is a hash now rather than a small row id, so the shift is what - * keeps the action slot intact; the top three bits it drops can make two - * *different* reminders share a code, which the per-reminder data URI in - * [intent] separates. + * A stable request code per (alert, action), so one notification's + * PendingIntents stay distinct. The shift keeps the action slot intact; + * the top bits it drops are separated by [intent]'s per-reminder URI. */ fun requestCode(alert: ReminderAlert, action: String): Int { val actionOffset = when (action) { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlarms.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlarms.kt index 89a4593..1fd5a75 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlarms.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlarms.kt @@ -19,17 +19,10 @@ internal fun AlarmManager.canScheduleExactCompat(): Boolean = Build.VERSION.SDK_INT < Build.VERSION_CODES.S || canScheduleExactAlarms() /** - * Holds the app's own wake-up for the next reminder — the half of delivery that - * used to be the provider's (#75). - * - * Exactly **one** alarm exists at a time, for the earliest reminder still ahead. - * Every firing re-scans and re-arms, so a reminder added, moved or deleted in - * between is picked up on the next pass instead of needing an alarm per reminder - * to be kept in sync with the provider's tables. - * - * A reminder that lands late is a broken reminder, hence an *exact* alarm; the - * inexact allow-while-idle fallback only applies where the OS withholds the - * capability (API 31–32 with the user's permission revoked). + * Holds the app's own wake-up for the next reminder (#75). Exactly one alarm + * exists at a time, for the earliest reminder ahead; every firing re-scans and + * re-arms. Exact, with an inexact allow-while-idle fallback where the OS + * withholds the capability (API 31–32 with the permission revoked). */ @Singleton class ReminderAlarmScheduler @Inject constructor( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlert.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlert.kt index 2d8c28b..8a5630a 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlert.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderAlert.kt @@ -4,14 +4,9 @@ import de.jeanlucmakiola.calendula.domain.reminders.PlannedReminder /** * One reminder as the notification layer needs it: what to show, and the stable - * [key] that identifies it across a reboot, a re-scan and a reinstall. - * - * [key] used to be the `CalendarAlerts` row id. It is now derived from the - * reminder itself (see [PlannedReminder.key]) because there is no row any more — - * in-house delivery reads `Instances` and `Reminders` and owns the alarm (#75). - * Everything downstream only ever needed it to be stable and unique, which it - * still is: it keys the notification tag, so a reminder posted twice replaces - * itself instead of stacking, and it keys the snooze/dismiss `PendingIntent`s. + * [key] identifying it across a reboot, a re-scan and a reinstall. Derived from + * the reminder itself (see [PlannedReminder.key]) — in-house delivery has no + * `CalendarAlerts` row to take an id from (#75). */ data class ReminderAlert( val key: Long, diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderInstanceSource.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderInstanceSource.kt index 7ab93ee..3b1c2e9 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderInstanceSource.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderInstanceSource.kt @@ -9,16 +9,11 @@ import javax.inject.Inject import javax.inject.Singleton /** - * The read side of in-house reminder delivery: occurrences and the reminder - * offsets hanging off them, straight out of the provider's own tables. + * The read side of in-house reminder delivery: occurrences and their reminder + * offsets, read from `Instances` and `Reminders` rather than `CalendarAlerts`, + * which cannot be assumed to be written (#75). * - * Deliberately *not* `CalendarAlerts`. That table is the provider's own working - * copy of this same information, and the whole point of #75's second round is - * that we can no longer assume it gets written. `Instances` and `Reminders` are - * plain data the app already reads everywhere else. - * - * An interface so [ReminderScanner] can be exercised on the JVM without a - * ContentResolver, in the shape the rest of `data/calendar` uses. + * An interface so [ReminderScanner] can be exercised on the JVM. */ interface ReminderInstanceSource { @@ -46,11 +41,9 @@ class ProviderReminderInstanceSource @Inject constructor( ContentUris.appendId(this, fromMillis) ContentUris.appendId(this, toMillis) }.build() - // `visible` is the calendar's system flag, which the app's one visibility - // model writes (#75) — a switched-off calendar must not plan reminders. - // The status clause mirrors CalendarDataSource.instances: a cancelled - // single occurrence of a series is a real row, and NULL means "normal", - // so a bare `!= CANCELED` would drop every ordinary event. + // `visible` is the flag the app's one visibility model writes (#75). + // The status clause mirrors CalendarDataSource.instances: NULL means + // "normal", so a bare `!= CANCELED` would drop every ordinary event. val selection = "${CalendarContract.Calendars.VISIBLE} = 1 AND " + "(${CalendarContract.Instances.STATUS} IS NULL OR " + "${CalendarContract.Instances.STATUS} != ${CalendarContract.Events.STATUS_CANCELED})" @@ -78,8 +71,8 @@ class ProviderReminderInstanceSource @Inject constructor( override fun reminderMinutes(eventIds: Collection): Map> { if (eventIds.isEmpty()) return emptyMap() val out = mutableMapOf>() - // Batched because the ids go into the selection literally; an unbounded - // `IN (...)` on a busy calendar would grow the SQL past what SQLite takes. + // Batched: the ids go into the selection literally, and an unbounded + // `IN (...)` would grow the SQL past what SQLite takes. eventIds.distinct().chunked(EVENT_ID_BATCH).forEach { batch -> context.contentResolver.query( CalendarContract.Reminders.CONTENT_URI, diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderMaintenanceWorker.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderMaintenanceWorker.kt index 7f04042..7ef7350 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderMaintenanceWorker.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderMaintenanceWorker.kt @@ -15,13 +15,8 @@ import java.util.concurrent.TimeUnit /** * The backstop under the alarm: a daily scan that runs whether or not the alarm - * survived. - * - * The scan alarm re-arms itself at most a day out, so in the steady state this - * finds nothing to do. It exists for the case the whole feature is about — a - * device that quietly drops the alarm without a reboot to announce it. The - * scheduling half of reminder delivery must not have a single point of failure, - * which is exactly what the provider's broadcast turned out to be. + * survived. Finds nothing to do in the steady state; it exists for the device + * that quietly drops the alarm without a reboot to announce it (#75). */ object ReminderMaintenanceScheduler { @@ -55,8 +50,8 @@ class ReminderMaintenanceWorker( .scan() Result.success() } catch (e: Exception) { - // The scan swallows its own failures; anything reaching here is the - // entry point itself, which a retry will not mend. Never fail the chain. + // The scan swallows its own failures, so anything reaching here is the + // entry point itself — a retry will not mend it. Log.w(TAG, "Reminder maintenance scan failed", e) Result.success() } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt index 7e512db..17d7213 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt @@ -54,12 +54,10 @@ class ReminderNotifier @Inject constructor( } /** - * The single choke point for "this calendar is switched off". The scan - * already filters on `Calendars.VISIBLE`, but two paths reach [post] around - * it: a snooze re-shown from its own alarm, armed before the calendar was - * switched off, and a read-only install whose switch lives app-side - * ([CalendarPrefs]) because it may not write the flag. Both are covered here - * rather than in either receiver. + * The single choke point for "this calendar is switched off", covering the + * two paths that reach [post] around the scan's own filter: a snooze armed + * before the switch-off, and a read-only install whose switch lives in + * [CalendarPrefs]. */ private suspend fun isSilenced(calendarId: Long): Boolean = calendarId in calendarPrefs.pendingDisabledCalendarIds.first() || @@ -123,7 +121,7 @@ class ReminderNotifier @Inject constructor( // POST_NOTIFICATIONS was revoked between canPost() and here. Log.w(TAG, "Could not post reminder for event ${alert.eventId}", e) } - // Handled either way: re-running it would hit the same revoked permission. + // Handled either way — a retry hits the same revoked permission. return true } @@ -142,8 +140,7 @@ class ReminderNotifier @Inject constructor( private fun detailIntent(alert: ReminderAlert): PendingIntent = PendingIntent.getActivity( context, - // Shares the per-(alert, action) request-code scheme with the buttons, so - // the key's wider value range can't collide one notification's intents. + // Shares the per-(alert, action) request-code scheme with the buttons. /* requestCode = */ ReminderActionReceiver.requestCode( alert, ReminderActionReceiver.ACTION_OPEN, ), diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScanner.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScanner.kt index 1522850..a4f447f 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScanner.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScanner.kt @@ -33,13 +33,9 @@ import javax.inject.Singleton /** * One pass of in-house reminder delivery: read what is planned, post what has - * come due, and arm the next wake-up. - * - * This is the whole loop. Every trigger — the alarm firing, boot, a clock or - * timezone change, an edit landing in the provider, the app starting, the daily - * safety net — runs the same [scan], so there is a single path to reason about - * and no ordering between triggers to get wrong. Re-running it is always safe: - * the watermark in [ReminderStatePrefs] decides what is owed, not the trigger. + * come due, and arm the next wake-up. Every trigger runs the same [scan], and + * re-running is always safe — the watermark in [ReminderStatePrefs] decides what + * is owed, not the trigger. */ @Singleton class ReminderScanner @Inject constructor( @@ -68,8 +64,7 @@ class ReminderScanner @Inject constructor( try { runScan() } catch (e: SecurityException) { - // The calendar permission was revoked mid-flight. Nothing to - // re-arm and nothing to recover from — the next grant re-scans. + // Permission revoked mid-flight; the next grant re-scans. Log.w(TAG, "Reminder scan lacks the calendar permission", e) } catch (e: Exception) { Log.w(TAG, "Reminder scan failed", e) @@ -82,18 +77,16 @@ class ReminderScanner @Inject constructor( if (!hasReadCalendar()) return if (!settingsPrefs.remindersEnabled.first()) { - // Reminders off: drop the wake-up, but keep the watermark moving so - // switching them back on doesn't replay everything missed meanwhile. + // Reminders off: drop the wake-up, but keep the watermark moving + // so switching them back on doesn't replay the backlog. alarms.cancelScan() state.setLastScanMillis(now) return } val lookahead = reminderQueryHorizon(LOOKAHEAD_MILLIS, source.longestReminderMinutes()) - // Reach a little into the past as well: an event already under way can - // still owe a reminder (an all-day "at time of event" encodes to a - // negative offset, which fires after begin), and a catch-up pass needs - // to see the occurrences whose moment it missed. + // Reach into the past too: an all-day "at time of event" encodes to a + // negative offset, and a catch-up pass needs the occurrences it missed. val occurrences = source.occurrences(now - PAST_WINDOW_MILLIS, now + lookahead) val planned = planReminders( instances = occurrences, @@ -112,8 +105,8 @@ class ReminderScanner @Inject constructor( if (notifier.canPost()) { schedule.due.forEach { notifier.post(it.toAlert()) } } - // Advance regardless of whether anything could be posted: a user who - // muted notifications is not owed a backlog when they unmute. + // Advance even when nothing could be posted, so muting notifications + // doesn't build a backlog. state.setLastScanMillis(now) alarms.scheduleScan(schedule.nextAlarmMillis) } @@ -123,13 +116,9 @@ class ReminderScanner @Inject constructor( ) == PackageManager.PERMISSION_GRANTED /** - * Re-scan when the provider changes, so an event saved or deleted in the app - * re-arms the alarm immediately rather than waiting for the next pass. - * - * Debounced: a single save writes the event, its reminders and its - * attendees, and each lands as its own notification. Only useful while the - * process is alive — every other trigger covers the rest, which is why - * nothing here needs to survive it. + * Re-scan when the provider changes, so a saved or deleted event re-arms the + * alarm at once. Debounced, since a single save lands as several + * notifications. Process-lifetime only; other triggers cover the rest. */ fun startWatchingProvider() { if (watching) return @@ -159,9 +148,8 @@ class ReminderScanner @Inject constructor( const val PAST_WINDOW_MILLIS = 24L * 60 * 60 * 1000 /** - * Never wait longer than a day for the next pass, even with nothing - * pending: it rolls the lookahead window forward and re-arms an alarm - * the system may have dropped. + * Never wait longer than a day for the next pass: it rolls the lookahead + * window forward and re-arms an alarm the system may have dropped. */ const val MAX_ALARM_INTERVAL_MILLIS = 24L * 60 * 60 * 1000 diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScheduleReceiver.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScheduleReceiver.kt index 929b0d7..0df3eab 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScheduleReceiver.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderScheduleReceiver.kt @@ -11,22 +11,13 @@ import kotlinx.coroutines.launch import javax.inject.Inject /** - * Every reason to re-run a reminder scan that arrives from outside the process. + * Every out-of-process reason to re-run a reminder scan: our own [ACTION_SCAN] + * alarm, boot and package-replaced (both wipe pending alarms), and time or + * timezone changes (both move reminders relative to the armed alarm). All do the + * same thing, since [ReminderScanner.scan] is idempotent. * - * All of them do the same thing, because [ReminderScanner.scan] is idempotent - * and works out what is owed from its watermark rather than from why it was - * called: - * - * - **our own alarm** ([ACTION_SCAN]) — the ordinary case, a reminder is due; - * - **boot** and **package replaced** — both wipe pending alarms, so the app has - * to re-arm or reminders stop silently, which is the failure #75 is about; - * - **time and timezone changes** — they move every reminder relative to the - * armed alarm, and an all-day reminder's fire hour is recomposed in the - * current zone, so both need a fresh plan. - * - * Exported because the system broadcasts arrive from outside. [ACTION_SCAN] is - * ours and always sent as an explicit intent; another app triggering a scan - * early would only make it re-read the provider and re-arm, which is harmless. + * Exported for the system broadcasts; an early scan triggered by another app is + * harmless. */ @AndroidEntryPoint class ReminderScheduleReceiver : BroadcastReceiver() { @@ -34,10 +25,9 @@ class ReminderScheduleReceiver : BroadcastReceiver() { @Inject lateinit var scanner: ReminderScanner override fun onReceive(context: Context, intent: Intent) { - // Every action here does the same thing, but the filter still has to be - // checked: the receiver is exported, and the system broadcasts it takes - // are protected, so an intent arriving with any other action did not - // come from where it claims to. + // Checked despite every action doing the same thing: the receiver is + // exported and the broadcasts it takes are protected, so any other + // action did not come from where it claims to. if (intent.action !in HANDLED_ACTIONS) return val pendingResult = goAsync() CoroutineScope(SupervisorJob() + Dispatchers.IO).launch { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderSnoozeScheduler.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderSnoozeScheduler.kt index ea1ad44..694f052 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderSnoozeScheduler.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderSnoozeScheduler.kt @@ -10,17 +10,13 @@ import javax.inject.Inject import javax.inject.Singleton /** - * Schedules a one-off exact alarm that re-shows a snoozed reminder. + * Schedules a one-off exact alarm that re-shows a snoozed reminder. Separate + * from [ReminderAlarmScheduler]'s single moving scan alarm: a snooze is pinned + * to one reminder and has to outlive the watermark moving past it, so it carries + * the reminder in its own intent. * - * Separate from [ReminderAlarmScheduler]'s scan alarm, and deliberately so: a - * snooze is pinned to one reminder at a time the user chose, while the scan - * alarm is a single moving wake-up for whatever comes next. A re-show also has - * to outlive the scan's watermark moving past that reminder, so it carries the - * reminder in its own intent rather than re-deriving it. - * - * A snooze that lands late is a broken snooze, hence an *exact* alarm; we fall - * back to an inexact allow-while-idle alarm only if the OS withholds the - * exact-alarm capability (API 31–32 where the user revoked it). + * Falls back to an inexact allow-while-idle alarm where the OS withholds the + * exact-alarm capability (API 31–32 with the permission revoked). */ @Singleton class ReminderSnoozeScheduler @Inject constructor( @@ -40,8 +36,8 @@ class ReminderSnoozeScheduler @Inject constructor( AlarmManager.RTC_WAKEUP, triggerAtMillis, pendingIntent, ) } else { - // Exact alarms revoked (API 31–32): an inexact wake is the honest - // best we can do without nagging for SCHEDULE_EXACT_ALARM. + // Exact alarms revoked (API 31–32); an inexact wake is the best + // available without nagging for SCHEDULE_EXACT_ALARM. alarmManager.setAndAllowWhileIdle( AlarmManager.RTC_WAKEUP, triggerAtMillis, pendingIntent, ) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarRowState.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarRowState.kt index 0272370..30a80a7 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarRowState.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarRowState.kt @@ -1,15 +1,13 @@ package de.jeanlucmakiola.calendula.domain /** - * The ways a calendar can behave unlike a plain, writable one — each of them a - * reason it is missing from the event and import pickers, and each of them - * something the app knows and used to keep to itself (#76). + * The ways a calendar can behave unlike a plain, writable one — each a reason it + * is missing from the event and import pickers (#76). */ enum class CalendarStateLabel { /** * A special-dates mirror the app fills from contacts. Writable and visible, - * yet no event target: anything authored here is deleted by the next sync, - * which is why it is the one exclusion with nothing else to give it away. + * yet no event target: anything authored here is deleted by the next sync. */ MANAGED, @@ -21,22 +19,17 @@ enum class CalendarStateLabel { } /** - * Whether the account this calendar belongs to keeps its events off the device - * (`Calendars.SYNC_EVENTS = 0`) — an "empty by construction" calendar: the rows - * simply aren't here, so nothing can display them and no reminder can fire. - * - * Device-local calendars are excluded deliberately. Nothing syncs them by - * definition, so the flag says nothing about them, and a local calendar from - * another app can hold real events at `sync_events = 0` — the same unsoundness - * that made the #75 migration guard wrong. + * Whether the account keeps this calendar's events off the device + * (`Calendars.SYNC_EVENTS = 0`) — empty by construction. Device-local calendars + * are excluded: nothing syncs them by definition, and one from another app can + * hold real events at `sync_events = 0`. */ val CalendarSource.isNotSynced: Boolean get() = !syncsEvents && !isLocal /** * Whether a visibility switch on this calendar can change anything the user - * would see. It can't for a non-syncing one: there are no events on the device - * to reveal, so the switch would be a control that does nothing. + * would see — it can't for a non-syncing one, with no events on the device. */ val CalendarSource.hasVisibilitySwitch: Boolean get() = !isNotSynced @@ -44,18 +37,9 @@ val CalendarSource.hasVisibilitySwitch: Boolean /** * Whether this calendar can be offered as a target for a new or imported event. * The one predicate behind both pickers, so the states [CalendarStateLabel] - * names on a manager row are exactly the states that keep a calendar out of - * them (#76): - * - * - read-only has nowhere to write; - * - switched off would hide the event the moment it was saved; - * - a managed mirror has the next contact sync delete it; - * - a non-syncing one never carries the event up to the account, and - * `CalendarProvider2` wipes the calendar's rows outright when the - * subscription is switched back on — a saved event is a dead end either way. - * - * This is the test for *targets*. An event already living in an excluded - * calendar keeps it; the editor adds that calendar back to its picker. + * names on a manager row are exactly the states that keep a calendar out of them + * (#76). An event already living in an excluded calendar keeps it; the editor + * adds that calendar back to its picker. */ val CalendarSource.isEventTarget: Boolean get() = canModifyContents && isVisibleInSystem && !isManaged && !isNotSynced diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarVisibilityPlan.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarVisibilityPlan.kt index 4f57088..f9b84dc 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarVisibilityPlan.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarVisibilityPlan.kt @@ -12,22 +12,16 @@ data class CalendarVisibilityPlan( } /** - * Reconcile [pendingDisabledIds] — calendars switched off in Settings → - * Calendars while the app could not write `Calendars.VISIBLE`, plus whatever - * the retired app-local visibility model left behind (#75) — against the + * Reconcile [pendingDisabledIds] — switch-offs the app could not write, plus + * what the retired app-local visibility model left behind (#75) — against the * calendars actually on the device. * - * The plan only ever *hides*. Switching a calendar off is intent the user - * expressed in Calendula, so carrying it into the provider is fair. The other - * direction is deliberately absent: a calendar hidden at system level was hidden - * somewhere else (another calendar app, the account's own settings), and - * switching it back on would un-hide it there too *and* start firing reminders - * nobody asked for. Calendula follows that flag instead and explains itself once - * (see [hasSystemHiddenCalendars]). + * Only ever *hides*: switching a system-hidden calendar back on would un-hide it + * in every other calendar app too. Calendula follows the flag and explains + * itself once (see [hasSystemHiddenCalendars]). * - * [CalendarVisibilityPlan.settled] carries the ids that need no write — already - * hidden, or gone from the device. They leave the pending set exactly as a - * successful write would. + * [CalendarVisibilityPlan.settled] carries the ids needing no write — already + * hidden, or gone from the device. */ fun calendarVisibilityPlan( calendars: List, @@ -38,8 +32,7 @@ fun calendarVisibilityPlan( val settled = mutableSetOf() for (id in pendingDisabledIds) { val calendar = byId[id] - // No row means the calendar is gone; already invisible means someone - // (us, on an earlier run) got there first. Either way: nothing to write. + // Gone from the device, or already invisible — nothing to write. if (calendar != null && calendar.isVisibleInSystem) hide += id else settled += id } return CalendarVisibilityPlan(hide = hide, settled = settled) @@ -47,10 +40,7 @@ fun calendarVisibilityPlan( /** * Whether any calendar is switched off at system level without Calendula having - * asked for it. Those calendars showed their events before the app adopted - * `Calendars.VISIBLE` as its one visibility model and no longer do, which is - * what the one-time notice explains — the alternative, switching them on, would - * reach into every other calendar app on the device. + * asked for it — the condition the one-time notice explains. */ fun hasSystemHiddenCalendars( calendars: List, diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt index a01bf7c..76fb814 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt @@ -13,11 +13,9 @@ data class CalendarSource( val accountType: String, val color: Int, /** - * The system's per-calendar `Calendars.VISIBLE` flag — the single visibility - * model: it decides both what Calendula shows and whether the provider - * schedules this calendar's reminder alarms at all (#75). Settings → - * Calendars writes it; the drawer's filter sheet is a separate, purely - * in-app declutter that leaves reminders alone. + * The system's `Calendars.VISIBLE` flag — the single visibility model, + * deciding both what Calendula shows and whether this calendar plans + * reminders (#75). The drawer's filter sheet is a separate in-app declutter. */ val isVisibleInSystem: Boolean, /** @@ -47,10 +45,8 @@ data class CalendarSource( val isManaged: Boolean = false, /** * Whether the provider keeps this calendar's events on the device - * (`Calendars.SYNC_EVENTS`). Independent of [isVisibleInSystem]. For a - * synced account it means the events aren't stored locally at all, so the - * calendar reads as permanently empty; a device-local calendar another app - * created can hold events with the flag off, so it says nothing there. + * (`Calendars.SYNC_EVENTS`), independent of [isVisibleInSystem]. Says + * nothing about device-local calendars, which can hold events with it off. * Read for the "not synced" row label (#76). */ val syncsEvents: Boolean = true, @@ -77,12 +73,9 @@ data class EventInstance( fun EventInstance.hasEnded(now: Instant): Boolean = end <= now /** - * The zone this event's calendar dates live in. Timed events are resolved in the - * device [zone]; all-day events live at UTC midnights with an exclusive end, so - * resolving them anywhere else shifts the day boundaries — east of UTC the end - * leaks onto the following day (#65), west of UTC the start pulls back onto the - * previous one (#82). Every surface that has to name an all-day event's date - * goes through here, so grid, agenda, detail and search cannot disagree. + * The zone this event's calendar dates live in: the device [zone] for timed + * events, UTC for all-day ones, whose midnights would otherwise shift day + * boundaries (#65, #82). Every surface naming an all-day date goes through here. */ fun EventInstance.dateZone(zone: TimeZone): TimeZone = if (isAllDay) TimeZone.UTC else zone @@ -92,9 +85,8 @@ fun EventInstance.spanFirstDay(zone: TimeZone): LocalDate = start.toLocalDateTime(dateZone(zone)).date /** - * The last calendar day this event actually occupies. An event ending exactly at - * midnight (all-day events end at the exclusive next-midnight) does not reach - * into that boundary day, so resolve the instant just before [EventInstance.end]. + * The last calendar day this event occupies. An event ending exactly at midnight + * does not reach into that day, so resolve just before [EventInstance.end]. */ fun EventInstance.spanLastDay(zone: TimeZone): LocalDate { val lastInstant = if (end > start) end - 1.milliseconds else start diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlan.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlan.kt index 134c8fb..ef17d08 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlan.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlan.kt @@ -8,21 +8,8 @@ import java.time.ZoneOffset import java.time.temporal.ChronoUnit /** - * Works out *when* each reminder has to fire and *which* ones are due, with no - * provider and no clock of its own — the whole decision layer of in-house - * reminder delivery (#75). - * - * Calendula used to leave both halves to the calendar provider: it scheduled the - * alarms, wrote the `CalendarAlerts` rows, and broadcast `EVENT_REMINDER` at the - * right moment. That chain is intact on stock Android but demonstrably not on - * every device — AOSP's own unbundled calendar carries three separate - * workarounds for OEMs that retarget the broadcast, or that only write the alert - * row at alert time. An app that can only *react* to that broadcast has no way - * to notice it never came. - * - * So the offsets in `CalendarContract.Reminders` are now read as data and turned - * into alarms we own. Everything here is pure: instances and reminder offsets in, - * fire instants out. + * Pure decision layer of in-house reminder delivery (#75): instances and reminder + * offsets in, fire instants out. No provider, no clock. See docs/ARCHITECTURE.md. */ /** An occurrence that reminders can hang off, flattened out of `Instances`. */ @@ -46,11 +33,8 @@ data class PlannedReminder( val alarmMillis: Long, ) { /** - * Stable identity of this reminder, derived from what defines it rather - * than from a provider row id (there is none any more). It keys the - * notification tag and the snooze/dismiss `PendingIntent`s, so it has to - * survive a reboot, a re-scan and a reinstall — the same reminder must land - * on the same notification instead of stacking a second one. + * Stable identity, keying the notification tag and the snooze/dismiss + * `PendingIntent`s. Survives reboot, re-scan and reinstall. */ val key: Long = key(instance.eventId, instance.beginMillis, minutes) @@ -75,28 +59,10 @@ private const val MINUTES_PER_DAY = 1_440 /** * Pair every instance with each of its event's reminder offsets. * - * A **timed** occurrence is trivial: `begin` is an absolute instant, so - * `begin − minutes` is exact by construction, in any timezone, across any DST - * boundary. - * - * An **all-day** occurrence is not, and taking the offset at face value is what - * makes reminders land at the wrong hour. Its `begin` is UTC midnight, and the - * stored offset is not a plain lead time — `AllDayReminderEncoding` folds the - * wanted wall-clock hour into it, sampled against *one* date's UTC offset. Fire - * at `begin − minutes` and every occurrence in a different DST phase than the one - * that was sampled drifts by the offset delta, an hour early in one direction and - * an hour late in the other. Rows written by other apps carry no wall-clock at - * all — a conventional `1440` fires at UTC midnight, which is 01:00 or 02:00 - * local in Berlin and the wrong day west of UTC. - * - * So the offset is only read for *which day* it means, via [allDayLeadDays], and - * the hour comes from [allDayTimeMinutes] — the one global "show all-day - * reminders at" setting — recomposed against each occurrence's own date in - * [zone]. 09:00 Berlin is then 09:00 Berlin on every occurrence, whatever the - * offset was when the row was written. - * - * [minutesByEvent] may hold duplicate offsets (two identical reminder rows on one - * event); they collapse, because they would otherwise fight over one notification. + * Timed occurrences fire at `begin − minutes`. All-day ones read the offset only + * for *which day* it means ([allDayLeadDays]) and take the hour from + * [allDayTimeMinutes], recomposed against each occurrence's own date in [zone]. + * Duplicate offsets in [minutesByEvent] collapse. */ fun planReminders( instances: List, @@ -124,30 +90,15 @@ private fun allDayDate(beginMillis: Long): LocalDate = /** * How many whole days before its occurrence a raw all-day offset means. * - * Normally the local date of the encoded fire instant answers it: our own rows - * fold the wanted hour into the offset, so that date *is* the day the reminder - * belongs to, whatever the offset was when it was written. + * Our own rows fold the wanted hour into the offset, so the local date of the + * encoded instant is the answer. A plain multiple of 1440 is a foreign bare lead + * time and taken at face value instead — unless the instant lands on the hour the + * setting names (within [NAMED_HOUR_TOLERANCE_MINUTES], for DST drift), where the + * encodings collide and the tie goes to our own reading. * - * A plain multiple of 1440 is the exception. Those come from calendar apps that - * store a bare lead time against UTC midnight, and east of UTC both readings - * agree anyway — but west of UTC the instant falls on the previous local date, - * so the day count has to be taken at face value or it comes out one too many. - * - * Except when the row is one of ours after all: our offset lands on a multiple - * whenever the all-day hour equals the zone's UTC offset (20:00 in New York, - * 19:00 an hour further west, and so on), and reading those at face value fired - * them a day late — a "1 day before" arriving on the morning of the event. So - * the hour decides, not the shape of the number: an instant landing on the hour - * the setting names is ours. Within an hour or so of it counts, because a fixed - * offset written in one DST phase drifts by the offset delta in the other. - * - * The two are genuinely indistinguishable in that band — the encodings collide - * exactly there — so the tie goes to the reading that honours the lead time the - * user chose in this app. - * - * [de.jeanlucmakiola.calendula.data.calendar.fromProviderAllDayMinutes] calls - * this for display, so the notification arrives on the day the event screen says - * it will. + * Also used by + * [de.jeanlucmakiola.calendula.data.calendar.fromProviderAllDayMinutes] for + * display, so screen and notification agree. */ internal fun allDayLeadDays( rawMinutes: Int, @@ -185,17 +136,10 @@ private fun allDayAlarmMillis( /** * Split [planned] into what is due now and when to wake up next. * - * Due means the fire instant falls in `(lastFiredMillis, nowMillis]` — a - * half-open watermark, so a scan triggered twice cannot post the same reminder - * twice, while a scan that runs late still catches everything the missed alarm - * would have posted. That catch-up is the point: an alarm dropped by a reboot, - * an app update or a doze window is recovered by the next scan rather than lost. - * - * A reminder whose event has already ended is dropped rather than posted late — - * see [isStillRelevant]. - * - * [nextAlarmMillis] is capped at [horizonMillis] even when nothing is pending, so - * the scan re-runs at least that often and the lookahead window rolls forward. + * Due means the fire instant falls in `(lastFiredMillis, nowMillis]`, so a scan + * running twice cannot post twice while a late one still catches up. Reminders + * whose event has ended are dropped ([isStillRelevant]). [nextAlarmMillis] is + * capped at [horizonMillis] so the lookahead window keeps rolling forward. */ fun scheduleReminders( planned: List, @@ -225,31 +169,20 @@ fun ReminderEventInstance.isStillRelevant(nowMillis: Long): Boolean = (endMillis.takeIf { it > 0L } ?: beginMillis) >= nowMillis /** - * The watermark a scan at [nowMillis] should measure against, given what the - * last one recorded. - * - * A first-ever scan ([lastScanMillis] `null`) claims the present, so an install - * or an upgrade onto in-house delivery does not treat every reminder since the - * epoch as overdue and bury the user in notifications. A watermark in the - * *future* — the clock was moved back, or the user travelled across the date - * line — is clamped for the mirror-image reason: left alone it would silence - * every reminder until real time caught up with it. + * The watermark a scan at [nowMillis] should measure against. A first-ever scan + * claims the present rather than replaying everything since the epoch; a + * watermark in the future (clock moved back) is clamped so it can't silence + * every reminder until real time catches up. */ fun reminderWatermark(lastScanMillis: Long?, nowMillis: Long): Long = lastScanMillis?.coerceAtMost(nowMillis) ?: nowMillis /** - * How far ahead instances must be queried for [scheduleReminders] to see every - * reminder in time: the plain lookahead plus the longest offset any reminder row - * carries, so a "two weeks before" reminder is planned before it comes due - * instead of firing late (the limitation Etar's equivalent documents). - * - * The stretch is capped at [MAX_REMINDER_LEAD_MILLIS]. `maxReminderMinutes` is - * whatever the largest row in the whole provider says, across every calendar and - * whoever wrote it — an imported `TRIGGER:-P100W`, or a sync adapter writing - * nonsense, would otherwise make every scan (launch, every provider change, every - * alarm) expand every recurring series over years. A lead beyond the cap is not - * delivered; a year is far past anything the picker composes. + * How far ahead instances must be queried: the plain lookahead plus the longest + * reminder offset, so a "two weeks before" is planned before it comes due. The + * stretch is capped at [MAX_REMINDER_LEAD_MILLIS] — `maxReminderMinutes` is the + * largest row in the whole provider, and a nonsense one would otherwise make + * every scan expand every series over years. */ fun reminderQueryHorizon(lookaheadMillis: Long, maxReminderMinutes: Int): Long = lookaheadMillis + (maxReminderMinutes * MILLIS_PER_MINUTE).coerceIn(0L, MAX_REMINDER_LEAD_MILLIS) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/RootScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/RootScreen.kt index 83d432e..49e90d0 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/RootScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/RootScreen.kt @@ -51,8 +51,8 @@ fun RootScreen( ) } - // Whether the app came up already holding it — a launch scan has covered - // that case, so only a grant made during this session owes a re-scan. + // A launch scan already covers the app coming up with the permission, so + // only a grant made during this session owes a re-scan. val grantedAtLaunch = remember { hasPermission } val lifecycle = LocalLifecycleOwner.current.lifecycle @@ -86,12 +86,11 @@ fun RootScreen( val reminderOnboarding: ReminderOnboardingViewModel = hiltViewModel() val onboardingDone by reminderOnboarding.onboardingDone.collectAsStateWithLifecycle() // One-time explainer for the switch to the device's own calendar - // visibility (#75); armed by the reconciler, shown over the app. + // visibility (#75), armed by the reconciler. val visibilityNotice: CalendarVisibilityNoticeViewModel = hiltViewModel() val noticePending by visibilityNotice.pending.collectAsStateWithLifecycle() - // Runs on entry however the permission was granted — including from - // Android's app-settings screen, which only comes back through the - // ON_RESUME check above. Cheap once there is nothing left to do. + // Runs on entry however the permission was granted, including via + // Android's app-settings screen (caught by the ON_RESUME above). LaunchedEffect(Unit) { visibilityNotice.reconcile() if (!grantedAtLaunch) reminderOnboarding.rearmAfterGrant() diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/BackupScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/BackupScreen.kt index 68eb813..f32663f 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/BackupScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/BackupScreen.kt @@ -59,10 +59,8 @@ import de.jeanlucmakiola.floret.components.Position import de.jeanlucmakiola.floret.components.positionOf import java.time.LocalDate -// SAF mime filter for the restore picker. `.ics` files reach us under several -// mimes depending on the source app (our own export uses text/calendar; others -// hand them out as octet-stream or text/plain), so accept the common set rather -// than hide valid backups behind an over-tight filter. +// SAF mime filter for the restore picker. Source apps hand `.ics` files out +// under several mimes, so accept the common set. private val RESTORE_MIME_TYPES = arrayOf( "text/calendar", "application/octet-stream", @@ -70,11 +68,8 @@ private val RESTORE_MIME_TYPES = arrayOf( ) /** - * Backup & restore (#69). Local calendars aren't synced anywhere, so a `.ics` - * export is their only safety net — that made it a data-safety feature hiding in - * the calendar manager, where nobody went looking. It now has its own Settings - * entry, sharing [CalendarsViewModel] with the manager (both are driven by the - * same calendar list). + * Backup & restore (#69): `.ics` export and restore for local calendars, with + * optional automatic backup. Shares [CalendarsViewModel] with the manager. * * A full-screen destination hoisted in `CalendarHost`; [onBack] pops it, * [onImport] hands a picked file to the app's normal .ics import flow. @@ -92,29 +87,24 @@ fun BackupScreen( val context = LocalContext.current val snackbarHostState = remember { SnackbarHostState() } - // Export covers the user's own local calendars (managed special-dates - // mirrors don't count — they are rebuilt from contacts). Restore can target - // any calendar the import picker would offer, so its availability is broader. + // Export covers local calendars only; managed special-dates mirrors are + // rebuilt from contacts. Restore can target anything the import picker offers. val exportable = calendars.filter { it.isLocal && it.canModifyContents && !it.isManaged } val canImport = calendars.any { it.isEventTarget } - // SAF "create document" target for the backup file. The picked Uri is handed - // to the VM to stream the .ics into. This launcher exports everything - // eligible (null); the per-calendar selector owns its own launcher. + // Exports everything eligible (null); the per-calendar selector owns its + // own launcher. val createBackup = rememberLauncherForActivityResult( contract = ActivityResultContracts.CreateDocument("text/calendar"), ) { uri -> uri?.let { viewModel.exportBackup(it, null) } } var showExportPicker by rememberSaveable { mutableStateOf(false) } - // SAF "open document" picker for restoring events from a .ics file. The - // picked Uri is handed up to the host, which runs it through the same import - // flow as an externally opened .ics (parse, dedup by UID, target picker). + // Restore runs the picked file through the normal .ics import flow. val openBackup = rememberLauncherForActivityResult( contract = ActivityResultContracts.OpenDocument(), ) { uri -> uri?.let(onImport) } - // SAF folder picker for the automatic-backup destination; the VM persists the - // write grant so background runs can keep writing to it. + // The VM persists the write grant so background runs can keep writing. val pickFolder = rememberLauncherForActivityResult( contract = ActivityResultContracts.OpenDocumentTree(), ) { uri -> uri?.let(viewModel::setAutoBackupFolder) } @@ -148,15 +138,12 @@ fun BackupScreen( HintText(stringResource(R.string.calendars_backup_hint)) if (exportable.isNotEmpty()) { - // One connected card: the one-time export on top, restore under it, - // then automatic backup (and its folder/interval rows when on). GroupedRow( title = stringResource(R.string.calendars_backup_action), position = Position.Top, leading = { LeadingAvatar(Icons.Default.FileDownload) }, onClick = { - // With more than one exportable calendar, let the user choose - // which to include; a single one exports straight away. + // A single exportable calendar skips the selector. if (exportable.size == 1) { runCatching { createBackup.launch("calendula-backup-${LocalDate.now()}.ics") } } else { @@ -198,9 +185,8 @@ fun BackupScreen( HintText(backupStatusText(autoBackup.status)) } } else if (canImport) { - // Nothing to back up (no writable local calendar), but events can - // still be restored into a writable calendar — offer restore on its - // own so it isn't hidden behind export eligibility. + // Nothing to back up, but restore is still possible — don't hide + // it behind export eligibility. SectionHeader(stringResource(R.string.calendars_restore_header)) HintText(stringResource(R.string.calendars_restore_hint)) GroupedRow( @@ -239,11 +225,8 @@ private fun ExportCalendarPicker( onExport: (Uri, Set?) -> Unit, onDismiss: () -> Unit, ) { - // Seed once with everything selected and hold it across recomposition and - // rotation. NOT keyed on [calendars]: the list is observer-driven, so keying - // it would silently reset the user's de-selections whenever the provider - // re-emits (a background sync, a recolor). Ids that later vanish are harmless - // — the data layer intersects the chosen set with the eligible calendars. + // Deliberately not keyed on [calendars]: that list is observer-driven, so + // keying it would reset the user's de-selections on every provider re-emit. var selected by rememberSaveable( stateSaver = listSaver( save = { it.toList() }, @@ -348,7 +331,7 @@ private fun BackupIntervalDialog( onConfirm: (Long) -> Unit, onDismiss: () -> Unit, ) { - // minutes-per-unit for each entry; pick the largest unit the current value divides into. + // Pick the largest unit the current value divides into. val unitMinutes = remember { listOf(1L, 60L, MINUTES_PER_DAY, MINUTES_PER_WEEK) } val units = stringArrayResource(R.array.backup_interval_units).toList() val initialUnit = unitMinutes.indexOfLast { currentMinutes % it == 0L }.coerceAtLeast(0) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarVisibilityNotice.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarVisibilityNotice.kt index 98c3356..107005c 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarVisibilityNotice.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarVisibilityNotice.kt @@ -23,10 +23,8 @@ import javax.inject.Inject /** * The one-time notice that Calendula now follows the device's per-calendar - * visibility (#75). Armed by `CalendarVisibilityReconciler` on the first launch - * that finds a calendar switched off outside the app — those used to show their - * events here and no longer do, and the app deliberately does not switch them - * back on, because that would un-hide them in every other calendar app too. + * visibility (#75), armed by `CalendarVisibilityReconciler`. The app does not + * switch those calendars back on — that would un-hide them everywhere else too. */ @HiltViewModel class CalendarVisibilityNoticeViewModel @Inject constructor( @@ -35,12 +33,9 @@ class CalendarVisibilityNoticeViewModel @Inject constructor( ) : ViewModel() { /** - * Reconcile whenever the app comes up with the calendar permission held. - * The launch itself is covered by `CalendulaApp`, but a permission granted - * on Android's app-settings screen comes back through `RootScreen`'s - * ON_RESUME and never touches the permission screen's callback — so the - * trigger hangs off "we are showing the app", not off one grant route. - * Settled runs cost two DataStore reads and stop there. + * Reconcile whenever the app comes up with the calendar permission held, + * rather than off one grant route: a permission granted on Android's + * app-settings screen never reaches the permission screen's callback. */ fun reconcile() { viewModelScope.launch { reconciler.run() } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsScreen.kt index c9f3508..c9190d0 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsScreen.kt @@ -107,14 +107,11 @@ private const val NEW_CALENDAR_ID = Long.MIN_VALUE /** * Calendar manager (reached from Settings). Lists the app's own device-only - * calendars with create / rename / recolor / delete (via a full-screen editor), - * and lists synced calendars read-only with a per-account "manage in the source - * app" deep-link — the app never touches a synced calendar's server. + * calendars with create / rename / recolor / delete, and synced calendars + * read-only with a per-account "manage in the source app" deep-link. * * Export/import lives in its own Settings entry ([BackupScreen], #69); this - * screen only points at it, because the two are looked for separately: "which - * calendars do I have" versus "keep a copy of them". A full-screen destination; - * [onBack] pops it. + * screen only points at it. [onBack] pops the destination. */ @Composable fun CalendarsScreen( @@ -252,9 +249,7 @@ private fun CalendarsList( } } - // Backup lives in its own Settings entry now (#69) — this row is the - // pointer, so someone looking at their local calendars still finds the - // way to keep a copy of them. + // Pointer to the Backup entry (#69), so it stays findable from here. Spacer(Modifier.height(16.dp)) GroupedRow( title = stringResource(R.string.settings_section_backup), @@ -314,8 +309,8 @@ private fun CalendarsList( onSetAccountVisible(switchable.map { it.id }, enabled) }, ) { - // Calendars you can act on first; the ones this device isn't - // syncing sit at the bottom, dimmed and switchless. + // Actionable calendars first; non-syncing ones at the + // bottom, dimmed and switchless. val ordered = cals.orderedForManager() ordered.forEachIndexed { index, calendar -> val disabled = !calendar.isVisibleInSystem || calendar.isNotSynced @@ -392,11 +387,8 @@ private fun CalendarEditor( }, actions = { if (!isNew) { - // Kept in place while the special-dates sync owns this - // calendar, rather than hidden: the button is where you - // expect it, disabled, with the card below saying why — - // and it comes back to life the moment the feature is - // off, when the delete would actually stick. + // Disabled rather than hidden while the special-dates + // sync owns this calendar; the card below says why. IconButton( onClick = { confirmDelete = true }, enabled = !deleteLocked, @@ -523,9 +515,7 @@ private fun CalendarEditor( /** * The row's supporting line: the states that make this calendar behave unlike a - * plain writable one (#76), then its own description. Text rather than badges — - * a row can carry several of these at once next to a switch, which is exactly - * what M3 supporting text composes and a row of static chips doesn't. + * plain writable one (#76), then its own description. */ @Composable private fun calendarRowSummary(calendar: CalendarSource): String? { @@ -543,12 +533,9 @@ private fun calendarRowSummary(calendar: CalendarSource): String? { } /** - * The per-row on/off control, writing the system's `Calendars.VISIBLE`: checked - * = the calendar is shown, unchecked = it drops out of every surface (events, - * filters, pickers) and the provider stops scheduling its reminders. The flag is - * device-local — nothing is deleted and nothing is synced anywhere. Carries its - * own content description so the toggle is self-describing to screen readers - * even on a dimmed row. + * The per-row on/off control, writing the system's device-local + * `Calendars.VISIBLE`. Unchecked drops the calendar out of every surface and + * stops its reminders. Carries its own content description. */ @Composable private fun EnableSwitch( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt index 3f2fc3f..49576df 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt @@ -73,18 +73,10 @@ class CalendarsViewModel @Inject constructor( ) /** - * Managed special-dates calendars whose deletion would not stick. While the - * feature is on, the sync owns every mirror: it recreates a missing one for - * an enabled type on the next pass and deletes the leftover of a disabled - * one (`SpecialDatesSyncEngine.reconcileCalendars`), so either way the - * delete would appear to work and then undo itself. Turning special dates - * off empties this set, and deleting a leftover mirror is a real delete from - * then on. - * - * Read off each calendar's own durable marker ([CalendarSource.isManaged], - * the `CAL_SYNC2` one the editor lock already trusts) rather than the stored - * ids, which are only rewritten on the next sync pass — a preferences loss - * would otherwise unlock a live mirror until then. + * Managed special-dates calendars whose deletion would not stick: while the + * feature is on, `SpecialDatesSyncEngine.reconcileCalendars` undoes it on + * the next pass. Read off each calendar's durable [CalendarSource.isManaged] + * marker rather than the stored ids, which lag a sync pass behind. */ val deleteLockedCalendarIds: StateFlow> = combine( calendars, @@ -147,27 +139,18 @@ class CalendarsViewModel @Inject constructor( } /** - * Switch a calendar on or off. This is the app's one visibility model: it - * writes the system's `Calendars.VISIBLE`, so the calendar disappears from - * every surface *and* the provider stops (or resumes) scheduling its - * reminders. Nothing is patched by hand — the provider notifies and the - * observer re-queries. - * - * A reminder that came due while the calendar was off stays gone when it is - * switched back on. Delivery plans from `Instances` and `Reminders` on every - * scan (#75), but the watermark has already moved past that moment, so no - * scan returns it again — and on a read-only install the switch never - * reaches the provider at all, so nothing even triggers one. Deliberate: the - * reminder was silenced on purpose, and the event itself is back in view. + * Switch a calendar on or off — the app's one visibility model, writing the + * system's `Calendars.VISIBLE`. A reminder that came due while the calendar + * was off stays gone when it is switched back on: the watermark has already + * moved past it (#75). */ fun setCalendarVisible(id: Long, visible: Boolean) = write { repository.setCalendarsVisible(listOf(id), visible) } /** - * Switch every calendar of one account on or off — the "toggle all" - * affordance on an account header. Each row is written on its own, in one - * coroutine so the writes can't race each other. + * Switch every calendar of one account on or off. Each row is written on its + * own, in one coroutine so the writes can't race. */ fun setAccountVisible(ids: Collection, visible: Boolean) = write { repository.setCalendarsVisible(ids, visible) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/AccountGroups.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/AccountGroups.kt index 3240a53..5d01a7d 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/AccountGroups.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/AccountGroups.kt @@ -9,13 +9,8 @@ import de.jeanlucmakiola.calendula.domain.CalendarSource /** * One account's calendars, as every surface that lists calendars by account - * shows them. - * - * An account is identified by name **and** type (#77). A Google account and a - * DAVx5 account can carry the same address and still be two separate accounts, - * from separate apps: merging them mixed their calendars into one group whose - * header — source logo, "manage in app", toggle-all, collapsed state — was - * derived from whichever calendar happened to sort first. + * shows them. An account is identified by name **and** type (#77): a Google and + * a DAVx5 account can share an address and still be two separate accounts. */ data class CalendarAccountGroup( /** Stable identity: what makes two calendars belong to the same account. */ @@ -69,9 +64,8 @@ fun accountGroupTitle(group: CalendarAccountGroup): String = } /** - * The human name of the app backing [accountType] — the same app whose icon - * [SourceLogo] draws. Falls back to the raw account type, which is at least - * unique, when no installed app resolves for it. + * The human name of the app backing [accountType], falling back to the raw + * account type when no installed app resolves for it. */ @Composable fun sourceAppName(accountType: String): String { 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 92285c5..61c073e 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 @@ -45,11 +45,9 @@ import de.jeanlucmakiola.floret.components.SelectedCheck * each and a check on the selected one. Emits into the caller's [ColumnScope] * (a scrolling column), so the caller owns the surrounding chrome. * - * The list holds event *targets* only, so a calendar that is switched off, - * read-only or managed is silently absent — which reads as a missing calendar - * rather than an excluded one (#76). [onManageCalendars], when given, adds the - * footer row that names the possible reasons and opens the calendar manager, - * where each row then says which one applies. + * The list holds event *targets* only, so a switched-off, read-only or managed + * calendar is absent (#76). [onManageCalendars], when given, adds the footer row + * naming the possible reasons and opening the calendar manager. */ @Composable fun ColumnScope.CalendarPickerGroups( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RecurrenceText.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RecurrenceText.kt index fb3a582..252bc9e 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RecurrenceText.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/RecurrenceText.kt @@ -105,29 +105,19 @@ fun recurrenceText(rrule: String, locale: Locale): AnnotatedString { /** * The rule's first few dates as one line — "Next: 30 Jul, 6 Aug, 13 Aug" — for - * the recurrence picker. - * - * The phrase [recurrenceText] renders says what the rule *is*; this says what - * it *does*, which is the part a monthly rule on the 31st or an every-other-week - * rule with weekday picks gets wrong in people's heads. It expands the rule - * with [upcomingOccurrences], so it inherits that function's RFC reading — and - * its limits: only the shapes the picker itself can build. - * - * A rule that yields nothing at all (an end date before the start) says so - * rather than showing an empty list, since that is a mistake worth catching - * before saving. + * the recurrence picker, where [recurrenceText]'s phrase says what the rule is + * and this says what it does. Expands via [upcomingOccurrences] and inherits its + * limits. A rule that yields nothing says so rather than showing an empty list. */ @Composable fun nextOccurrencesText( rule: SimpleRecurrence, - // Spelled out: in this file, the bare LocalDate is java.time's. start: kotlinx.datetime.LocalDate, locale: Locale, ): String { val dates = rule.upcomingOccurrences(start, limit = NEXT_OCCURRENCE_COUNT) if (dates.isEmpty()) return stringResource(R.string.event_edit_recurrence_next_none) - // Years only once they carry information — a yearly rule is otherwise the - // same date repeated, and a monthly one crossing New Year hides that it did. + // Years only once they carry information (see [spansMultipleYears]). val pattern = if (occurrencesSpanYears(dates, start)) "dMMMy" else "dMMM" val formatter = localizedDateFormatter(locale, pattern) val formatted = dates.map { formatter.format(it.toJavaLocalDate()) } 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 93105a4..d495474 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 @@ -515,8 +515,8 @@ private fun EventEditContent( // they're locked here; everything else (reminders, location, notes) is the // user's to edit. val locked = state.isManaged - // Read in the form's own window, not the picker's: the field holding focus - // lives here, so this is the controller that can put its keyboard away. + // Read in the form's own window, not the picker's: the focused field lives + // here, so this is the controller that can put its keyboard away. val focusManager = LocalFocusManager.current val keyboardController = LocalSoftwareKeyboardController.current var picker by remember { mutableStateOf(null) } @@ -1124,10 +1124,8 @@ private fun EventEditContent( null -> Unit } - // A full-screen picker over the form is a change of place, so the form's - // keyboard has no business following it there — least of all onto the - // calendar manager, which the picker can hand off to. The form's own field - // keeps its text; only focus and the IME go. + // A full-screen picker is a change of place, so the keyboard shouldn't + // follow it. The field keeps its text; only focus and the IME go. LaunchedEffect(showCalendarPicker) { if (showCalendarPicker) { focusManager.clearFocus(force = true) @@ -1143,11 +1141,9 @@ private fun EventEditContent( viewModel.setCalendar(it) showCalendarPicker = false }, - // Close the picker on the way out. It is a Compose Dialog — its own - // window, always above the activity's content — so the manager would - // otherwise open behind it and the tap would look dead. The form - // stays standing underneath, its calendar row one tap from a picker - // that re-queries on open. + // Close the picker first: it is a Compose Dialog in its own window, + // always above the activity's content, so the manager would + // otherwise open behind it and the tap would look dead. onManageCalendars = onManageCalendars?.let { openManager -> { showCalendarPicker = false @@ -1344,10 +1340,7 @@ private enum class RecurrenceEndMode { Never, Until, Count } * * Both steps show the rule as *dates* as well as words: every preset row * carries the next few occurrences it would produce from [startDate], and the - * custom step repeats that under its live read-out. It is where a rule most - * easily means something other than it sounds like — "monthly" on the 31st - * skips February, an every-other-week rule lands a fortnight from the start's - * own week — and only the dates say so. + * custom step repeats that under its live read-out. */ @Composable private fun RecurrencePickerDialog( @@ -1409,9 +1402,8 @@ private fun RecurrencePickerDialog( RecurrenceEndMode.Until -> untilDate?.let { RecurrenceEnd.Until(it) } RecurrenceEndMode.Count -> count?.let { RecurrenceEnd.Count(it) } } - // Kept as the rule object, not just its RRULE text: the read-out renders the - // string, the date list expands the rule, and both must describe the one - // thing OK would save. + // Kept as the rule object, not just its RRULE text: the read-out and the + // date list must describe the one thing OK would save. val customRule: SimpleRecurrence? = if (interval != null && customEnd != null) { SimpleRecurrence( freq = freq, @@ -1505,9 +1497,8 @@ private fun RecurrencePickerDialog( .padding(horizontal = 16.dp), ) - // The same rule as dates. Rendered even while the form is - // incomplete (as an empty line) so the controls below don't - // shift as it comes and goes. + // Rendered even while incomplete (as an empty line), so the + // controls below don't shift as it comes and goes. Text( text = customRule?.let { nextOccurrencesText(it, startDate, locale) }.orEmpty(), style = MaterialTheme.typography.bodyMedium, @@ -2034,10 +2025,8 @@ private fun readContactAddress(context: Context, uri: Uri): String? = * Visibility selector: one card per level, each with its own icon; the * current level is highlighted. Tap picks and closes. * - * Every level carries a line saying who this affects, because the four words - * alone don't: visibility is about what *other people on a shared calendar* - * see, which is exactly the part the label leaves out — and "Confidential" has - * no meaning at all until you know the server decides what it does with it. + * Every level carries a line saying who this affects — the labels alone don't + * say that visibility is about what others on a shared calendar see. */ @Composable private fun VisibilityPickerDialog( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt index d5284ec..93e3c1d 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt @@ -233,11 +233,9 @@ class EventEditViewModel @Inject constructor( // off the calendar's durable marker, not a stored id, so it holds after a // backup restore too. val isManaged = local.editTarget != null && resolvedCalendar?.isManaged == true - // The picker offers writable calendars only; the event's own calendar is - // added back whenever it isn't among them — a managed special-dates one, - // or one switched off on this device — so the row keeps naming it instead - // of reading as the "no calendar" error, and saving can leave the event - // where it is. A calendar the app may not write to is still no target. + // The event's own calendar is added back whenever it isn't a target + // (managed, or switched off), so the row keeps naming it and saving can + // leave the event where it is. A read-only one is still no target. val ownCalendar = resolvedCalendar?.takeIf { own -> own.canModifyContents && external.writable.none { it.id == own.id } } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterViewModel.kt index bdcf193..86d7924 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterViewModel.kt @@ -32,9 +32,8 @@ class FilterViewModel @Inject constructor( repository.calendars(), prefs.hiddenCalendarIds, ) { calendars, hidden -> - // Calendars switched off in Settings → Calendars are off device-wide - // and don't belong in the drawer's hide/show list (you can't hide - // what is already off). They live only in Settings → Calendars. + // Calendars switched off device-wide don't belong in the drawer's + // hide/show list; they live only in Settings → Calendars. val enabled = calendars.filter { it.isVisibleInSystem } if (enabled.isEmpty()) { FilterUiState.Failure(FailureReason.NoCalendarsConfigured) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportScreen.kt index 253b02e..56e6602 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportScreen.kt @@ -175,10 +175,8 @@ private fun ManyContent( onSelect: (Long) -> Unit, onManageCalendars: (() -> Unit)? = null, ) { - // No calendar to import into — tell the user honestly, and carry the same - // way out the picker's footer offers below. This is the state that footer - // exists for: every writable calendar being switched off, read-only or - // contact-filled is exactly what empties this list (#76). + // No calendar to import into; carries the same way out the picker's footer + // offers below (#76). if (state.calendars.isEmpty()) { CenteredMessage( message = stringResource(R.string.import_no_calendar), diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt index 2e8e066..723132b 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt @@ -86,9 +86,7 @@ class ImportViewModel @Inject constructor( warnings = parsed.warnings, ) else -> { - // The same targets the event form offers ([isEventTarget]): - // an import is a bulk create, so a calendar that can't hold - // one event can't hold thirty. + // The same targets the event form offers ([isEventTarget]). ImportUiState.Many( events = parsed.events, warnings = parsed.warnings, diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt index d997c56..c6ca86c 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthScreen.kt @@ -365,10 +365,8 @@ fun MonthScreen( todayText = stringResource(R.string.month_today_action), onToday = jumpToToday, onCreate = { - // Split has a selected day and lists it below the grid, so - // that is the day the new event belongs to. The other - // styles have no selection: anchor on today when its month - // is shown, else the 1st. + // Split has a selected day; the other styles anchor on + // today when its month is shown, else the 1st. onCreateEvent( when { viewStyle == MonthViewStyle.Split -> selectedDate @@ -2075,10 +2073,9 @@ private fun MonthBar( } /** - * Overflow row: a dot per hidden colour (up to three) plus "+N" for the rest. - * - * A dot stands for every hidden event sharing its colour, so it dims only once - * all of them have ended; the "+N" dims once the whole overflow has (#79). + * Overflow row: a dot per hidden colour (up to three) plus "+N" for the rest. A + * dot dims once every event sharing its colour has ended, the "+N" once the + * whole overflow has (#79). */ @Composable private fun OverflowDots( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthUiState.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthUiState.kt index b7f5f6a..ac3c7e4 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthUiState.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/month/MonthUiState.kt @@ -68,12 +68,9 @@ fun MonthWeek.laneEvents(col: Int, day: LocalDate, laneCap: Int): List { val seatedLanes = spans.count { it.lane < laneCap && col in it.startCol..it.endCol } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/PermissionViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/PermissionViewModel.kt index 6cc03eb..750af2f 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/PermissionViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/PermissionViewModel.kt @@ -13,8 +13,8 @@ class PermissionViewModel @Inject constructor() : ViewModel() { private val _state = MutableStateFlow(PermissionUiState.Rationale) val state: StateFlow = _state.asStateFlow() - // The visibility reconcile a grant owes (#75) hangs off RootScreen showing - // the app instead: it has to cover the grants made outside it too. + // The visibility reconcile a grant owes (#75) hangs off RootScreen, which + // also catches grants made outside the app. fun onGranted() { _state.value = PermissionUiState.Granted } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/ReminderOnboardingViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/ReminderOnboardingViewModel.kt index f2570c0..ee705ac 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/ReminderOnboardingViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/ReminderOnboardingViewModel.kt @@ -37,8 +37,7 @@ class ReminderOnboardingViewModel @Inject constructor( prefs.setRemindersEnabled(remindersEnabled) prefs.setReminderOnboardingDone() // Nothing else re-arms the scan: turning reminders off cancels the - // alarm, so a later "on" would sit without one until a provider - // change or the daily worker happened to run (#75). + // alarm (#75). scanner.scan() } } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/search/SearchScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/search/SearchScreen.kt index b1caa35..420be28 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/search/SearchScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/search/SearchScreen.kt @@ -223,10 +223,9 @@ private fun searchSummary(event: EventInstance): String { val start = remember(event.start, zone) { JavaInstant.ofEpochMilli(event.start.toEpochMilliseconds()).atZone(zone) } - // The date comes from the shared span rule, not from [start]: an all-day - // event sits at UTC midnight, so reading its date in the device zone names - // the day before west of UTC (#82). The clock time below stays in the device - // zone — it is only ever rendered for timed events. + // From the shared span rule, not [start]: an all-day event sits at UTC + // midnight and would name the day before west of UTC (#82). The clock time + // below stays device-zone — it is only rendered for timed events. val dateText = remember(event.start, event.end, event.isAllDay, locale) { DateTimeFormatter.ofLocalizedDate(FormatStyle.MEDIUM).withLocale(locale) .format(event.spanFirstDay(TimeZone.currentSystemDefault()).toJavaLocalDate()) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/AppearanceSettings.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/AppearanceSettings.kt index 37645e6..40f1704 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/AppearanceSettings.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/AppearanceSettings.kt @@ -84,7 +84,6 @@ internal fun AppearanceScreen( val fonts by viewModel.fontState.collectAsStateWithLifecycle() val launcherName by viewModel.launcherName.collectAsStateWithLifecycle() - // A picked file that didn't parse as a font: tell the user and keep the old choice. val context = LocalContext.current val importFailedMessage = stringResource(R.string.settings_font_import_failed) LaunchedEffect(Unit) { @@ -107,8 +106,6 @@ internal fun AppearanceScreen( ) GroupedRow( title = stringResource(R.string.settings_dynamic_color), - // Says what it does when on; below Android 12 that is replaced by - // the reason the switch is dead. summary = if (state.dynamicColorAvailable) { stringResource(R.string.settings_dynamic_color_summary) } else { @@ -143,9 +140,7 @@ internal fun AppearanceScreen( Spacer(Modifier.height(16.dp)) - // Fonts — the two Material typeface roles, each independently choosable - // (issue #19). Headings = brand (display/headline); body = plain - // (title/body/label). Both default to the system typeface. + // Fonts — the two Material typeface roles (#19). GroupedRow( title = stringResource(R.string.settings_font_headings), summary = fontLabel(fonts.brand), @@ -161,10 +156,8 @@ internal fun AppearanceScreen( Spacer(Modifier.height(16.dp)) - // App name — chooses the launcher label between "Calendula" and "Calendar" - // (issue #44). Own group: it's a launcher/system concern, not app styling. - // A sub-page chooser (not a switch), matching the app's other "choose one" - // settings and leaving room for more names later. + // App name — the launcher label (#44); its own group, being a + // launcher concern rather than app styling. GroupedRow( title = stringResource(R.string.settings_app_name), summary = launcherNameLabel(launcherName), @@ -179,10 +172,8 @@ internal fun AppearanceScreen( onDismiss = { showAppName = false }, predictiveBack = true, ) { - // Show both names as launcher-mark previews so the user sees what - // they'd switch to, not just the current state. Tapping applies - // immediately and highlights — the picker stays open so the change is - // visible; back exits. + // Both names shown as launcher-mark previews; tapping applies at + // once and the picker stays open so the change is visible. Text( text = stringResource(R.string.settings_app_name_summary), style = MaterialTheme.typography.bodyMedium, @@ -211,10 +202,8 @@ internal fun AppearanceScreen( } } if (showTheme) { - // No preview here on purpose: picking a theme repaints the app itself, - // which is a better demonstration than any thumbnail. What the list - // *can't* show is which way "Follow the system" currently falls, so that - // one option says so. + // No thumbnails: picking a theme repaints the app itself. Only which + // way "Follow the system" currently falls needs spelling out. val systemDark = isSystemInDarkTheme() OptionPicker( title = stringResource(R.string.settings_theme), @@ -307,19 +296,8 @@ private val FONT_PICKER_MIME_TYPES = arrayOf( /** * Full-screen font chooser for one [FontRole]: the system default, each bundled - * font (previewed in its own face), and "Choose file…" which opens the system - * picker to load a .ttf/.otf. Selecting a system/bundled option applies at once; - * a file is validated and imported by the caller, switching to the custom font - * on success. - * - * Above the rows sits a specimen of the *current* choice, set in the type role - * this picker governs — headline for [FontRole.BRAND], body for - * [FontRole.PLAIN]. The per-row "Ag" says what a face looks like; only a full - * line at the real size says whether it works for the role, and it is the one - * thing that makes the two font settings distinguishable from each other. - * - * Being a preview picker, it stays open on selection (see `FullScreenPicker`): - * the specimen re-renders instead, which is the whole point of the screen. + * font, and "Choose file…" for a .ttf/.otf. A specimen of the current choice + * sits above the rows, so the picker stays open on selection and re-renders it. */ @Composable private fun FontPicker( @@ -335,22 +313,17 @@ private fun FontPicker( val launcher = rememberLauncherForActivityResult( contract = ActivityResultContracts.OpenDocument(), ) { uri -> - // Stay open on a successful import too: the specimen switches to the - // imported face, which is the only look the user gets before committing. if (uri != null) onImport(uri) } // System default + the bundled fonts + the "Choose file…" row. val rowCount = BundledFont.entries.size + 2 val isCustom = selected == FONT_CUSTOM_TOKEN - // Resolving the custom face stats the disk and builds a fresh FontFamily, so - // memoise it; re-keyed on [stamp] (bumped on re-import) so a replaced file - // refreshes the preview while plain recompositions reuse the cached family. + // Resolving the custom face stats the disk, so memoise it; [stamp] is bumped + // on re-import to refresh a replaced file. val customPreview = remember(role, isCustom, stamp) { if (isCustom) resolveFontFamily(FONT_CUSTOM_TOKEN, role, context) else null } - // The face the specimen is set in: the same resolution the theme performs, - // so what is shown here is what the app will use. val selectedFamily = remember(role, selected, stamp) { resolveFontFamily(selected, role, context) } @@ -362,8 +335,6 @@ private fun FontPicker( preview = FontFamily.Default, selected = selected == FONT_SYSTEM_TOKEN, position = positionOf(0, rowCount), - // Applies straight away and stays open — the specimen above is the - // answer to "what does this one look like". onClick = { onSelect(FONT_SYSTEM_TOKEN) }, ) BundledFont.entries.forEachIndexed { index, font -> @@ -381,7 +352,6 @@ private fun FontPicker( } else { stringResource(R.string.settings_font_choose_file) }, - // A loaded font previews in its own face; otherwise show an upload cue. preview = customPreview, leadingIcon = if (isCustom) null else Icons.Default.UploadFile, selected = isCustom, @@ -392,11 +362,8 @@ private fun FontPicker( } /** - * A specimen of [family] set in the type role [role] governs: a headline for - * the brand role, a paragraph for the plain one — the same styles the app draws - * with, only the family swapped, so nothing here can flatter a face the app - * won't reproduce. A null [family] is the system typeface, i.e. the Material - * default the styles already carry. + * A specimen of [family] in the type role [role] governs, using the app's own + * styles with only the family swapped. A null [family] is the system typeface. */ @Composable private fun FontSpecimen(role: FontRole, family: FontFamily?) { @@ -412,8 +379,8 @@ private fun FontSpecimen(role: FontRole, family: FontFamily?) { ), style = style.copy(fontFamily = family ?: style.fontFamily), color = MaterialTheme.colorScheme.onSurface, - // Two lines up front so switching between a wide and a narrow face - // doesn't shuffle the option list up and down under it. + // Fixed height, so switching between a wide and a narrow face doesn't + // shuffle the option list under it. minLines = 2, modifier = Modifier .fillMaxWidth() @@ -422,9 +389,8 @@ private fun FontSpecimen(role: FontRole, family: FontFamily?) { } /** - * One row in the [FontPicker]: the font's name, a leading "Ag" sample rendered in - * the option's own [preview] face (or an [leadingIcon] cue when there's nothing - * to preview), and a check when it's the current selection. + * One row in the [FontPicker]: the font's name, an "Ag" sample in its own + * [preview] face (or an [leadingIcon] cue), and a check when selected. */ @Composable private fun FontOptionRow( @@ -460,11 +426,8 @@ private fun FontOptionRow( } /** - * One selectable launcher-name preview in the App name picker (issue #44): the - * app's launcher mark over the name, framed as a card. The active one carries a - * primary border, a tinted container and a check; tapping selects it. The mark - * is the same for both — only the label changes — so the card previews exactly - * what the home screen will read. + * One selectable launcher-name preview in the App name picker (#44): the app's + * launcher mark over the name, framed as a card. */ @Composable private fun AppNameOptionCard( @@ -494,8 +457,8 @@ private fun AppNameOptionCard( horizontalAlignment = Alignment.CenterHorizontally, verticalArrangement = Arrangement.spacedBy(12.dp), ) { - // The adaptive launcher mark, reconstructed as a squircle (as in the - // onboarding BrandHero) so it renders identically everywhere. + // The adaptive mark rebuilt as a squircle, as in the onboarding + // BrandHero, so it renders identically everywhere. Box( modifier = Modifier .size(64.dp) @@ -515,7 +478,6 @@ private fun AppNameOptionCard( textAlign = TextAlign.Center, maxLines = 1, ) - // Selection indicator: a filled check when active, an empty ring otherwise. Box( modifier = Modifier .size(24.dp) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/EventFormSettings.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/EventFormSettings.kt index 9f6c8a3..585fd28 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/EventFormSettings.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/EventFormSettings.kt @@ -40,8 +40,7 @@ internal fun EventFormScreen( GroupedRow( title = stringResource(eventFormFieldLabel(field)), position = positionOf(index, fields.size), - // Same icon the field carries in the new-event form, so a toggle - // is easy to match to the field it controls. + // The same icon the field carries in the new-event form. leading = { Icon( imageVector = eventFormFieldIcon(field), @@ -59,9 +58,7 @@ internal fun EventFormScreen( ) } - // Auto-focus the title on a new event (issue #10) — on by default, since - // most events get a title; raising the keyboard saves a tap. Off lets you - // set the time/calendar first without the keyboard in the way. + // Auto-focus the title on a new event (#10), on by default. Spacer(Modifier.height(24.dp)) GroupedRow( title = stringResource(R.string.settings_autofocus_title), @@ -84,8 +81,7 @@ internal fun EventFormScreen( ) // Per-event colour on calendars that publish no colour set (some - // CalDAV) — off by default, with the honest caveat that the colour may - // not survive their next sync. Local and palette calendars ignore it. + // CalDAV); off by default, since it may not survive their next sync. Spacer(Modifier.height(24.dp)) GroupedRow( title = stringResource(R.string.settings_color_unsupported), diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/NotificationSettings.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/NotificationSettings.kt index 2242e41..3735b30 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/NotificationSettings.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/NotificationSettings.kt @@ -124,11 +124,8 @@ internal fun NotificationsScreen( onClick = { showAllDayReminderTime = true }, ) - // Delivery reliability + snooze: both are global reminder-delivery - // settings, so they sit with the defaults above rather than below the - // long per-calendar list. Reliability is a soft, optional battery- - // optimisation exemption (system-settings deep-link, no special - // permission); shown as live status, reversible at any time. + // Reliability is a soft, optional battery-optimisation exemption via + // a system-settings deep-link, shown as live status. Spacer(Modifier.height(24.dp)) val batteryExempt = rememberBatteryOptimizationExempt() GroupedRow( @@ -147,7 +144,6 @@ internal fun NotificationsScreen( onClick = { openBatteryOptimizationSettings(context) }, ) - // Snooze: how long the notification's "Snooze" action defers a reminder. GroupedRow( title = stringResource(R.string.settings_snooze_duration), summary = snoozeDurationLabel(state.snoozeMinutes), @@ -155,10 +151,9 @@ internal fun NotificationsScreen( onClick = { showSnooze = true }, ) - // Per-calendar overrides: the whole section folds behind one header to - // keep the screen tidy. Expanded, each writable calendar gets its own - // expandable card that may keep, drop, or replace the global default — - // separately for timed and all-day events. + // Per-calendar overrides, folded behind one header. Each writable + // calendar may keep, drop or replace the global default, separately for + // timed and all-day events. if (state.writableCalendars.isNotEmpty()) { Spacer(Modifier.height(24.dp)) GroupedRow( @@ -182,8 +177,8 @@ internal fun NotificationsScreen( Column { state.writableCalendars.forEach { calendar -> Spacer(Modifier.height(16.dp)) - // A contact special-dates calendar owns its reminders in - // its own section — link there instead of an override. + // Special-dates calendars own their reminders in their + // own section — link there instead. if (calendar.id in state.managedCalendarIds) { GroupedRow( title = calendar.displayName, @@ -202,8 +197,6 @@ internal fun NotificationsScreen( return@forEach } val expanded = calendar.id in expandedCalendars - // Calendar card; tapping expands it into a grouped list - // of three (the card + the timed and all-day rows). GroupedRow( title = calendar.displayName, position = if (expanded) Position.Top else Position.Alone, @@ -366,8 +359,7 @@ private fun snoozeDurationLabel(minutes: Int): String = /** * Whether Calendula is exempt from battery optimisation, re-read on every - * `ON_RESUME` so the row reflects a change the user just made in system - * settings without needing to leave and re-enter the screen. + * `ON_RESUME` so a change made in system settings shows up at once. */ @Composable private fun rememberBatteryOptimizationExempt(): Boolean { @@ -392,10 +384,8 @@ private fun isIgnoringBatteryOptimizations(context: Context): Boolean { } /** - * Take the user straight to Calendula's exemption: the direct - * `REQUEST_IGNORE_BATTERY_OPTIMIZATIONS` dialog ("Allow Calendula to ignore - * battery optimisation?") rather than the full app list they'd have to scroll. - * Falls back to the optimisation list if the OS refuses the direct intent. + * Open the direct `REQUEST_IGNORE_BATTERY_OPTIMIZATIONS` dialog, falling back to + * the optimisation list if the OS refuses it. */ private fun openBatteryOptimizationSettings(context: Context) { val direct = Intent( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsCommon.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsCommon.kt index cb36a77..ebba693 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsCommon.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsCommon.kt @@ -31,23 +31,13 @@ import de.jeanlucmakiola.floret.locale.currentLocale */ /** - * Token-based accent for a leading icon chip (container / on-container pair). - * Neutral chips stay grey; accents are drawn from the M3 scheme so they adapt - * to theme, dark mode and dynamic colour. - * - * The chips are a scanning aid, so the rule is **no two rows in one group share - * an accent** — a group in a single colour carries no more information than no - * colour at all. [Neutral] is not a fourth colour to rotate through but a - * deliberate step back, for rows that are reference or last-resort rather than - * somewhere you routinely go. + * Accent for a leading icon chip. The chips are a scanning aid, so no two rows + * in one group share an accent; [Neutral] is a step back for reference rows + * rather than a fourth colour to rotate through. */ internal enum class ChipAccent { Neutral, Primary, Secondary, Tertiary } -/** - * Leading circular icon chip. Colours come from the M3 scheme via a container / - * on-container token pair, so each accent stays correctly paired across theme, - * dark mode and dynamic colour. - */ +/** Leading circular icon chip, coloured by an M3 container/on-container pair. */ @Composable internal fun CategoryIcon(icon: ImageVector, accent: ChipAccent) { val scheme = MaterialTheme.colorScheme @@ -75,9 +65,7 @@ internal fun CategoryIcon(icon: ImageVector, accent: ChipAccent) { /** * A small primary-coloured group label, matching the Calendars settings screen. - * - * Starts on [GroupedListInset] — the cards' own edge — so a header, the hint - * under it and the group it names all share one left margin. + * Inset to [GroupedListInset] so it shares the cards' left margin. */ @Composable internal fun SectionHeader(text: String) { @@ -117,34 +105,18 @@ internal fun reminderChoiceLabel(minutes: List): String { return minutes.map { reminderLeadTimeLabel(it) }.joinToString(", ") } -/** - * Lead times offered for the all-day default — day-scale, since a "minutes - * before midnight" reminder on an all-day event is rarely what's wanted. Shared - * with the contact special-dates calendars, whose events are all all-day. - */ +/** Day-scale lead times for all-day defaults and contact special dates. */ internal val ALLDAY_REMINDER_PRESETS = listOf(0, 1_440, 2_880, 10_080) -/** - * A minute-of-day ("09:00") in the app's *own* 12/24-hour convention — the same - * [LocalUse24HourFormat] the agenda and event rows read, so a time shown in - * settings can't disagree with the times shown everywhere else. - */ +/** A minute-of-day in the app's own 12/24-hour convention ([LocalUse24HourFormat]). */ @Composable internal fun settingsTimeOfDay(minutesOfDay: Int): String = formatMinuteOfDay(minutesOfDay, LocalUse24HourFormat.current, currentLocale()) /** - * Second line for an all-day lead-time row: the clock time it fires at. - * - * An all-day occurrence has no time of its own, so a reminder's stored offset - * only decides *which day* it belongs to — the hour always comes from the - * global "show all-day reminders at" setting (see - * [de.jeanlucmakiola.calendula.domain.reminders.planReminders]). Naming it on - * every row is what makes "At time of event" readable on an all-day default at - * all: it means that day at 09:00, not midnight. - * - * Shared by the notification defaults, the per-calendar overrides and the - * contact special-dates calendars — all three read the same one setting. + * Second line for an all-day lead-time row: the clock time it fires at. The + * offset only decides which day; the hour comes from the global all-day setting + * (see [de.jeanlucmakiola.calendula.domain.reminders.planReminders]). */ @Composable internal fun allDayFiringTimeSummary(allDayReminderTimeMinutes: Int): @Composable (Int) -> String? { 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 f28dbff..5894d9d 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 @@ -70,15 +70,10 @@ import de.jeanlucmakiola.floret.locale.AppLanguage private enum class SettingsSection { Appearance, Views, EventForm, Notifications, SpecialDates, Widgets } /** - * Settings (M4), restructured in v2.3 into a category hub with sub-screens and - * re-sorted in v2.17 (#69) so every setting sits where it is looked for. - * - * The hub reads as three labelled groups — what the app looks like and how it - * behaves, what it does with your data, and the app itself — and each sub-screen - * lives in its own `*Settings.kt` file. Calendars and Backup open screens - * hoisted in `CalendarHost` (they are driven by the calendar list, not by - * preferences); Language opens a full-screen picker; About is a card at the top. - * A full-screen destination; [onBack] pops it. + * Settings hub: three labelled groups of category rows, each opening a + * sub-screen from its own `*Settings.kt` file (#69). Calendars and Backup open + * screens hoisted in `CalendarHost`; Language opens a full-screen picker. + * [onBack] pops the destination. */ @Composable fun SettingsScreen( @@ -166,18 +161,14 @@ private fun SettingsHub( onOpenBackup: () -> Unit, ) { CollapsingScaffold(title = stringResource(R.string.settings_title), onBack = onBack, predictiveBack = true) { - // The card and the support row are one grouped block: the call to action - // is a row continuing the container rather than a tonal button sitting - // inside the card. + // Card and support row are one grouped block, so the call to action + // continues the container instead of sitting inside it as a button. Box(Modifier.padding(horizontal = GroupedListInset)) { AboutCard() } SupportRow() Spacer(Modifier.height(16.dp)) - // Three labelled groups instead of one long undifferentiated list (#69): - // how the app presents itself, what it does with your data, and the app - // as an installed thing. Each group cycles its chip accents so no two - // rows in it look alike (see [ChipAccent]) — a uniformly coloured group - // is as unscannable as an uncoloured one. + // Each group cycles its chip accents so no two rows in it look alike + // (see [ChipAccent]). SectionHeader(stringResource(R.string.settings_group_look)) GroupedRow( title = stringResource(R.string.settings_section_appearance), @@ -224,9 +215,8 @@ private fun SettingsHub( leading = { CategoryIcon(Icons.Default.Cake, ChipAccent.Primary) }, onClick = { onOpenSection(SettingsSection.SpecialDates) }, ) - // Export/import used to hide inside the calendar manager, where nobody - // looked for it (#69). It is a data-safety feature, so it gets its own - // top-level entry; the manager keeps a pointer row to here. + // Export/import gets its own top-level entry (#69); the calendar + // manager keeps a pointer row to here. GroupedRow( title = stringResource(R.string.settings_section_backup), summary = stringResource(R.string.settings_backup_subtitle), @@ -247,10 +237,8 @@ private fun SettingsHub( LanguageRow(position = Position.Middle) ReportProblemRow(position = Position.Bottom) - // Source, licence and privacy sit at the very bottom rather than in the - // About card: they are reference material you look up once, not settings - // you come here to change, and the card at the top now leads with what - // the app *is* plus the one link worth offering (support). + // Source, licence and privacy sit at the bottom as reference material + // rather than inside the About card. Spacer(Modifier.height(8.dp)) SectionHeader(stringResource(R.string.settings_group_about)) AboutLinkRow( @@ -364,11 +352,8 @@ private fun languageLabel(tag: String?): String = @Composable private fun AboutCard() { - // The card layout lives in floret-kit (components.AboutCard); Calendula - // supplies its own logo and author. Source, licence and privacy moved to - // rows at the foot of the page, and support is now the [SupportRow] joined - // to the bottom of this card — so the card itself carries nothing but what - // the app is and who made it. + // Layout lives in floret-kit (components.AboutCard); Calendula supplies + // its own logo and author. Support is the [SupportRow] joined below. AboutCard( logo = { AppLogo() }, appName = stringResource(R.string.app_name), @@ -378,11 +363,7 @@ private fun AboutCard() { ) } -/** - * "Support development", as the closing row of the About card's group rather - * than a tonal button inside it. Same weight as any other row you can tap, with - * a primary chip to keep it the one accent in this block. - */ +/** "Support development", as the closing row of the About card's group. */ @Composable private fun SupportRow() { val context = LocalContext.current @@ -396,10 +377,9 @@ private fun SupportRow() { } /** - * One reference link at the foot of the hub (source, licence, privacy): a plain - * grouped row that opens the URL in the browser. The privacy policy in - * particular has to stay reachable from inside the app, not only from the store - * listing, because Calendula touches calendar and contact data. + * One reference link at the foot of the hub (source, licence, privacy). The + * privacy policy has to stay reachable from inside the app, not only from the + * store listing. */ @Composable private fun AboutLinkRow(label: String, url: String, icon: ImageVector, position: Position) { @@ -433,15 +413,9 @@ private fun AppVersionText() { } /** - * The app icon as a rounded chip: the off-white launcher mark over its slate - * background colour, rendered oversized and clipped to fill the chip the way a - * launcher mask would. - * - * Sized just past the two text lines beside it, so the card's height is set by - * its content rather than by the mark towering over it. That also keeps it a - * step above the 40dp category chips on the rows below — still the block's - * anchor, no longer its outlier. The mark is a vector, so the size is purely a - * layout choice; [MARK_OVERSCAN] holds the launcher-mask crop as it shrinks. + * The app icon as a rounded chip: the launcher mark over its background colour, + * oversized and clipped the way a launcher mask would. Sized just past the two + * text lines beside it; [MARK_OVERSCAN] holds the crop as it shrinks. */ @Composable private fun AppLogo() { @@ -464,8 +438,7 @@ private fun AppLogo() { private val LOGO_SIZE = 56.dp /** - * How far the mark overruns its chip. An adaptive icon's foreground is drawn - * with a wide safe margin, so at 1:1 the flower would sit small and lost inside - * the chip; the launcher crops the same way. + * How far the mark overruns its chip. An adaptive icon's foreground carries a + * wide safe margin, so at 1:1 it would sit small and lost; launchers crop it too. */ private const val MARK_OVERSCAN = 1.5f diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt index fddee1d..0e6000d 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt @@ -80,9 +80,8 @@ class SettingsViewModel @Inject constructor( /** * Writable calendars that are switched on — the only ones that take a - * per-calendar reminder override. A calendar switched off in Settings → - * Calendars is `VISIBLE = 0`, so the provider schedules no alarms for it and - * a default reminder configured there could never fire (#75). + * per-calendar reminder override, since a switched-off one plans no + * reminders at all (#75). */ private val writableCalendars: Flow> = repository.calendars() .map { calendars -> calendars.filter { it.canModifyContents && it.isVisibleInSystem } } @@ -225,17 +224,10 @@ class SettingsViewModel @Inject constructor( ) /** - * The launcher-label choice (issue #44). Backed by the manifest aliases' - * component-enabled state rather than a stored preference, so it's read - * 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 - * 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. + * The launcher-label choice (#44), backed by the manifest aliases' + * component-enabled state rather than a preference. Seeded with the manifest + * default and corrected off-thread — `getComponentEnabledSetting` is a + * binder round-trip and this ViewModel is built on the main thread. */ private val _launcherName = MutableStateFlow(LauncherName.CALENDULA) val launcherName: StateFlow = _launcherName.asStateFlow() @@ -251,10 +243,8 @@ class SettingsViewModel @Inject constructor( private val _fontImportFailed = MutableSharedFlow(extraBufferCapacity = 1) val fontImportFailed: SharedFlow = _fontImportFailed - // Serialises widget redraws so a rapid flip-and-flip-back can't run two - // updateAll calls at once — concurrent calls coalesce around a stale read - // and can leave the widget on the intermediate value. Held sequentially, - // the last call reads the final committed pref and renders it. + // Serialises widget redraws: concurrent updateAll calls coalesce around a + // stale read and can leave the widget on an intermediate value. private val widgetRefreshMutex = Mutex() private data class ReminderDefaults( @@ -407,9 +397,7 @@ class SettingsViewModel @Inject constructor( fun setSoftenColors(enabled: Boolean) { viewModelScope.launch { prefs.setSoftenCalendarColors(enabled) - // Both widgets paint event colours through the same softener, so a - // change has to redraw them too (they read the flag in their data - // preamble, like is24Hour). + // Both widgets paint event colours through the same softener. widgetRefreshMutex.withLock { AgendaWidget().updateAll(appContext) MonthWidget().updateAll(appContext) @@ -435,9 +423,8 @@ class SettingsViewModel @Inject constructor( viewModelScope.launch { val ok = withContext(io) { CustomFontStore.import(appContext, role, uri) } if (ok) { - // Bump the stamp so replacing an already-active custom font (same - // file, same "custom" token) still breaks font-settings equality - // and refreshes the resolved typeface. + // Bump the stamp so replacing an already-active custom font + // still breaks equality and refreshes the resolved typeface. prefs.bumpCustomFontStamp(role) setFont(role, FONT_CUSTOM_TOKEN) } else { @@ -466,10 +453,8 @@ class SettingsViewModel @Inject constructor( viewModelScope.launch { prefs.setAgendaWidgetRange(range) widgetRefreshMutex.withLock { - // Push the range into each instance's Glance state and recompose. - // The widget reads it via currentState, so this reflects reliably - // even when updateAll only recomposes a live session (it does not - // re-run provideGlance's data preamble). + // Push into each instance's Glance state and recompose: + // updateAll alone won't re-run provideGlance's data preamble. val manager = GlanceAppWidgetManager(appContext) manager.getGlanceIds(AgendaWidget::class.java).forEach { id -> updateAppWidgetState(appContext, id) { it[AGENDA_RANGE_KEY] = range.storageValue() } @@ -484,13 +469,9 @@ class SettingsViewModel @Inject constructor( } /** - * Set the size the agenda widget draws its text at (#51). Pushed into every - * instance's Glance state and recomposed — the same reliable-update path as - * [setAgendaWidgetRange], since `updateAll` alone won't re-run the data - * preamble a live session already ran. - * - * Agenda-only on purpose: the month widget sizes itself from the space it is - * given and always has, so it takes no size setting (#103). + * Set the size the agenda widget draws its text at (#51), via the same + * Glance-state path as [setAgendaWidgetRange]. Agenda-only: the month widget + * sizes itself from the space it is given (#103). */ fun setWidgetSize(size: WidgetSize) { viewModelScope.launch { @@ -508,9 +489,7 @@ class SettingsViewModel @Inject constructor( fun setAgendaShowToday(enabled: Boolean) { viewModelScope.launch { prefs.setAgendaShowToday(enabled) - // The agenda widget reads this reactively, so push it into each - // instance's Glance state and recompose — same reliable-update path as - // setPastEventDisplay (updateAll alone won't re-run the data preamble). + // Same Glance-state path as setPastEventDisplay. widgetRefreshMutex.withLock { val manager = GlanceAppWidgetManager(appContext) manager.getGlanceIds(AgendaWidget::class.java).forEach { id -> @@ -538,10 +517,9 @@ class SettingsViewModel @Inject constructor( } /** - * 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. + * Switch the launcher label (#44). The card highlights at once, then settles + * on what the component state reports; the writes are binder round-trips and + * run off the main thread. */ fun setLauncherName(name: LauncherName) { _launcherName.value = name @@ -556,9 +534,7 @@ class SettingsViewModel @Inject constructor( fun setPastEventDisplay(mode: PastEventDisplay) { viewModelScope.launch { prefs.setPastEventDisplay(mode) - // The agenda widget honours this setting too, so push it into each - // instance's Glance state and recompose — same reliable-update path as - // setAgendaWidgetRange (updateAll alone won't re-run the data preamble). + // Same Glance-state path as setAgendaWidgetRange. widgetRefreshMutex.withLock { val manager = GlanceAppWidgetManager(appContext) manager.getGlanceIds(AgendaWidget::class.java).forEach { id -> @@ -582,12 +558,10 @@ class SettingsViewModel @Inject constructor( } /** - * Enable or disable [view] in the quick-switch cycle. Goes through an atomic - * read-modify-write so a concurrent reorder can't clobber this toggle (and - * vice versa) by re-serialising a stale snapshot. The MIN_ENABLED floor is - * re-checked inside the transform: the screen's own guard reads the - * async-echoed snapshot, so two quick disable-taps could both look allowed - * yet compose to a below-minimum cycle. + * Enable or disable [view] in the quick-switch cycle, via an atomic + * read-modify-write so a concurrent reorder can't clobber it. The + * MIN_ENABLED floor is re-checked inside the transform, because the screen's + * own guard reads an async-echoed snapshot. */ fun setQuickSwitchViewEnabled(view: CalendarView, enabled: Boolean) { viewModelScope.launch { @@ -612,9 +586,8 @@ class SettingsViewModel @Inject constructor( } /** - * A scan follows the write, in that order. Switching reminders off cancels - * the scan alarm, so switching them back on has to arm a new one — nothing - * else would until a provider change or the daily worker came along (#75). + * A scan follows the write: switching reminders off cancels the scan alarm, + * so switching them back on has to arm a new one (#75). */ fun setRemindersEnabled(enabled: Boolean) { viewModelScope.launch { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SpecialDatesSettings.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SpecialDatesSettings.kt index 5627285..8933214 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SpecialDatesSettings.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SpecialDatesSettings.kt @@ -48,14 +48,10 @@ internal fun SpecialDatesScreen( onBack: () -> Unit, ) { val state by viewModel.specialDatesState.collectAsStateWithLifecycle() - // The all-day reminder hour lives in the general settings state; the reminder - // picker below names it, since that is when these reminders actually fire. val settings by viewModel.state.collectAsStateWithLifecycle() val context = LocalContext.current - // READ_CONTACTS is requested only here, on enable — never at startup. On a - // grant we either enable (if turning on) or just re-sync (clearing a stalled - // banner after the permission was re-granted). + // READ_CONTACTS is requested only here, on enable — never at startup. val permissionLauncher = rememberLauncherForActivityResult( contract = ActivityResultContracts.RequestPermission(), ) { granted -> @@ -122,8 +118,6 @@ internal fun SpecialDatesScreen( ) if (state.enabled) { - // Per-type card: the toggle, and while on, its title format and its - // calendar-wide reminder. SpecialDateType.entries.forEachIndexed { index, type -> Spacer(Modifier.height(if (index == 0) 24.dp else 16.dp)) val on = type in state.types @@ -222,8 +216,6 @@ internal fun SpecialDatesScreen( allowInherit = false, onSelect = { viewModel.setSpecialDatesReminders(type, it) }, onDismiss = { reminderPickerType = null }, - // Special dates are all-day events, so their reminders fire at the - // one global all-day hour like any other all-day reminder. leadTimeSummary = allDayFiringTimeSummary(settings.allDayReminderTimeMinutes), ) } @@ -268,8 +260,7 @@ private fun SpecialDatesTemplateDialog( title = { Text(stringResource(R.string.settings_special_dates_template)) }, text = { Column { - // The app's borderless input over a tonal surface (the dialog - // convention — see DialogControls), not Material's outlined field. + // The dialog convention — see DialogControls. Surface( color = MaterialTheme.colorScheme.surfaceContainerHighest, shape = RoundedCornerShape(12.dp), diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/ViewsSettings.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/ViewsSettings.kt index 5b72f3a..9483132 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/ViewsSettings.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/ViewsSettings.kt @@ -52,15 +52,9 @@ import java.time.format.TextStyle as JavaTextStyle * Views: everything that changes how a calendar view reads, grouped by the view * it belongs to, plus the two cross-view ordering lists (#24, #69). * - * The first group holds what applies everywhere (default view, week start, time - * format); the rest are per-view. Anything that styles the *app* rather than a - * view (theme, fonts) is in [AppearanceScreen]; the widget's own copies of the - * agenda settings are in [WidgetsScreen]. - * - * The quick-switch cycle and the navigation-drawer list are two independent - * orders — a view disabled in the quick-switch cycle is still reachable from the - * drawer, which always lists every view. The switch needs at least two targets, - * so the last [QuickSwitchConfig.MIN_ENABLED] enabled views can't be turned off. + * The quick-switch cycle and the drawer list are independent orders — a view + * off in the cycle is still reachable from the drawer. The cycle needs at least + * [QuickSwitchConfig.MIN_ENABLED] targets. */ @Composable internal fun ViewsScreen( @@ -266,8 +260,6 @@ internal fun ViewsScreen( options = IMPLEMENTED_VIEWS, selected = state.defaultView, label = { stringResource(it.labelRes) }, - // The same icon each view carries in the drawer and the switcher - // pill, so the row is matched to the view by eye, not by reading. leading = { Icon( imageVector = it.icon, @@ -299,9 +291,8 @@ internal fun ViewsScreen( options = TimeFormatPref.entries, selected = state.timeFormat, label = { timeFormatLabel(it) }, - // Each option renders the same sample time the way it would write - // it — an afternoon one, since that is where 12h and 24h diverge. - // "Automatic" additionally says which of the two it resolves to now. + // Each option renders the same sample time its own way; + // "Automatic" also names which of the two it resolves to. summary = { pref -> val sample = formatTimeOfDay( hour = SAMPLE_HOUR, @@ -335,8 +326,6 @@ internal fun ViewsScreen( title = stringResource(R.string.settings_agenda_range), description = stringResource(R.string.settings_agenda_range_hint), selected = state.agendaScreenRange, - // Resolving each option to real dates needs the same week start the - // agenda itself windows by. weekStart = state.weekStart.resolveFirstDay(currentLocale()), onSelect = viewModel::setAgendaScreenRange, onDismiss = { showAgendaScreenRange = false }, @@ -400,8 +389,7 @@ private val WEEK_START_OPTIONS: List = @Composable internal fun weekStartLabel(pref: WeekStartPref): String = when (pref) { WeekStartPref.Auto -> stringResource(R.string.settings_week_start_auto) - // Localised full weekday name, so any of the seven days reads naturally - // without a per-day string resource. + // Localised weekday name, so no per-day string resource is needed. is WeekStartPref.Day -> java.time.DayOfWeek.of(pref.day.ordinal + 1) .getDisplayName(JavaTextStyle.FULL, currentLocale()) } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/WeekStartPicker.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/WeekStartPicker.kt index 8a04727..732d2b1 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/WeekStartPicker.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/WeekStartPicker.kt @@ -31,20 +31,11 @@ import de.jeanlucmakiola.floret.identity.rememberReduceMotion import de.jeanlucmakiola.floret.locale.currentLocale /** - * The week-start chooser, with the grid it rearranges shown above it. + * The week-start chooser, with the grid it rearranges above it — the same + * [MonthStylePreview] the Month style picker uses, drawn in the user's own month + * style. "Follow the system" carries the day it resolves to as its summary. * - * Which column a week opens on is the most visual setting in the app, and the - * one hardest to hold in your head from a weekday name alone. The preview is - * the same [MonthStylePreview] the Month style picker uses, drawn in the user's - * *own* month style, so what moves is exactly what will move on the real - * screen. It is short here (the option list runs to eight rows), which the - * preview supports natively by scaling. - * - * "Follow the system" carries the day it currently resolves to as its summary — - * otherwise the one automatic option is the only one whose effect is invisible. - * - * A preview picker, so selecting applies at once and keeps the picker open; - * back exits (see `FullScreenPicker`). + * A preview picker, so selecting applies at once and keeps the picker open. */ @Composable internal fun WeekStartPicker( @@ -93,8 +84,6 @@ internal fun WeekStartPicker( val isSelected = option == selected GroupedRow( title = weekStartLabel(option), - // Only the automatic option needs a second line: it is the one - // whose effect the label doesn't name. summary = if (option == WeekStartPref.Auto) { stringResource( R.string.settings_week_start_auto_summary, diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/WidgetSettings.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/WidgetSettings.kt index e0cf871..8aa6e34 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/WidgetSettings.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/WidgetSettings.kt @@ -33,13 +33,9 @@ import de.jeanlucmakiola.floret.locale.currentLocale import kotlin.math.roundToInt /** - * Widgets & tiles (#69): the home-screen widgets' own settings and the Quick - * Settings tile shortcut — the app's system surfaces, collected in one place. - * - * The agenda widget keeps its own range and text size, deliberately separate - * from the Agenda *screen*'s range in [ViewsScreen]: a widget is glanced at, a - * screen is browsed, and having the two rows sit next to each other (as they did - * before) made them easy to mistake for one another. + * Widgets & tiles (#69): the home-screen widgets' settings and the Quick + * Settings tile shortcut. The agenda widget keeps its own range and text size, + * separate from the Agenda screen's range in [ViewsScreen]. */ @Composable internal fun WidgetsScreen( @@ -71,9 +67,8 @@ internal fun WidgetsScreen( onClick = { showWidgetSize = true }, ) - // One-tap add of the "New event" Quick Settings tile. The system prompt - // is API 33+; on older versions the tile is still addable manually from - // the QS editor, so the row simply doesn't appear here. + // The add-tile prompt is API 33+; below that the tile is still + // addable from the QS editor, so the row just doesn't appear. if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { Spacer(Modifier.height(24.dp)) QuickSettingsTileRow() @@ -98,10 +93,8 @@ internal fun WidgetsScreen( options = WidgetSize.entries, selected = state.widgetSize, label = { widgetSizeLabel(it) }, - // A size name says nothing about what you get; the text scale it - // draws at does. No live preview here on purpose: the widget is - // Glance/RemoteViews, so a Compose mock-up would be a second - // implementation free to drift from the real thing. + // No live preview: the widget is Glance/RemoteViews, so a Compose + // mock-up would be a second implementation free to drift. summary = { widgetSizeSummary(it) }, onSelect = viewModel::setWidgetSize, onDismiss = { showWidgetSize = false }, @@ -138,9 +131,8 @@ private fun requestAddQsTile(context: Context) { } /** - * What a size step actually does, as a percentage of the smallest one's event - * text. Derived from the widget's own [metricsFor] table rather than written - * out here, so a retuned step can't leave this line claiming the old number. + * What a size step does, as a percentage of the smallest one's event text. + * Derived from the widget's own [metricsFor] table rather than hardcoded. */ @Composable private fun widgetSizeSummary(size: WidgetSize): String { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/widget/WidgetSize.kt b/app/src/main/java/de/jeanlucmakiola/calendula/widget/WidgetSize.kt index f03b0ab..3a7ff5e 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/widget/WidgetSize.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/widget/WidgetSize.kt @@ -2,19 +2,9 @@ package de.jeanlucmakiola.calendula.widget /** * The size step the **agenda** widget draws its text and rows at — a user - * setting, not something derived from the widget's measured size (#51). + * setting, not the measured size (#51), which a launcher reports inaccurately. + * The month widget keeps sizing itself from the width it is given (#103). * - * The agenda widget used to bucket its live size (`SizeMode.Exact` + - * `LocalSize.current`) into a tier and scale from that. The size a launcher - * reports is not the size the widget is drawn into, so the tier it picked could - * disagree with what was on screen; a size the user sets is predictable and is - * what the #51 thread actually asked for. - * - * The month widget deliberately has no size setting: its grid divides whatever - * width it is given by seven and always has, which is the behaviour to keep - * (#103). - * - * [SMALL] is the default and reproduces the widget's original constants, so an - * existing widget looks as it did until its owner turns the size up. + * [SMALL] is the default and reproduces the widget's original constants. */ enum class WidgetSize { SMALL, MEDIUM, LARGE, EXTRA_LARGE } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/widget/agenda/AgendaScale.kt b/app/src/main/java/de/jeanlucmakiola/calendula/widget/agenda/AgendaScale.kt index 4e7540c..3414741 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/widget/agenda/AgendaScale.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/widget/agenda/AgendaScale.kt @@ -8,10 +8,8 @@ import de.jeanlucmakiola.calendula.widget.WidgetSize /** * Horizontal layout constants for an agenda event row. These don't scale with the - * size step — a wider stripe or gap would eat title width, which is the thing the - * row is short of — but [TEXT_INDENT] is *derived* from them so the day header and - * the "nothing left today" line can never drift out of alignment with the event - * title column the way a hardcoded 19dp could. + * size step, since a wider stripe or gap would eat title width, but [TEXT_INDENT] + * is derived from them so non-event text can't drift out of alignment. */ internal val ROW_H_PAD = 4.dp internal val STRIPE_WIDTH = 5.dp @@ -39,34 +37,21 @@ internal data class AgendaMetrics( val dayHeaderTopPad: Dp, // space above a day header ) { /** - * Resolves the stripe height against the user's system font scale. - * - * The stripe is a [Dp] but the two text lines it sits beside are `sp`, so - * they scale with the accessibility font setting and the stripe does not — - * at "Largest" the text outgrows the stripe and it visibly under-runs the row - * it is supposed to mark. Multiplying by the same factor keeps them locked, - * and at the default scale of 1.0 reproduces the step's value exactly. + * Resolves the stripe height against the user's system font scale: the stripe + * is a [Dp] while the lines beside it are `sp`, so without this the text + * outgrows it at large font settings. */ fun scaledForFont(fontScale: Float): AgendaMetrics = if (fontScale == 1f) this else copy(stripeH = stripeH * fontScale) } /* - * Type sizes are anchored to the Material 3 type scale (see the `material-3` - * skill's typography reference) rather than invented: 16sp is Title Medium, 14sp - * Body Medium / Title Small, 12sp Body Small, 16sp Body Large, 22sp Title Large. + * Type sizes are anchored to the Material 3 type scale, with two deviations: * - * Two documented deviations: - * - * ‡ SMALL's 13sp day header is off-scale. It is held there deliberately — - * SMALL reproduces the widget's original constants verbatim so a widget whose - * owner never touches the size setting looks exactly as it did (#51), and - * snapping it to Title Small (14sp) would break that promise for a 1sp gain. - * - * † Above Title Medium the M3 scale jumps 16 → 22 → 24 with nothing in between, - * which is far too coarse for four size steps. Where a role would force a - * ≥1.4x step between adjacent steps we hold an interpolated value instead and - * mark it. The endpoints stay on real roles. + * ‡ SMALL's 13sp day header is off-scale, held there so SMALL reproduces the + * widget's original constants verbatim (#51). + * † Above Title Medium the M3 scale jumps 16 → 22 → 24, too coarse for four + * steps; interpolated values are used where a role would force a ≥1.4x jump. */ private val SMALL_METRICS = AgendaMetrics( diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/widget/agenda/AgendaWidget.kt b/app/src/main/java/de/jeanlucmakiola/calendula/widget/agenda/AgendaWidget.kt index 3130678..e092b9e 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/widget/agenda/AgendaWidget.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/widget/agenda/AgendaWidget.kt @@ -116,14 +116,10 @@ class AgendaWidget : GlanceAppWidget() { override val stateDefinition = PreferencesGlanceStateDefinition - // Single: type and row metrics come from the user's chosen WidgetSize, not - // from the widget's measured size (#103, #51), so there is nothing to gain - // from Glance building one RemoteViews per host size bucket — and plenty to - // lose, since that roughly doubles the serialized payload. The row list is - // still capped below: an uncapped agenda (the range goes up to - // AgendaRange.MAX_CUSTOM_DAYS = 365) could push the RemoteViews past the - // binder transaction limit and the host would just show "Problem loading - // widget". + // Single: metrics come from the chosen WidgetSize, not the measured one + // (#103, #51), so a RemoteViews per host size bucket would only double the + // payload. The row list stays capped below — an uncapped agenda could push + // the RemoteViews past the binder transaction limit. override val sizeMode = SizeMode.Single override suspend fun provideGlance(context: Context, id: GlanceId) { @@ -164,10 +160,8 @@ private sealed interface AgendaRow { @Composable private fun AgendaWidgetBody(data: AgendaWidgetData, dark: Boolean) { - // Type and row metrics come from the user's chosen size step, read reactively - // from per-instance Glance state and falling back to the saved pref for a - // freshly placed widget (#103, #51). The permission screen has no loaded prefs - // to fall back to, so it takes the default. + // Read reactively from per-instance Glance state, falling back to the saved + // pref for a freshly placed widget (#103, #51). // The stripe is then re-resolved against the system font scale so it tracks the // sp-sized text beside it instead of drifting at large accessibility settings. val savedSize = (data as? AgendaWidgetData.Ready)?.savedWidgetSize ?: WidgetSize.SMALL diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImplTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImplTest.kt index 9baa38f..575b4ec 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImplTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImplTest.kt @@ -408,11 +408,9 @@ class CalendarRepositoryImplTest { fun `a flushed switch-off never reads as on again before the provider ticks`( @TempDir tempDir: Path, ) = runTest { - // The reconciler's shape: write VISIBLE = 0 straight to the provider, - // then release the id app-side. The provider's notification only arrives - // afterwards (it is dispatched through the main looper), so the release - // must not be read against the snapshot from before the write — that - // would flash exactly the events being hidden back into every view. + // The reconciler's shape: write VISIBLE = 0, then release the id + // app-side. The provider's notification arrives afterwards, so the + // release must not be read against the pre-write snapshot. val prefs = newPrefs(tempDir) prefs.addPendingDisabledCalendarIds(setOf(2L)) val fake = FakeCalendarDataSource().apply {