Fixes #367 - Refactor AppSettings usage patterns

ElementX.xcodeproj/project.pbxproj

@@ -443,6 +443,7 @@
 		D876EC0FED3B6D46C806912A /* AvatarSize.swift in Sources */ = {isa = PBXBuildFile; fileRef = E24B88AD3D1599E8CB1376E0 /* AvatarSize.swift */; };
 		D8CFF02C2730EE5BC4F17ABF /* ElementToggleStyle.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0960A7F5C1B0B6679BDF26F9 /* ElementToggleStyle.swift */; };
 		D9F80CE61BF8FF627FDB0543 /* LoadableImage.swift in Sources */ = {isa = PBXBuildFile; fileRef = C352359663A0E52BA20761EE /* LoadableImage.swift */; };
+		DA4620936DA42CBE2524E1AE /* UserSettingPropertyWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 68F8B8C529BF036E804B165E /* UserSettingPropertyWrapper.swift */; };
 		DBAA69CC2CE4D44BC8E20105 /* SettingsScreenModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = 548E7D356609ACD33AE7643E /* SettingsScreenModels.swift */; };
 		DC68E866D6E664B0D2B06E74 /* MockImageCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = AC1DA29A5A041CC0BACA7CB0 /* MockImageCache.swift */; };
 		DD9B70DE54B24E0694A35D8A /* Strings+Untranslated.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1A18F6CE4D694D21E4EA9B25 /* Strings+Untranslated.swift */; };
@@ -777,6 +778,7 @@
 		6654859746B0BE9611459391 /* cs */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = cs; path = cs.lproj/Localizable.stringsdict; sourceTree = "<group>"; };
 		667DD3A9D932D7D9EB380CAA /* sk */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = sk; path = sk.lproj/Localizable.stringsdict; sourceTree = "<group>"; };
 		66F2402D738694F98729A441 /* RoomTimelineProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomTimelineProvider.swift; sourceTree = "<group>"; };
+		68F8B8C529BF036E804B165E /* UserSettingPropertyWrapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserSettingPropertyWrapper.swift; sourceTree = "<group>"; };
 		6920A4869821BF72FFC58842 /* MockMediaProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockMediaProvider.swift; sourceTree = "<group>"; };
 		69219A908D7C22E6EE6689AE /* UserNotificationCenterSpy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserNotificationCenterSpy.swift; sourceTree = "<group>"; };
 		6A1AAC8EB2992918D01874AC /* rue */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = rue; path = rue.lproj/Localizable.strings; sourceTree = "<group>"; };
@@ -2264,6 +2266,7 @@
 				53482ECA4B6633961EC224F5 /* ScrollViewAdapter.swift */,
 				BB3073CCD77D906B330BC1D6 /* Tests.swift */,
 				1F2529D434C750ED78ADF1ED /* UserAgentBuilder.swift */,
+				68F8B8C529BF036E804B165E /* UserSettingPropertyWrapper.swift */,
 				44BBB96FAA2F0D53C507396B /* Extensions */,
 				8F9A844EB44B6AD7CA18FD96 /* HTMLParsing */,
 				06501F0E978B2D5C92771DC7 /* Logging */,
@@ -3333,6 +3336,7 @@
 				978BB24F2A5D31EE59EEC249 /* UserSessionProtocol.swift in Sources */,
 				7E91BAC17963ED41208F489B /* UserSessionStore.swift in Sources */,
 				AC69B6DF15FC451AB2945036 /* UserSessionStoreProtocol.swift in Sources */,
+				DA4620936DA42CBE2524E1AE /* UserSettingPropertyWrapper.swift in Sources */,
 				F07D88421A9BC4D03D4A5055 /* VideoRoomTimelineItem.swift in Sources */,
 				64F43D7390DA2A0AFD6BA911 /* VideoRoomTimelineView.swift in Sources */,
 				6FC10A00D268FCD48B631E37 /* ViewFrameReader.swift in Sources */,

ElementX/Sources/Application/AppSettings.swift

