#92 Fix PR remarks

ElementX/Resources/Localizations/en.lproj/Untranslated.strings

@@ -3,7 +3,5 @@
 "screenshot_detected_message" = "Would you like to submit a bug report?";
 
 "settings_timeline_style" = "Timeline Style";
-"room_timeline_style_plain_short_description" = "Plain";
-"room_timeline_style_bubbled_short_description" = "Bubbles";
 "room_timeline_style_plain_long_description" = "Plain Timeline";
 "room_timeline_style_bubbled_long_description" = "Bubbled Timeline";

ElementX/Sources/Generated/Strings+Untranslated.swift

@@ -12,12 +12,8 @@ import Foundation
 extension ElementL10n {
   /// Bubbled Timeline
   public static let roomTimelineStyleBubbledLongDescription = ElementL10n.tr("Untranslated", "room_timeline_style_bubbled_long_description")
-  /// Bubbles
-  public static let roomTimelineStyleBubbledShortDescription = ElementL10n.tr("Untranslated", "room_timeline_style_bubbled_short_description")
   /// Plain Timeline
   public static let roomTimelineStylePlainLongDescription = ElementL10n.tr("Untranslated", "room_timeline_style_plain_long_description")
-  /// Plain
-  public static let roomTimelineStylePlainShortDescription = ElementL10n.tr("Untranslated", "room_timeline_style_plain_short_description")
   /// Would you like to submit a bug report?
   public static let screenshotDetectedMessage = ElementL10n.tr("Untranslated", "screenshot_detected_message")
   /// You took a screenshot

ElementX/Sources/Other/ElementSettings.swift

@@ -10,7 +10,7 @@ import Foundation
 import SwiftUI
 
 /// Store Element specific app settings.
-final class ElementSettings {
+final class ElementSettings: ObservableObject {
 
     // MARK: - Constants
 
@@ -31,17 +31,6 @@ final class ElementSettings {
 
     // MARK: -
 
-    @AppStorage(wrappedValue: BuildSettings.defaultRoomTimelineStyle.rawValue,
-                UserDefaultsKeys.timelineStyle.rawValue,
-                store: store)
-    private var timelineStyleRaw
-
-    /// Computed timeline style
-    var timelineStyle: TimelineStyle {
-        get {
-            TimelineStyle(rawValue: timelineStyleRaw) ?? BuildSettings.defaultRoomTimelineStyle
-        } set {
-            timelineStyleRaw = newValue.rawValue
-        }
-    }
+    @AppStorage(UserDefaultsKeys.timelineStyle.rawValue, store: store)
+    var timelineStyle = BuildSettings.defaultRoomTimelineStyle
 }

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

@@ -117,7 +117,7 @@ struct TimelineItemBubbledStylerView_Previews: PreviewProvider {
                 RoomTimelineViewFactory().buildTimelineViewFor(timelineItem: item)
             }
         }
-        .timelineStyler(.bubbled)
+        .timelineStyle(.bubbled)
         .padding(.horizontal, 8)
         .previewLayout(.sizeThatFits)
     }

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

@@ -58,7 +58,7 @@ struct TimelineItemPlainStylerView_Previews: PreviewProvider {
                 RoomTimelineViewFactory().buildTimelineViewFor(timelineItem: item)
             }
         }
-        .timelineStyler(.plain)
+        .timelineStyle(.plain)
         .padding(.horizontal, 8)
         .previewLayout(.sizeThatFits)
     }

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

@@ -7,8 +7,49 @@
 //
 
 import Foundation
+import SwiftUI
 
 enum TimelineStyle: String, CaseIterable {
     case plain
     case bubbled
+
+    /// List row insets for a timeline
+    var listRowInsets: EdgeInsets {
+        switch self {
+        case .plain:
+            return EdgeInsets(top: 4, leading: 20, bottom: 4, trailing: 20)
+        case .bubbled:
+            return EdgeInsets(top: 1, leading: 8, bottom: 1, trailing: 8)
+        }
+    }
+}
+
+extension TimelineStyle: CustomStringConvertible {
+    var description: String {
+        switch self {
+        case .plain:
+            return ElementL10n.roomTimelineStylePlainLongDescription
+        case .bubbled:
+            return ElementL10n.roomTimelineStyleBubbledLongDescription
+        }
+    }
+}
+
+// MARK: - Environment
+
+private struct TimelineStyleKey: EnvironmentKey {
+    static let defaultValue = BuildSettings.defaultRoomTimelineStyle
+}
+
+extension EnvironmentValues {
+    var timelineStyle: TimelineStyle {
+        get { self[TimelineStyleKey.self] }
+        set { self[TimelineStyleKey.self] = newValue }
+    }
+}
+
+extension View {
+    func timelineStyle(_ style: TimelineStyle) -> some View {
+        environment(\.timelineStyle, style)
+    }
 }

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

