From 9dd41420b62b91ca030b9276bed53519a4d4e6a4 Mon Sep 17 00:00:00 2001 From: Guido Schmit Date: Thu, 2 Jul 2026 18:30:21 +0200 Subject: [PATCH] fix: safe performance and crash-safety fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - TimeGridView: remember Instant.now() (keyed on the day) and the day-header DateTimeFormatter instead of rebuilding both on every recomposition/frame — fixes flicker in the past-event dimming and avoids repeated allocation. - CalendarViewModel.refreshFromCache: short-circuit the non-group filter pass when both hiddenKeys and banishedKeys are empty, mirroring the existing group-mode short-circuit. - CalendarViewModel.setCalendarHiddenSynced: wrap the server PATCH in runCatching and surface failures via state.error instead of silently dropping them (the local toggle would otherwise diverge from the server unnoticed). - CalendarViewModel.afterMutation: refetch only the already-cached range (force-bypassing loadRange's cache check) instead of invalidateCache() + initialLoad(), which reloaded the entire ±cacheMonths window and froze the UI on every single event save. - CalendarrRoot: replace calendarVm!! with a null-safe ?.let, falling back to the splash screen instead of crashing if the VM isn't ready yet. - GroupsScreen: replace existing!!.icon!! / g.icon!! double force-unwraps with safe-call chains (existing?.icon?.takeIf { GroupIcons.isKey(it) }). Deliberately out of scope: R8/minify settings, MonthView's cache window size (MONTHS_BACK/MONTHS_AHEAD), and mergeIntoCache's merge logic (already reviewed as correct). No debounce added to setCalendarHiddenSynced — would need new machinery beyond this fix's scope. --- .../scarriffle/calendarr/ui/CalendarrRoot.kt | 16 +++++---- .../ui/calendar/CalendarViewModel.kt | 36 +++++++++++++++---- .../calendarr/ui/calendar/TimeGridView.kt | 12 +++++-- .../calendarr/ui/groups/GroupsScreen.kt | 4 +-- 4 files changed, 50 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/com/scarriffle/calendarr/ui/CalendarrRoot.kt b/app/src/main/java/com/scarriffle/calendarr/ui/CalendarrRoot.kt index 9a53551..75ff9b6 100644 --- a/app/src/main/java/com/scarriffle/calendarr/ui/CalendarrRoot.kt +++ b/app/src/main/java/com/scarriffle/calendarr/ui/CalendarrRoot.kt @@ -54,13 +54,15 @@ fun CalendarrRoot(vm: MainViewModel = hiltViewModel()) { onLoggedIn = vm::onLoggedIn, onBack = vm::switchServer, ) - AppRoute.MAIN -> CalendarScreen( - vm = calendarVm!!, - onLogout = vm::logout, - onSwitchServer = vm::switchServer, - onSettingsChanged = vm::applyLocalSettings, - onSettingsSynced = vm::refreshSettings, - ) + AppRoute.MAIN -> calendarVm?.let { cvm -> + CalendarScreen( + vm = cvm, + onLogout = vm::logout, + onSwitchServer = vm::switchServer, + onSettingsChanged = vm::applyLocalSettings, + onSettingsSynced = vm::refreshSettings, + ) + } ?: SplashScreen() } } } diff --git a/app/src/main/java/com/scarriffle/calendarr/ui/calendar/CalendarViewModel.kt b/app/src/main/java/com/scarriffle/calendarr/ui/calendar/CalendarViewModel.kt index b19ec09..b1d9fa0 100644 --- a/app/src/main/java/com/scarriffle/calendarr/ui/calendar/CalendarViewModel.kt +++ b/app/src/main/java/com/scarriffle/calendarr/ui/calendar/CalendarViewModel.kt @@ -229,10 +229,12 @@ class CalendarViewModel @Inject constructor( } /** Single serialized loader: avoids overlapping fetches that cause scroll jank. */ - private suspend fun loadRange(start: Instant, end: Instant, background: Boolean) { + private suspend fun loadRange(start: Instant, end: Instant, background: Boolean, force: Boolean = false) { loadMutex.withLock { // Another load (e.g. the background prefetch) may have covered this range. - if (isCached(start, end)) return + // `force` bypasses this so callers can explicitly re-fetch an already-cached + // range (e.g. to pick up a mutation) without wiping the whole cache first. + if (!force && isCached(start, end)) return val group = _state.value.activeGroup val flag = if (background) "bg" else "fg" _state.update { if (flag == "bg") it.copy(isBackgroundCaching = true) else it.copy(isLoading = true, error = null) } @@ -305,7 +307,8 @@ class CalendarViewModel @Inject constructor( } else { val hidden = st.hiddenKeys val banished = st.banishedKeys - allCachedEvents.filter { ev -> + if (hidden.isEmpty() && banished.isEmpty()) allCachedEvents + else allCachedEvents.filter { ev -> val key = calendarKey(ev.source, ev.calendarId) key !in hidden && key !in banished } @@ -346,7 +349,14 @@ class CalendarViewModel @Inject constructor( setCalendarHidden(key, hidden) val id = key.substringAfter(":").toIntOrNull() ?: return if (source in listOf("caldav", "google", "homeassistant")) { - viewModelScope.launch { repository.setCalendarSidebarHidden(source, id, hidden) } + viewModelScope.launch { + runCatching { repository.setCalendarSidebarHidden(source, id, hidden) } + .onFailure { e -> + // The local toggle already applied; only the server write failed — + // surface it so the client doesn't silently diverge from server state. + _state.update { it.copy(error = e.message ?: "Fehler beim Speichern") } + } + } } } @@ -427,9 +437,23 @@ class CalendarViewModel @Inject constructor( // ---- Event mutations ---- + /** + * Re-fetch just the already-cached range after a create/update so the edit is + * reflected everywhere it's currently loaded, without nuking and reloading the + * whole ±cacheMonths window (which caused a full-screen freeze on every save). + * Falls back to [initialLoad] if nothing was cached yet. + */ private fun afterMutation() { - invalidateCache() - initialLoad() + val start = cachedStart + val end = cachedEnd + if (start == null || end == null) { + initialLoad() + return + } + viewModelScope.launch { + loadRange(start, end, background = false, force = true) + markReady() + } } fun saveEvent( diff --git a/app/src/main/java/com/scarriffle/calendarr/ui/calendar/TimeGridView.kt b/app/src/main/java/com/scarriffle/calendarr/ui/calendar/TimeGridView.kt index 027e33c..a55b4d6 100644 --- a/app/src/main/java/com/scarriffle/calendarr/ui/calendar/TimeGridView.kt +++ b/app/src/main/java/com/scarriffle/calendarr/ui/calendar/TimeGridView.kt @@ -18,6 +18,7 @@ import androidx.compose.material3.Divider import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha @@ -47,16 +48,21 @@ fun TimeGridView( ) { val hourHeight = LocalAppSettings.current.hourHeight.coerceIn(28, 100).dp val dimPast = LocalAppSettings.current.dimPastEvents - val now = java.time.Instant.now() - val lang = LocalLang.current val today = LocalDate.now() + // Recomputed once per day (not on every recomposition/frame) — Instant.now() + // was previously called fresh each recomposition, causing visible flicker in + // the "is this now" past-dimming and wasted allocation. + val now = remember(today) { java.time.Instant.now() } + val lang = LocalLang.current Column(Modifier.fillMaxSize()) { // Day headers (only for multi-day / week view) if (days.size > 1) { Row(Modifier.fillMaxWidth()) { Box(Modifier.width(GUTTER)) - val dayFmt = DateTimeFormatter.ofPattern("EEE d", com.scarriffle.calendarr.ui.L10n.locale(lang)) + val dayFmt = remember(lang) { + DateTimeFormatter.ofPattern("EEE d", com.scarriffle.calendarr.ui.L10n.locale(lang)) + } days.forEach { day -> Text( dayFmt.format(day), diff --git a/app/src/main/java/com/scarriffle/calendarr/ui/groups/GroupsScreen.kt b/app/src/main/java/com/scarriffle/calendarr/ui/groups/GroupsScreen.kt index 52f4503..d57c822 100644 --- a/app/src/main/java/com/scarriffle/calendarr/ui/groups/GroupsScreen.kt +++ b/app/src/main/java/com/scarriffle/calendarr/ui/groups/GroupsScreen.kt @@ -208,7 +208,7 @@ private fun GroupEditSheet( ) { val me = vm.currentUserId var name by remember { mutableStateOf(existing?.name ?: "") } - var icon by remember { mutableStateOf(if (GroupIcons.isKey(existing?.icon)) existing!!.icon!! else "people") } + var icon by remember { mutableStateOf(existing?.icon?.takeIf { GroupIcons.isKey(it) } ?: "people") } var selected by remember { mutableStateOf(setOf()) } var existingMembers by remember { mutableStateOf(setOf()) } var detail by remember { mutableStateOf(null) } @@ -223,7 +223,7 @@ private fun GroupEditSheet( detail = g if (g != null) { name = g.name - icon = if (GroupIcons.isKey(g.icon)) g.icon!! else "people" + icon = g.icon?.takeIf { GroupIcons.isKey(it) } ?: "people" val members = g.members.map { it.id }.filter { it != me }.toSet() existingMembers = members selected = members