Refactor TimelineItemBubbledStylerView for polls (#1404)

* Refactor bubble bg colors and insets * Improve MapLibreStaticMapView layout * Add BubbleTimestampLayout * Refactor BubbleTimestampLayoutType * Refine .horizontal, .vertical layouts * Rename layout type * Add docs * Restore LocationRoomTimelineItem body * Refactor whitespaces * Fix background color for LocationRoomTimelineView. * Fix UTs * Fix analytics ui tests * Fix bug report tests * Fix settings ui tests * Rename private property * Record room screen tests
2026-11-02 21:33:26 +00:00 · 2023-07-26 17:17:41 +02:00
parent 99b5bf5150
commit 90ff2df00e
39 changed files with 209 additions and 231 deletions
@@ -48,9 +48,7 @@ struct MapLibreStaticMapView<PinAnnotation: View>: View {
                AsyncImage(url: url) { phase in
                    switch phase {
                    case .empty:
-                        Image("mapBlurred")
-                            .resizable()
-                            .aspectRatio(contentMode: .fill)
+                        placeholderImage
                    case .success(let image):
                        ZStack {
                            image
@@ -64,24 +62,31 @@ struct MapLibreStaticMapView<PinAnnotation: View>: View {
                        EmptyView()
                    }
                }
+                .position(x: geometry.frame(in: .local).midX, y: geometry.frame(in: .local).midY)
                .id(fetchAttempt)
            } else {
-                Image("mapBlurred")
+                placeholderImage
            }
        }
    }
-    
+
+    private var placeholderImage: some View {
+        Image("mapBlurred")
+            .resizable()
+            .scaledToFill()
+    }
+
    private var errorView: some View {
        Button {
            fetchAttempt += 1
        } label: {
-            ZStack {
-                Image("mapBlurred")
-                VStack {
-                    Image(systemName: "arrow.clockwise")
-                    Text(L10n.actionStaticMapLoad)
+            placeholderImage
+                .overlay {
+                    VStack {
+                        Image(systemName: "arrow.clockwise")
+                        Text(L10n.actionStaticMapLoad)
+                    }
                }
-            }
        }
    }
 }
@@ -26,11 +26,9 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
    @ViewBuilder let content: () -> Content

    @ScaledMetric private var senderNameVerticalPadding = 3
-    private let cornerRadius: CGFloat = 12
    
    @State private var showItemActionMenu = false

-    private var isTextItem: Bool { timelineItem is TextBasedRoomTimelineItem }
    private var isEncryptedOneToOneRoom: Bool { context.viewState.isEncryptedOneToOneRoom }
    
    /// The base padding applied to bubbles on either side.
@@ -65,7 +63,7 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
                        messageBubbleWithReactions
                    }
                    .padding(timelineItem.isOutgoing ? .leading : .trailing, 48) // Additional padding to differentiate alignment.