@@ -11,84 +11,18 @@ import SwiftUI
 
 // MARK: - TimelineStyler
 
-class TimelineStyler: ObservableObject, Identifiable {
-    var style: TimelineStyle
+struct TimelineStyler<Content: View>: View {
+    @Environment(\.timelineStyle) private var style
 
-    fileprivate init(style: TimelineStyle) {
-        self.style = style
-    }
-
-    static let plain = TimelineStyler(style: .plain)
-    static let bubbled = TimelineStyler(style: .bubbled)
+    let timelineItem: EventBasedTimelineItemProtocol
+    @ViewBuilder let content: () -> Content
 
-    var shortDescription: String {
-        switch style {
-        case .plain:
-            return ElementL10n.roomTimelineStylePlainShortDescription
-        case .bubbled:
-            return ElementL10n.roomTimelineStyleBubbledShortDescription
-        }
-    }
-
-    @ViewBuilder
-    /// Builds a styled view fron given timeline item and content. Can add a sender info if configured.
-    /// - Parameters:
-    ///   - timelineItem: timeline item
-    ///   - content: content
-    /// - Returns: Styled content view
-    func styled<Content: View>(timelineItem: EventBasedTimelineItemProtocol,
-                               @ViewBuilder content: @escaping () -> Content) -> some View {
+    var body: some View {
         switch style {
         case .plain:
             TimelineItemPlainStylerView(timelineItem: timelineItem, content: content)
         case .bubbled:
             TimelineItemBubbledStylerView(timelineItem: timelineItem, content: content)
         }
     }
-
-    /// List row insets for a timeline
-    var listRowInsets: EdgeInsets {
-        switch style {
-        case .plain:
-            return EdgeInsets(top: 4, leading: 20, bottom: 4, trailing: 20)
-        case .bubbled:
-            return EdgeInsets(top: 1, leading: 8, bottom: 1, trailing: 8)
-        }
-    }
-}
-
-extension TimelineStyler: CustomStringConvertible {
-    var description: String {
-        switch style {
-        case .plain:
-            return ElementL10n.roomTimelineStylePlainLongDescription
-        case .bubbled:
-            return ElementL10n.roomTimelineStyleBubbledLongDescription
-        }
-    }
-}
-
-extension TimelineStyler: CaseIterable {
-    static var allCases: [TimelineStyler] {
-        TimelineStyle.allCases.map { TimelineStyler(style: $0) }
-    }
-}
-
-// MARK: - Environment
-
-private struct TimelineStylerKey: EnvironmentKey {
-    static let defaultValue = TimelineStyler(style: ElementSettings.shared.timelineStyle)
-}
-
-extension EnvironmentValues {
-    var timelineStyler: TimelineStyler {
-        get { self[TimelineStylerKey.self] }
-        set { self[TimelineStylerKey.self] = newValue }
-    }
-}
-
-extension View {
-    func timelineStyler(_ styler: TimelineStyler) -> some View {
-        environment(\.timelineStyler, styler)
-    }
 }

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

@@ -11,11 +11,9 @@ import SwiftUI
 
 struct EmoteRoomTimelineView: View {
     let timelineItem: EmoteRoomTimelineItem
-
-    @Environment(\.timelineStyler) private var timelineStyler
 
     var body: some View {
-        timelineStyler.styled(timelineItem: timelineItem) {
+        TimelineStyler(timelineItem: timelineItem) {
             HStack(alignment: .top) {
                 Image(systemName: "face.dashed").padding(.top, 1.0)
                 if let attributedComponents = timelineItem.attributedComponents {

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

@@ -11,12 +11,10 @@ import SwiftUI
 
 struct ImageRoomTimelineView: View {
     let timelineItem: ImageRoomTimelineItem
-
-    @Environment(\.timelineStyler) private var timelineStyler
 
     var body: some View {
         if timelineItem.image != nil || timelineItem.blurhash != nil { // Fixes view heights after loading finishes
-            timelineStyler.styled(timelineItem: timelineItem) {
+            TimelineStyler(timelineItem: timelineItem) {
                 if let image = timelineItem.image {
                     if let aspectRatio = timelineItem.aspectRatio {
                         Image(uiImage: image)
@@ -39,7 +37,7 @@ struct ImageRoomTimelineView: View {
             .animation(.default, value: timelineItem.image)
             .frame(maxHeight: 1000.0)
         } else {
-            timelineStyler.styled(timelineItem: timelineItem) {
+            TimelineStyler(timelineItem: timelineItem) {
                 HStack {
                     Spacer()
                     ProgressView("Loading")

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

@@ -11,11 +11,9 @@ import SwiftUI
 
 struct NoticeRoomTimelineView: View {
     let timelineItem: NoticeRoomTimelineItem
-
-    @Environment(\.timelineStyler) private var timelineStyler
 
     var body: some View {
-        timelineStyler.styled(timelineItem: timelineItem) {
+        TimelineStyler(timelineItem: timelineItem) {
             HStack(alignment: .top) {
                 Image(systemName: "exclamationmark.bubble").padding(.top, 2.0)
                 if let attributedComponents = timelineItem.attributedComponents {

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

@@ -11,11 +11,9 @@ import SwiftUI
 
 struct TextRoomTimelineView: View {
     let timelineItem: TextRoomTimelineItem
-
-    @Environment(\.timelineStyler) private var timelineStyler
 
     var body: some View {
-        timelineStyler.styled(timelineItem: timelineItem) {
+        TimelineStyler(timelineItem: timelineItem) {
             if let attributedComponents = timelineItem.attributedComponents {
                 FormattedBodyText(attributedComponents: attributedComponents)
             } else {
@@ -54,14 +52,14 @@ struct TextRoomTimelineView_Previews: PreviewProvider {
                                                         shouldShowSenderDetails: true,
                                                         isOutgoing: false,
                                                         senderId: "Bob"))
-            .timelineStyler(.plain)
+            .timelineStyle(.plain)
 
             TextRoomTimelineView(timelineItem: itemWith(text: "Some other text",
                                                         timestamp: "Later",
                                                         shouldShowSenderDetails: true,
                                                         isOutgoing: true,
                                                         senderId: "Anne"))
-            .timelineStyler(.plain)
+            .timelineStyle(.plain)
         }
         .padding(.horizontal, 8)
     }

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

@@ -15,8 +15,7 @@ struct TimelineItemList: View {
     @State private var tableViewObserver: ListTableViewAdapter = ListTableViewAdapter()
     @State private var timelineItems: [RoomTimelineViewProvider] = []
     @State private var hasPendingChanges = false
-
-    @Environment(\.timelineStyler) private var timelineStyler
+    @ObservedObject private var settings = ElementSettings.shared
 
     @ObservedObject var context: RoomScreenViewModel.Context
 
@@ -44,7 +43,7 @@ struct TimelineItemList: View {
                         })
                         .listRowBackground(Color.clear)
                         .listRowSeparator(.hidden)
-                        .listRowInsets(timelineStyler.listRowInsets)
+                        .listRowInsets(settings.timelineStyle.listRowInsets)
                         .onAppear {
                             context.send(viewAction: .itemAppeared(id: timelineItem.id))
                         }
@@ -58,6 +57,7 @@ struct TimelineItemList: View {
                 }
             }
             .listStyle(.plain)
+            .timelineStyle(settings.timelineStyle)
             .environment(\.defaultMinListRowHeight, 0.0)
             .introspectTableView { tableView in
                 if tableView == tableViewObserver.tableView {

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

@@ -21,9 +21,8 @@ struct Settings: View {
     // MARK: Private
 
     @State private var showingLogoutConfirmation = false
-    @State private var showingTimelineStyles = false
     @Environment(\.colorScheme) private var colorScheme
-    @Environment(\.timelineStyler) private var timelineStyler
+    @ObservedObject private var settings = ElementSettings.shared
 
     // MARK: Public
 
@@ -94,26 +93,10 @@ struct Settings: View {
     private var userInterfaceSection: some View {
         if BuildSettings.settingsShowTimelineStyle {
             Section(header: Text(ElementL10n.settingsUserInterface)) {
-                Button { showingTimelineStyles = true } label: {
-                    HStack {
-                        Text(ElementL10n.settingsTimelineStyle)
-                        Spacer()
-                        Text(timelineStyler.shortDescription)
-                        Image(systemName: "chevron.right")
-                            .font(.body)
-                    }
-                }
-                .foregroundColor(Color.element.primaryContent)
-                .accessibilityIdentifier("timelineStyleButton")
-                .confirmationDialog(ElementL10n.settingsTimelineStyle,
-                                    isPresented: $showingTimelineStyles,
-                                    titleVisibility: .visible) {
-                    ForEach(TimelineStyler.allCases) { styler in
-                        Button { ElementSettings.shared.timelineStyle = styler.style
-                            timelineStyler.style = styler.style
-                        } label: {
-                            Text(styler.description)
-                        }
+                Picker(ElementL10n.settingsTimelineStyle, selection: $settings.timelineStyle) {
+                    ForEach(TimelineStyle.allCases, id: \.self) { style in
+                        Text(style.description)
+                            .tag(style)
                     }
                 }
             }