@@ -52,7 +52,7 @@ final class AppSettings: ObservableObject {
     /// The last known version of the app that was launched on this device, which is
     /// used to detect when migrations should be run. When `nil` the app may have been
     /// deleted between runs so should clear data in the shared container and keychain.
-    @AppStorage(UserDefaultsKeys.lastVersionLaunched.rawValue, store: store)
+    @UserSetting(key: UserDefaultsKeys.lastVersionLaunched.rawValue, defaultValue: nil, storage: store)
     var lastVersionLaunched: String?
     
     /// The default homeserver address used. This is intentionally a string without a scheme
@@ -114,27 +114,27 @@ final class AppSettings: ObservableObject {
     }
     
     /// `true` when the user has opted in to send analytics.
-    @AppStorage(UserDefaultsKeys.enableAnalytics.rawValue, store: store)
+    @UserSetting(key: UserDefaultsKeys.enableAnalytics.rawValue, defaultValue: false, storage: store)
-    var enableAnalytics = false
+    var enableAnalytics
     
     /// Indicates if the device has already called identify for this session to PostHog.
     /// This is separate to `enableAnalytics` as logging out leaves analytics
     /// enabled, but requires the next account to be identified separately.
-    @AppStorage(UserDefaultsKeys.isIdentifiedForAnalytics.rawValue, store: store)
+    @UserSetting(key: UserDefaultsKeys.isIdentifiedForAnalytics.rawValue, defaultValue: false, storage: store)
-    var isIdentifiedForAnalytics = false
+    var isIdentifiedForAnalytics
     
     // MARK: - Room Screen
     
-    @AppStorage(UserDefaultsKeys.timelineStyle.rawValue, store: store)
+    @UserSettingRawRepresentable(key: UserDefaultsKeys.timelineStyle.rawValue, defaultValue: TimelineStyle.bubbles, storage: store)
-    var timelineStyle = TimelineStyle.bubbles
+    var timelineStyle
 
     // MARK: - Notifications
 
-    @AppStorage(UserDefaultsKeys.enableInAppNotifications.rawValue, store: store)
+    @UserSetting(key: UserDefaultsKeys.timelineStyle.rawValue, defaultValue: true, storage: store)
-    var enableInAppNotifications = true
+    var enableInAppNotifications
 
     /// Tag describing which set of device specific rules a pusher executes.
-    @AppStorage(UserDefaultsKeys.pusherProfileTag.rawValue, store: store)
+    @UserSetting(key: UserDefaultsKeys.pusherProfileTag.rawValue, defaultValue: nil, storage: store)
     var pusherProfileTag: String?
         
     // MARK: - Other

ElementX/Sources/Other/UserSettingPropertyWrapper.swift

@@ -0,0 +1,127 @@
+//
+// Copyright 2022 New Vector Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+// Taken from https://www.swiftbysundell.com/articles/property-wrappers-in-swift/
+
+import Combine
+import Foundation
+
+/// Property wrapper that allows transparent access to user defaults and exposes
+/// a combine publisher for listening to value changes
+///
+/// Please use `UserSettingRawRepresentable` for storing RawRepresentable values
+@propertyWrapper
+struct UserSetting<Value: Equatable> {
+    private let key: String
+    private let defaultValue: Value
+    private let storage: UserDefaults
+    private let publisher: CurrentValueSubject<Value, Never>
+    
+    init(key: String, defaultValue: Value, storage: UserDefaults = .standard) {
+        self.key = key
+        self.defaultValue = defaultValue
+        self.storage = storage
+        
+        let value = storage.value(forKey: key) as? Value ?? defaultValue
+        publisher = CurrentValueSubject<Value, Never>(value)
+    }
+    
+    var wrappedValue: Value {
+        get {
+            let value = storage.value(forKey: key) as? Value
+            return value ?? defaultValue
+        }
+        set {
+            if let optional = newValue as? AnyOptional, optional.isNil {
+                storage.removeObject(forKey: key)
+                publisher.send(defaultValue)
+            } else {
+                storage.setValue(newValue, forKey: key)
+                publisher.send(newValue)
+            }
+        }
+    }
+    
+    var projectedValue: AnyPublisher<Value, Never> {
+        publisher.removeDuplicates(by: { $0 == $1 }).eraseToAnyPublisher()
+    }
+}
+
+extension UserSetting where Value: ExpressibleByNilLiteral {
+    init(key: String, storage: UserDefaults = .standard) {
+        self.init(key: key, defaultValue: nil, storage: storage)
+    }
+}
+
+/// Property wrapper that allows transparent access to user defaults for RawRepresentable types
+/// and exposes a combine publisher for listening to value changes
+///
+/// Tried extending UserSetting with RawRepresentable conformance but in that case the non-restricted
+/// method takes precedence. Decided to go with with the simple solution instead of fighting the system
+@propertyWrapper
+struct UserSettingRawRepresentable<Value: RawRepresentable & Equatable> {
+    private let key: String
+    private let defaultValue: Value
+    private let storage: UserDefaults
+    private let publisher: CurrentValueSubject<Value, Never>
+    
+    init(key: String, defaultValue: Value, storage: UserDefaults = .standard) {
+        self.key = key
+        self.defaultValue = defaultValue
+        self.storage = storage
+        
+        let value = (storage.value(forKey: key) as? Value.RawValue).flatMap { Value(rawValue: $0) } ?? defaultValue
+        publisher = CurrentValueSubject<Value, Never>(value)
+    }
+    
+    var wrappedValue: Value {
+        get {
+            guard let value = storage.value(forKey: key) as? Value.RawValue else {
+                return defaultValue
+            }
+            
+            return Value(rawValue: value) ?? defaultValue
+        }
+        set {
+            if let optional = newValue as? AnyOptional, optional.isNil {
+                storage.removeObject(forKey: key)
+                publisher.send(newValue)
+            } else {
+                storage.setValue(newValue.rawValue, forKey: key)
+                publisher.send(newValue)
+            }
+        }
+    }
+    
+    var projectedValue: AnyPublisher<Value, Never> {
+        publisher.removeDuplicates(by: { $0 == $1 }).eraseToAnyPublisher()
+    }
+}
+
+extension UserSettingRawRepresentable where Value: ExpressibleByNilLiteral {
+    init(key: String, storage: UserDefaults = .standard) {
+        self.init(key: key, defaultValue: nil, storage: storage)
+    }
+}
+
+// Casting to AnyOptional will fail for any types that are not Optional (below)
+private protocol AnyOptional {
+    var isNil: Bool { get }
+}
+
+extension Optional: AnyOptional {
+    var isNil: Bool { self == nil }
+}

ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift

@@ -64,6 +64,8 @@ struct RoomScreenViewState: BindableState {
     var canBackPaginate = true
     var isBackPaginating = false
     var showLoading = false
+    var timelineStyle: TimelineStyle
+    
     var bindings: RoomScreenViewStateBindings
     
     var contextMenuActionProvider: (@MainActor (_ itemId: String) -> TimelineItemContextMenuActions?)?

ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift

@@ -40,6 +40,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
         super.init(initialViewState: RoomScreenViewState(roomId: timelineController.roomID,
                                                          roomTitle: roomName ?? "Unknown room 💥",
                                                          roomAvatarURL: roomAvatarUrl,
+                                                         timelineStyle: ServiceLocator.shared.settings.timelineStyle,
                                                          bindings: .init(composerText: "", composerFocused: false)),
                    imageProvider: mediaProvider)
         
@@ -78,6 +79,11 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
             return self.contextMenuActionsForItemId(itemId)
         }
         
+        ServiceLocator.shared.settings.$timelineStyle.sink { [weak self] timelineStyle in
+            self?.state.timelineStyle = timelineStyle
+        }
+        .store(in: &cancellables)
+        
         buildTimelineViews()
     }
     

ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift

