fix(settings): quick-switch write races, override parsing, stale custom fonts
Four fixes across the settings/prefs layer: - Quick-switch toggles and reorders were read-modify-write against the async-echoed UI snapshot, so two rapid interactions reverted each other. Writes now go through SettingsPrefs.updateQuickSwitch, an atomic transform over the currently-stored value, via intent-level ViewModel ops; the MIN_ENABLED floor is re-checked inside the transform since the screen's guard reads the stale snapshot. - parseReminderOverrides treated any unparseable stored value as an explicit empty override (no reminder). Only the deliberate 'none' sentinel means that now; garbage drops the entry so the calendar inherits the global default. Partially-valid values salvage their valid minutes. - Replacing an already-active custom font never refreshed typography: the unchanged 'custom' token made AppFontSettings value-equal, so the StateFlow never re-emitted. A per-role import stamp now breaks equality on re-import (missing key = 0, backward compatible). - The FontPicker custom preview resolved the font file unmemoized on every recomposition (disk stat + fresh FontFamily defeating the typeface cache); it's now remembered, keyed on the import stamp. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -103,8 +103,10 @@ class MainActivity : AppCompatActivity() {
|
||||
settings.timeFormat.is24Hour(android.text.format.DateFormat.is24HourFormat(context))
|
||||
}
|
||||
// The user's custom-font choice, resolved to a Material typography
|
||||
// (issue #19). Recomputed only when a token changes; "system for both"
|
||||
// returns the default scale untouched.
|
||||
// (issue #19). Recomputed only when a token — or the custom-font
|
||||
// re-import stamp AppFontSettings carries, so replacing the file
|
||||
// behind an active "custom" token still refreshes — changes;
|
||||
// "system for both" returns the default scale untouched.
|
||||
val fonts by settingsViewModel.fontState.collectAsStateWithLifecycle()
|
||||
val typography = remember(fonts, context) {
|
||||
calendulaTypography(
|
||||
|
||||
@@ -8,6 +8,7 @@ import androidx.datastore.preferences.core.intPreferencesKey
|
||||
import androidx.datastore.preferences.core.longPreferencesKey
|
||||
import androidx.datastore.preferences.core.stringPreferencesKey
|
||||
import de.jeanlucmakiola.calendula.domain.EventFormField
|
||||
import de.jeanlucmakiola.calendula.domain.FontRole
|
||||
import de.jeanlucmakiola.calendula.ui.agenda.AgendaRange
|
||||
import de.jeanlucmakiola.calendula.ui.agenda.parseAgendaRange
|
||||
import de.jeanlucmakiola.calendula.ui.agenda.storageValue
|
||||
@@ -128,6 +129,26 @@ class SettingsPrefs @Inject constructor(
|
||||
store.edit { it[PLAIN_FONT_KEY] = token }
|
||||
}
|
||||
|
||||
/**
|
||||
* A per-role bump counter for the user-loaded custom font. Re-importing
|
||||
* overwrites the same file under the same "custom" token, so the token alone
|
||||
* can't signal the change; this stamp — carried into AppFontSettings — breaks
|
||||
* value equality so the resolved FontFamily (and its previews) refresh. A
|
||||
* missing key is 0 (existing installs); the first import bumps it to 1.
|
||||
*/
|
||||
val brandFontStamp: Flow<Int> = store.data.map { prefs -> prefs[BRAND_FONT_STAMP_KEY] ?: 0 }
|
||||
|
||||
val plainFontStamp: Flow<Int> = store.data.map { prefs -> prefs[PLAIN_FONT_STAMP_KEY] ?: 0 }
|
||||
|
||||
/** Bump [role]'s custom-font stamp after a re-import so equality-keyed caches refresh. */
|
||||
suspend fun bumpCustomFontStamp(role: FontRole) {
|
||||
val key = when (role) {
|
||||
FontRole.BRAND -> BRAND_FONT_STAMP_KEY
|
||||
FontRole.PLAIN -> PLAIN_FONT_STAMP_KEY
|
||||
}
|
||||
store.edit { it[key] = (it[key] ?: 0) + 1 }
|
||||
}
|
||||
|
||||
suspend fun setWeekStart(pref: WeekStartPref) {
|
||||
store.edit { it[WEEK_START_KEY] = pref.storageValue() }
|
||||
}
|
||||
@@ -241,6 +262,19 @@ class SettingsPrefs @Inject constructor(
|
||||
store.edit { it[QUICK_SWITCH_VIEWS_KEY] = serializeQuickSwitch(config) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Atomic read-modify-write of the quick-switch config: [transform] sees the
|
||||
* value currently stored, parsed inside the edit block, not the async-echoed
|
||||
* UI snapshot. Two rapid intents (a toggle then a drag) therefore compose
|
||||
* instead of each re-serialising a stale copy of the other field over it.
|
||||
*/
|
||||
suspend fun updateQuickSwitch(transform: (QuickSwitchConfig) -> QuickSwitchConfig) {
|
||||
store.edit { prefs ->
|
||||
val current = parseQuickSwitch(prefs[QUICK_SWITCH_VIEWS_KEY])
|
||||
prefs[QUICK_SWITCH_VIEWS_KEY] = serializeQuickSwitch(transform(current))
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Navigation-drawer view order (#24). Comma-joined enum names; missing views
|
||||
* are appended in default order and unknown names dropped. Absent key means
|
||||
@@ -525,6 +559,8 @@ class SettingsPrefs @Inject constructor(
|
||||
internal val DYNAMIC_COLOR_KEY = booleanPreferencesKey("dynamic_color")
|
||||
internal val BRAND_FONT_KEY = stringPreferencesKey("brand_font")
|
||||
internal val PLAIN_FONT_KEY = stringPreferencesKey("plain_font")
|
||||
internal val BRAND_FONT_STAMP_KEY = intPreferencesKey("brand_font_stamp")
|
||||
internal val PLAIN_FONT_STAMP_KEY = intPreferencesKey("plain_font_stamp")
|
||||
internal val WEEK_START_KEY = stringPreferencesKey("week_start")
|
||||
internal val AGENDA_SCREEN_RANGE_KEY = stringPreferencesKey("agenda_screen_range")
|
||||
internal val AGENDA_WIDGET_RANGE_KEY = stringPreferencesKey("agenda_widget_range")
|
||||
@@ -675,7 +711,15 @@ private fun parseReminderOverrides(stored: String?): Map<Long, List<Int>> {
|
||||
return stored.split(ENTRY_SEP).mapNotNull { entry ->
|
||||
val parts = entry.split(KEY_VALUE_SEP).takeIf { it.size == 2 } ?: return@mapNotNull null
|
||||
val id = parts[0].toLongOrNull() ?: return@mapNotNull null
|
||||
id to parts[1].toReminderList()
|
||||
// Only the deliberate "none" sentinel means an explicit no-reminder
|
||||
// override (empty list); a non-sentinel value that parses to no valid
|
||||
// minutes is garbage and drops the entry, so the calendar inherits the
|
||||
// global default rather than silently reading as "no reminder".
|
||||
when (val value = parts[1]) {
|
||||
NONE -> id to emptyList()
|
||||
else -> value.toReminderList().takeIf { it.isNotEmpty() }?.let { id to it }
|
||||
?: return@mapNotNull null
|
||||
}
|
||||
}.toMap()
|
||||
}
|
||||
|
||||
|
||||
@@ -671,6 +671,7 @@ private fun AppearanceScreen(
|
||||
title = stringResource(R.string.settings_font_headings),
|
||||
role = FontRole.BRAND,
|
||||
selected = fonts.brand,
|
||||
stamp = fonts.brandStamp,
|
||||
onSelect = { viewModel.setFont(FontRole.BRAND, it) },
|
||||
onImport = { viewModel.importCustomFont(FontRole.BRAND, it) },
|
||||
onDismiss = { showBrandFont = false },
|
||||
@@ -681,6 +682,7 @@ private fun AppearanceScreen(
|
||||
title = stringResource(R.string.settings_font_body),
|
||||
role = FontRole.PLAIN,
|
||||
selected = fonts.plain,
|
||||
stamp = fonts.plainStamp,
|
||||
onSelect = { viewModel.setFont(FontRole.PLAIN, it) },
|
||||
onImport = { viewModel.importCustomFont(FontRole.PLAIN, it) },
|
||||
onDismiss = { showPlainFont = false },
|
||||
@@ -773,7 +775,7 @@ private fun ViewsScreen(
|
||||
ReorderableColumn(
|
||||
items = config.order,
|
||||
keyOf = { it },
|
||||
onReorder = { viewModel.setQuickSwitchConfig(config.copy(order = it)) },
|
||||
onReorder = { viewModel.setQuickSwitchOrder(it) },
|
||||
) { view, position, dragHandle, isDragging ->
|
||||
val checked = view in config.enabled
|
||||
ViewRow(
|
||||
@@ -787,10 +789,7 @@ private fun ViewsScreen(
|
||||
checked = checked,
|
||||
// Keep the last two on: with fewer, the pill can't switch.
|
||||
enabled = !checked || canDisable,
|
||||
onCheckedChange = { on ->
|
||||
val enabled = if (on) config.enabled + view else config.enabled - view
|
||||
viewModel.setQuickSwitchConfig(config.copy(enabled = enabled))
|
||||
},
|
||||
onCheckedChange = { on -> viewModel.setQuickSwitchViewEnabled(view, on) },
|
||||
)
|
||||
},
|
||||
)
|
||||
@@ -1431,6 +1430,7 @@ private fun FontPicker(
|
||||
title: String,
|
||||
role: FontRole,
|
||||
selected: String,
|
||||
stamp: Int,
|
||||
onSelect: (String) -> Unit,
|
||||
onImport: (Uri) -> Unit,
|
||||
onDismiss: () -> Unit,
|
||||
@@ -1448,6 +1448,12 @@ private fun FontPicker(
|
||||
// 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.
|
||||
val customPreview = remember(role, isCustom, stamp) {
|
||||
if (isCustom) resolveFontFamily(FONT_CUSTOM_TOKEN, role, context) else null
|
||||
}
|
||||
|
||||
FullScreenPicker(title = title, onDismiss = onDismiss) {
|
||||
FontOptionRow(
|
||||
@@ -1479,7 +1485,7 @@ private fun FontPicker(
|
||||
stringResource(R.string.settings_font_choose_file)
|
||||
},
|
||||
// A loaded font previews in its own face; otherwise show an upload cue.
|
||||
preview = if (isCustom) resolveFontFamily(FONT_CUSTOM_TOKEN, role, context) else null,
|
||||
preview = customPreview,
|
||||
leadingIcon = if (isCustom) null else Icons.Default.UploadFile,
|
||||
selected = isCustom,
|
||||
position = positionOf(rowCount - 1, rowCount),
|
||||
|
||||
@@ -159,8 +159,10 @@ class SettingsViewModel @Inject constructor(
|
||||
val fontState: StateFlow<AppFontSettings> = combine(
|
||||
prefs.brandFont,
|
||||
prefs.plainFont,
|
||||
) { brand, plain ->
|
||||
AppFontSettings(brand = brand, plain = plain)
|
||||
prefs.brandFontStamp,
|
||||
prefs.plainFontStamp,
|
||||
) { brand, plain, brandStamp, plainStamp ->
|
||||
AppFontSettings(brand = brand, plain = plain, brandStamp = brandStamp, plainStamp = plainStamp)
|
||||
}.stateIn(
|
||||
scope = viewModelScope,
|
||||
started = SharingStarted.WhileSubscribed(5_000L),
|
||||
@@ -239,6 +241,10 @@ 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.
|
||||
prefs.bumpCustomFontStamp(role)
|
||||
setFont(role, FONT_CUSTOM_TOKEN)
|
||||
} else {
|
||||
_fontImportFailed.emit(Unit)
|
||||
@@ -319,8 +325,26 @@ class SettingsViewModel @Inject constructor(
|
||||
viewModelScope.launch { prefs.setFormFieldDefault(field, enabled) }
|
||||
}
|
||||
|
||||
fun setQuickSwitchConfig(config: QuickSwitchConfig) {
|
||||
viewModelScope.launch { prefs.setQuickSwitchConfig(config) }
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
fun setQuickSwitchViewEnabled(view: CalendarView, enabled: Boolean) {
|
||||
viewModelScope.launch {
|
||||
prefs.updateQuickSwitch { config ->
|
||||
val next = if (enabled) config.enabled + view else config.enabled - view
|
||||
if (next.size < QuickSwitchConfig.MIN_ENABLED) config else config.copy(enabled = next)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Reorder the quick-switch views (atomic read-modify-write; keeps the enabled set). */
|
||||
fun setQuickSwitchOrder(order: List<CalendarView>) {
|
||||
viewModelScope.launch { prefs.updateQuickSwitch { it.copy(order = order) } }
|
||||
}
|
||||
|
||||
fun setDrawerViewOrder(order: List<CalendarView>) {
|
||||
|
||||
@@ -71,6 +71,11 @@ private fun weightAxis(weight: Int) = FontVariation.Settings(FontVariation.weigh
|
||||
data class AppFontSettings(
|
||||
val brand: String = FONT_SYSTEM_TOKEN,
|
||||
val plain: String = FONT_SYSTEM_TOKEN,
|
||||
// Per-role custom-font re-import stamp: bumped when the "custom" file is
|
||||
// replaced, so a same-token re-import still breaks equality and refreshes the
|
||||
// resolved family. 0 for existing installs / no import yet (issue #19).
|
||||
val brandStamp: Int = 0,
|
||||
val plainStamp: Int = 0,
|
||||
)
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user