fix: Restoring backup from file doesn't respect 'swipe actions' setting#10893
fix: Restoring backup from file doesn't respect 'swipe actions' setting#10893shamim-emon wants to merge 15 commits intothunderbird:mainfrom
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
wmontwe
left a comment
There was a problem hiding this comment.
Using the PreferenceChangeBroker solved the issue, but it causes side effects.
This is using a simple broadcaster that notifies all subscribers and therefore could lead to a broadcast storm as now every settings related manager reacts to any change. Just add a log to every receive and then change the "animation" setting you will see 11 managers updating.
While the individual reloads are relatively small, they quickly add up causing simultaneous updates across multiple managers that end up aggregated as GeneralSettings with multiple updates to consumers in the app.
Some ideas:
These are rough ideas and maybe not doable or adding to much complexity.
- Keep the change broker, but scope the emissions (adds complexity)
enum class PreferenceScope { ALL, DISPLAY, PRIVACY, NETWORK, DEBUGGING }
interface PreferenceChangePublisher {
fun publish(scope: PreferenceScope = PreferenceScope.ALL)
}
fun interface PreferenceChangeSubscriber {
fun receive(scope: PreferenceScope)
}
// In a manager:
override fun receive(scope: PreferenceScope) {
if (scope == PreferenceScope.ALL || scope == PreferenceScope.PRIVACY) {
configState.update { loadConfig() }
}
}- Use key-based notifications
fun interface PreferenceChangeSubscriber {
fun receive(changedKey: String)
}
// In a manager:
override fun receive(changedKey: String) {
if (myOwnedKeys.contains(changedKey)) {
configState.update { loadConfig() }
}
}- Use a reactive storage
class DefaultNotificationPreferenceManager(...) {
val configFlow = combine(
storage.observeBoolean(KEY_QUIET_TIME),
storage.observeString(KEY_NOTIFICATION_VIBRATE)
) { quietTime, vibrate ->
NotificationSettings(quietTime, vibrate)
}.stateIn(scope, Eagerly, initialValue)
}But our storage is not reactive yet.
Other ideas are welcome
1f2dc65 to
fb88e8c
Compare
fb88e8c to
76889ba
Compare
76889ba to
618f321
Compare
618f321 to
ecde527
Compare
16ca4ae to
e82ba21
Compare
|
✅ Validation Passed: All report and feature-flag labels are correctly set. |
…renceChangeBroker and stop implementing PreferenceChangeSubscriber.
…preference change notifications.
…ttingsPreferenceManager
…reference managers.
…SettingsPreferenceManager
…ingsPreferenceManager
…nPreferenceManager
…ingsPreferenceManager
…ues to group preference keys
…PreferenceManager
… to ensure correct behavior of the updated PreferenceChangeSubscriber
e82ba21 to
0f1401d
Compare
🐛 Problem
GeneralSettingsWriteruses legacy preference system to store GeneralSettings values.🛠️ Solution
PreferenceChangeBroker.GeneralSettingsWriter.