-                    
+
                    HStack(spacing: 0) {
                        if !timelineItem.isOutgoing {
                            Spacer()
@@ -150,50 +148,37 @@ struct TimelineItemBubbledStylerView<Content: View>: View {

    @ViewBuilder
    var styledContent: some View {
-        if shouldFillBubble {
-            contentWithTimestamp
-                .bubbleStyle(inset: false,
-                             cornerRadius: cornerRadius,
-                             corners: roundedCorners)
-        } else {
-            contentWithTimestamp
-                .bubbleStyle(inset: true,
-                             color: timelineItem.isOutgoing ? .compound._bgBubbleOutgoing : .compound._bgBubbleIncoming,
-                             cornerRadius: cornerRadius,
-                             corners: roundedCorners)
-        }
+        contentWithTimestamp
+            .bubbleStyle(insets: timelineItem.bubbleInsets,
+                         color: timelineItem.bubbleBackgroundColor,
+                         corners: roundedCorners)
    }

    @ViewBuilder
    var contentWithTimestamp: some View {
-        if isTextItem || shouldFillBubble {
-            ZStack(alignment: .bottomTrailing) {
+        timelineItem.bubbleSendInfoLayoutType
+            .layout {
                contentWithReply
                interactiveLocalizedSendInfo
            }
-        } else {
-            HStack(alignment: .bottom, spacing: 4) {
-                contentWithReply
-                interactiveLocalizedSendInfo
-            }
-        }
    }

    @ViewBuilder
    var interactiveLocalizedSendInfo: some View {
        if timelineItem.hasFailedToSend {
-            backgroundedLocalizedSendInfo
+            layoutedLocalizedSendInfo
                .onTapGesture {
                    context.sendFailedConfirmationDialogInfo = .init(itemID: timelineItem.id)
                }
        } else {
-            backgroundedLocalizedSendInfo
+            layoutedLocalizedSendInfo
        }
    }

    @ViewBuilder
-    var backgroundedLocalizedSendInfo: some View {
-        if shouldFillBubble {
+    var layoutedLocalizedSendInfo: some View {
+        switch timelineItem.bubbleSendInfoLayoutType {
+        case .overlay(capsuleStyle: true):
            localizedSendInfo
                .padding(.horizontal, 4)
                .padding(.vertical, 2)
@@ -201,9 +186,20 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
                .cornerRadius(10)
                .padding(.trailing, 4)
                .padding(.bottom, 4)
-
-        } else {
+        case .overlay(capsuleStyle: false):
            localizedSendInfo
+                .padding(.bottom, -4)
+        case .horizontal:
+            localizedSendInfo
+                .padding(.bottom, 4)
+                .padding(.trailing, 4)
+        case .vertical:
+            GridRow {
+                localizedSendInfo
+                    .padding(.bottom, 4)
+                    .padding(.trailing, 4)
+                    .gridColumnAlignment(.trailing)
+            }
        }
    }

@@ -222,7 +218,6 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
        }
        .font(.compound.bodyXS)
        .foregroundColor(timelineItem.hasFailedToSend ? .compound.textCriticalPrimary : .compound.textSecondary)
-        .padding(.bottom, shouldFillBubble ? 0 : -4)
    }
    
    @ViewBuilder
@@ -257,19 +252,6 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
        guard timelineItem.isOutgoing || isEncryptedOneToOneRoom else { return 0 }
        return timelineGroupStyle == .single || timelineGroupStyle == .first ? 8 : 0
    }
-
-    private var shouldFillBubble: Bool {
-        switch timelineItem {
-        case is ImageRoomTimelineItem,
-             is VideoRoomTimelineItem,
-             is StickerRoomTimelineItem:
-            return true
-        case let locationTimelineItem as LocationRoomTimelineItem:
-            return locationTimelineItem.content.geoURI != nil
-        default:
-            return false
-        }
-    }
    
    private var alignment: HorizontalAlignment {
        timelineItem.isOutgoing ? .trailing : .leading
@@ -302,13 +284,84 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
 }

 private extension View {
-    func bubbleStyle(inset: Bool, color: Color? = nil, cornerRadius: CGFloat, corners: UIRectCorner) -> some View {
-        padding(inset ? 8 : 0)
-            .background(inset ? color : nil)
+    func bubbleStyle(insets: CGFloat, color: Color? = nil, cornerRadius: CGFloat = 12, corners: UIRectCorner) -> some View {
+        padding(insets)
+            .background(color)
            .cornerRadius(cornerRadius, corners: corners)
    }
 }

+// Describes how the content and the send info should be arranged inside a bubble
+private enum BubbleSendInfoLayoutType {
+    case horizontal
+    case vertical
+    case overlay(capsuleStyle: Bool)
+
+    var layout: AnyLayout {
+        let layout: any Layout
+
+        switch self {
+        case .horizontal:
+            layout = HStackLayout(alignment: .bottom, spacing: 4)
+        case .vertical:
+            layout = GridLayout(alignment: .leading, verticalSpacing: 4)
+        case .overlay:
+            layout = ZStackLayout(alignment: .bottomTrailing)
+        }
+
+        return AnyLayout(layout)
+    }
+}
+
+private extension EventBasedTimelineItemProtocol {
+    var bubbleBackgroundColor: Color? {
+        let defaultColor: Color = isOutgoing ? .compound._bgBubbleOutgoing : .compound._bgBubbleIncoming
+
+        switch self {
+        case is ImageRoomTimelineItem,
+             is VideoRoomTimelineItem,
+             is StickerRoomTimelineItem:
+            return nil
+        default:
+            return defaultColor
+        }
+    }
+
+    // The insets for the full bubble content.
+    // Padding affecting just the "send info" should be added inside `layoutedLocalizedSendInfo`
+    var bubbleInsets: CGFloat {
+        let defaultPadding: CGFloat = 8
+
+        switch self {
+        case is ImageRoomTimelineItem,
+             is VideoRoomTimelineItem,
+             is StickerRoomTimelineItem:
+            return 0
+        case let locationTimelineItem as LocationRoomTimelineItem:
+            return locationTimelineItem.content.geoURI == nil ? defaultPadding : 0
+        default:
+            return defaultPadding
+        }
+    }
+
+    var bubbleSendInfoLayoutType: BubbleSendInfoLayoutType {
+        let defaultTimestampLayout: BubbleSendInfoLayoutType = .horizontal
+
+        switch self {
+        case is TextBasedRoomTimelineItem:
+            return .overlay(capsuleStyle: false)
+        case is ImageRoomTimelineItem,
+             is VideoRoomTimelineItem,
+             is StickerRoomTimelineItem:
+            return .overlay(capsuleStyle: true)
+        case let locationTimelineItem as LocationRoomTimelineItem:
+            return .overlay(capsuleStyle: locationTimelineItem.content.geoURI != nil)
+        default:
+            return defaultTimestampLayout
+        }
+    }
+}
+
 struct TimelineItemBubbledStylerView_Previews: PreviewProvider {
    static let viewModel = RoomScreenViewModel.mock

@@ -24,9 +24,9 @@ struct EmoteRoomTimelineView: View, TextBasedRoomTimelineViewProtocol {
    var body: some View {
        TimelineStyler(timelineItem: timelineItem) {
            if let attributedString = timelineItem.content.formattedBody {
-                FormattedBodyText(attributedString: attributedString, additionalWhitespacesCount: additionalWhitespaces)
+                FormattedBodyText(attributedString: attributedString, additionalWhitespacesCount: timelineItem.additionalWhitespaces(timelineStyle: timelineStyle))
            } else {
-                FormattedBodyText(text: timelineItem.content.body, additionalWhitespacesCount: additionalWhitespaces)
+                FormattedBodyText(text: timelineItem.content.body, additionalWhitespacesCount: timelineItem.additionalWhitespaces(timelineStyle: timelineStyle))
            }
        }
    }
@@ -34,9 +34,8 @@ struct LocationRoomTimelineView: View {
                    .aspectRatio(mapAspectRatio, contentMode: .fit)
                    .clipped()
                }
-                .background(backgroundView)
            } else {
-                FormattedBodyText(text: timelineItem.body)
+                FormattedBodyText(text: timelineItem.body, additionalWhitespacesCount: timelineItem.additionalWhitespaces(timelineStyle: timelineStyle))
            }
        }
    }
@@ -52,16 +51,6 @@ struct LocationRoomTimelineView: View {
        }
    }

-    @ViewBuilder
-    private var backgroundView: some View {
-        switch timelineStyle {
-        case .bubbles:
-            timelineItem.isOutgoing ? Color.compound._bgBubbleOutgoing : Color.compound._bgBubbleIncoming
-        case .plain:
-            EmptyView()
-        }
-    }
-
    private let mapAspectRatio: Double = 3 / 2
    private let mapMaxHeight: Double = 300
 }
@@ -33,9 +33,9 @@ struct NoticeRoomTimelineView: View, TextBasedRoomTimelineViewProtocol {
                    .foregroundColor(.compound.iconSecondary)
                
                if let attributedString = timelineItem.content.formattedBody {
-                    FormattedBodyText(attributedString: attributedString, additionalWhitespacesCount: additionalWhitespaces)
+                    FormattedBodyText(attributedString: attributedString, additionalWhitespacesCount: timelineItem.additionalWhitespaces(timelineStyle: timelineStyle))
                } else {
-                    FormattedBodyText(text: timelineItem.content.body, additionalWhitespacesCount: additionalWhitespaces)
+                    FormattedBodyText(text: timelineItem.content.body, additionalWhitespacesCount: timelineItem.additionalWhitespaces(timelineStyle: timelineStyle))
                }
            }
            .padding(.leading, 4) // Trailing padding is provided by FormattedBodyText
@@ -17,15 +17,3 @@
 import Foundation

 protocol TextBasedRoomTimelineItem: EventBasedMessageTimelineItemProtocol { }
-
-extension TextBasedRoomTimelineItem {
-    /// contains the timestamp and an optional edited localised prefix
-    /// example: (edited) 12:17 PM
-    var localizedSendInfo: String {
-        var start = ""
-        if properties.isEdited {
-            start = "\(L10n.commonEditedSuffix) "
-        }
-        return start + timestamp
-    }
-}
@@ -1,33 +0,0 @@
-//
-// Copyright 2023 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.
-//
-
-import Foundation
-
-// generated with auto mockable and customised to support the generic
-class TextBasedRoomTimelineViewMock<TimelineItemType: TextBasedRoomTimelineItem>: TextBasedRoomTimelineViewProtocol {
-    var timelineItem: TimelineItemType {
-        get { underlyingTimelineItem }
-        set(value) { underlyingTimelineItem = value }
-    }
-
-    var underlyingTimelineItem: TimelineItemType!
-    var timelineStyle: TimelineStyle {
-        get { underlyingTimelineStyle }
-        set(value) { underlyingTimelineStyle = value }
-    }
-
-    var underlyingTimelineStyle: TimelineStyle!
-}
@@ -20,22 +20,3 @@ protocol TextBasedRoomTimelineViewProtocol {
    var timelineItem: TimelineItemType { get }
    var timelineStyle: TimelineStyle { get }
 }
-
-extension TextBasedRoomTimelineViewProtocol {
-    var additionalWhitespaces: Int {
-        guard timelineStyle == .bubbles else {
-            return 0
-        }
-        var whiteSpaces = 1
-        timelineItem.localizedSendInfo.forEach { _ in
-            whiteSpaces += 1
-        }
-
-        // To account for the extra spacing created by the alert icon
-        if timelineItem.hasFailedToSend {
-            whiteSpaces += 3
-        }
-
-        return whiteSpaces
-    }
-}
@@ -25,11 +25,11 @@ struct TextRoomTimelineView: View, TextBasedRoomTimelineViewProtocol {
        TimelineStyler(timelineItem: timelineItem) {
            if let attributedString = timelineItem.content.formattedBody {
                FormattedBodyText(attributedString: attributedString,
-                                  additionalWhitespacesCount: additionalWhitespaces,
+                                  additionalWhitespacesCount: timelineItem.additionalWhitespaces(timelineStyle: timelineStyle),
                                  boostEmojiSize: true)
            } else {
                FormattedBodyText(text: timelineItem.body,
-                                  additionalWhitespacesCount: additionalWhitespaces,
+                                  additionalWhitespacesCount: timelineItem.additionalWhitespaces(timelineStyle: timelineStyle),
                                  boostEmojiSize: true)
            }
        }
@@ -53,4 +53,31 @@ extension EventBasedTimelineItemProtocol {
    var hasFailedDecryption: Bool {
        self is EncryptedRoomTimelineItem
    }
+
+    func additionalWhitespaces(timelineStyle: TimelineStyle) -> Int {
+        guard timelineStyle == .bubbles else {
+            return 0
+        }
+        var whiteSpaces = 1
+        localizedSendInfo.forEach { _ in
+            whiteSpaces += 1
+        }
+
+        // To account for the extra spacing created by the alert icon
+        if hasFailedToSend {
+            whiteSpaces += 3
+        }
+
+        return whiteSpaces
+    }
+
+    /// contains the timestamp and an optional edited localised prefix
+    /// example: (edited) 12:17 PM
+    var localizedSendInfo: String {
+        var start = ""
+        if properties.isEdited {
+            start = "\(L10n.commonEditedSuffix) "
+        }
+        return start + timestamp
+    }
 }
@@ -26,7 +26,7 @@ struct LocationRoomTimelineItem: EventBasedMessageTimelineItemProtocol, Equatabl
    let content: LocationRoomTimelineItemContent

    var body: String {
-        L10n.commonSharedLocation
+        content.body
    }

    var replyDetails: TimelineItemReplyDetails?