@@ -17,7 +17,6 @@
 import SwiftUI
 
 struct RoomScreen: View {
-    @ObservedObject private var settings = ServiceLocator.shared.settings
     @ObservedObject var context: RoomScreenViewModel.Context
     @State private var showReactionsMenuForItemId = ""
     
@@ -45,7 +44,7 @@ struct RoomScreen: View {
         TimelineView()
             .id(context.viewState.roomId)
             .environmentObject(context)
-            .timelineStyle(settings.timelineStyle)
+            .timelineStyle(context.viewState.timelineStyle)
             .overlay(alignment: .bottomTrailing) { scrollToBottomButton }
     }
     

ElementX/Sources/Screens/Settings/SettingsScreenModels.swift

@@ -35,7 +35,7 @@ struct SettingsScreenViewState: BindableState {
 }
 
 struct SettingsScreenViewStateBindings {
-    var enableAnalytics = ServiceLocator.shared.settings.enableAnalytics
+    var timelineStyle: TimelineStyle
 }
 
 enum SettingsScreenViewAction {
@@ -44,4 +44,5 @@ enum SettingsScreenViewAction {
     case reportBug
     case sessionVerification
     case logout
+    case changedTimelineStyle
 }

ElementX/Sources/Screens/Settings/SettingsScreenViewModel.swift

@@ -14,6 +14,7 @@
 // limitations under the License.
 //
 
+import Combine
 import SwiftUI
 
 typealias SettingsScreenViewModelType = StateStoreViewModel<SettingsScreenViewState, SettingsScreenViewAction>
@@ -25,13 +26,15 @@ class SettingsScreenViewModel: SettingsScreenViewModelType, SettingsScreenViewMo
 
     init(withUserSession userSession: UserSessionProtocol) {
         self.userSession = userSession
-        let bindings = SettingsScreenViewStateBindings()
+        let bindings = SettingsScreenViewStateBindings(timelineStyle: ServiceLocator.shared.settings.timelineStyle)
         super.init(initialViewState: .init(bindings: bindings,
                                            deviceID: userSession.deviceId,
                                            userID: userSession.userID,
                                            showSessionVerificationSection: !(userSession.sessionVerificationController?.isVerified ?? false)),
                    imageProvider: userSession.mediaProvider)
         
+        listenToSettingsChange(publisher: ServiceLocator.shared.settings.$timelineStyle, keyPath: \.timelineStyle)
+        
         Task {
             if case let .success(userAvatarURL) = await userSession.clientProxy.loadUserAvatarURL() {
                 state.userAvatarURL = userAvatarURL
@@ -71,6 +74,20 @@ class SettingsScreenViewModel: SettingsScreenViewModelType, SettingsScreenViewMo
             callback?(.logout)
         case .sessionVerification:
             callback?(.sessionVerification)
+        case .changedTimelineStyle:
+            ServiceLocator.shared.settings.timelineStyle = state.bindings.timelineStyle
         }
     }
+    
+    private func listenToSettingsChange<Value>(publisher: AnyPublisher<Value, Never>,
+                                               keyPath: WritableKeyPath<SettingsScreenViewStateBindings, Value>) where Value: Equatable {
+        publisher.sink { [weak self] newValue in
+            guard newValue != self?.state.bindings[keyPath: keyPath] else {
+                return
+            }
+            
+            self?.state.bindings[keyPath: keyPath] = newValue
+        }
+        .store(in: &cancellables)
+    }
 }

ElementX/Sources/Screens/Settings/View/SettingsScreen.swift

@@ -19,7 +19,6 @@ import SwiftUI
 struct SettingsScreen: View {
     @State private var showingLogoutConfirmation = false
     @Environment(\.colorScheme) private var colorScheme
-    @ObservedObject private var settings = ServiceLocator.shared.settings
 
     @ScaledMetric private var avatarSize = AvatarSize.user(on: .settings).value
     @ScaledMetric private var menuIconSize = 30.0
@@ -104,13 +103,16 @@ struct SettingsScreen: View {
         Section {
             SettingsPickerRow(title: ElementL10n.settingsTimelineStyle,
                               image: Image(systemName: "rectangle.grid.1x2"),
-                              selection: $settings.timelineStyle) {
+                              selection: $context.timelineStyle) {
                 ForEach(TimelineStyle.allCases, id: \.self) { style in
-                    Text(style.description)
+                    Text(style.name)
                         .tag(style)
                 }
             }
             .accessibilityIdentifier("timelineStylePicker")
+            .onChange(of: context.timelineStyle) { _ in
+                context.send(viewAction: .changedTimelineStyle)
+            }
             
             SettingsDefaultRow(title: ElementL10n.sendBugReport,
                                image: Image(systemName: "questionmark.circle")) {
@@ -166,8 +168,8 @@ struct SettingsScreen: View {
     }
 }
 
-extension TimelineStyle: CustomStringConvertible {
+private extension TimelineStyle {
-    var description: String {
+    var name: String {
         switch self {
         case .plain:
             return ElementL10n.roomTimelineStylePlainLongDescription