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
element-hq · Jul 26, 2023 · 90ff2df · 90ff2df
1 parent 99b5bf5
commit 90ff2df
Show file tree

Hide file tree

Showing 39 changed files with 209 additions and 231 deletions.
diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj
@@ -392,7 +392,6 @@
  8C1A5ECAF895D4CAF8C4D461 /* AppActivityView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8F21ED7205048668BEB44A38 /* AppActivityView.swift */; };
  8C454500B8073E1201F801A9 /* MXLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = A34A814CBD56230BC74FFCF4 /* MXLogger.swift */; };
  8CC12086CBF91A7E10CDC205 /* HomeScreenCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = D653265D006E708E4E51AD64 /* HomeScreenCoordinator.swift */; };
- 8D0C5BC670D514760CC84E2A /* TextBasedRoomTimelineViewMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A542BC40D6EC2E66BC5659B /* TextBasedRoomTimelineViewMock.swift */; };
  8D3E1FADD78E72504DE0E402 /* UserAgentBuilderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EB3B237387B8288A5A938F1B /* UserAgentBuilderTests.swift */; };
  8D605456793F243649EC96AA /* target.yml in Resources */ = {isa = PBXBuildFile; fileRef = CD6B0C4639E066915B5E6463 /* target.yml */; };
  8D71E5E53F372202379BECCE /* BugReportScreenViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 303FCADE77DF1F3670C086ED /* BugReportScreenViewModel.swift */; };
@@ -1017,7 +1016,6 @@
  49D2C8E66E83EA578A7F318A /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist; path = Info.plist; sourceTree = "<group>"; };
  49E751D7EDB6043238111D90 /* UNNotificationRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UNNotificationRequest.swift; sourceTree = "<group>"; };
  4A4AD793D50748F8997E5B15 /* TimelineItemMacContextMenu.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineItemMacContextMenu.swift; sourceTree = "<group>"; };
- 4A542BC40D6EC2E66BC5659B /* TextBasedRoomTimelineViewMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TextBasedRoomTimelineViewMock.swift; sourceTree = "<group>"; };
  4AB7D7DAAAF662DED9D02379 /* MockMediaLoader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockMediaLoader.swift; sourceTree = "<group>"; };
  4AD6299F4516797E9BBE14C3 /* AnalyticsLocationType.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AnalyticsLocationType.swift; sourceTree = "<group>"; };
  4B41FABA2B0AEF4389986495 /* LoginMode.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoginMode.swift; sourceTree = "<group>"; };
@@ -3207,7 +3205,6 @@
  ED983D4DCA5AFA6E1ED96099 /* StateRoomTimelineView.swift */,
  612EF972F2A1800682D32C5E /* StickerRoomTimelineView.swift */,
  B9227F7495DA43324050A863 /* TextBasedRoomTimelineItem.swift */,
- 4A542BC40D6EC2E66BC5659B /* TextBasedRoomTimelineViewMock.swift */,
  4E47F18A9A077E351CEA10D4 /* TextBasedRoomTimelineViewProtocol.swift */,
  F9E785D5137510481733A3E8 /* TextRoomTimelineView.swift */,
  F9ED8E731E21055F728E5FED /* TimelineStartRoomTimelineView.swift */,
@@ -3670,14 +3667,6 @@
  path = Timeline;
  sourceTree = "<group>";
  };
- "TEMP_8B4A8977-AAD5-44D7-8CB7-F1BC31A64C84" /* element-x-ios */ = {
- isa = PBXGroup;
- children = (
- 41553551C55AD59885840F0E /* secrets.xcconfig */,
- );
- path = "element-x-ios";
- sourceTree = "<group>";
- };
 /* End PBXGroup section */
 
 /* Begin PBXNativeTarget section */
@@ -4657,7 +4646,6 @@
  2C4C750D0039AFABDF24236C /* TemplateScreenViewModelProtocol.swift in Sources */,
  D85D4FA590305180B4A41795 /* Tests.swift in Sources */,
  8317E1314C00DCCC99D30DA8 /* TextBasedRoomTimelineItem.swift in Sources */,
- 8D0C5BC670D514760CC84E2A /* TextBasedRoomTimelineViewMock.swift in Sources */,
  A2A5AB2E8B3F5CA769E531FA /* TextBasedRoomTimelineViewProtocol.swift in Sources */,
  BB784A02BADB03C820617A46 /* TextRoomTimelineItem.swift in Sources */,
  53F1196F9C69512306A2693F /* TextRoomTimelineItemContent.swift in Sources */,

diff --git a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@@ -11,7 +11,7 @@
  {
  "identity" : "compound-ios",
  "kind" : "remoteSourceControl",
- "location" : "https://github.com/vector-im/compound-ios.git",
+ "location" : "https://github.com/vector-im/compound-ios",
  "state" : {
  "revision" : "1f3eb60c4f87249d95addf84ce1aef22c2968763"
  }
@@ -208,10 +208,10 @@
  {
  "identity" : "swiftui-introspect",
  "kind" : "remoteSourceControl",
- "location" : "https://github.com/siteline/SwiftUI-Introspect.git",
+ "location" : "https://github.com/siteline/SwiftUI-Introspect",
  "state" : {
- "revision" : "730ab9e6cdbb3122ad88277b295c4cecd284a311",
- "version" : "0.9.1"
+ "revision" : "b94da693e57eaf79d16464b8b7c90d09cba4e290",
+ "version" : "0.9.2"
  }
  },
  {

diff --git a/ElementX/Sources/Other/MapLibre/MapLibreStaticMapView.swift b/ElementX/Sources/Other/MapLibre/MapLibreStaticMapView.swift
@@ -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)
+ }
  }
- }
  }
  }
 }

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift
@@ -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,60 +148,58 @@ 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) {
- contentWithReply
- interactiveLocalizedSendInfo
- }
- } else {
- HStack(alignment: .bottom, spacing: 4) {
+ timelineItem.bubbleSendInfoLayoutType
+ .layout {
  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)
  .background(Color.compound.bgSubtleSecondary)
  .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
 

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Timeline/EmoteRoomTimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/Timeline/EmoteRoomTimelineView.swift
@@ -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))
  }
  }
  }

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Timeline/LocationRoomTimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/Timeline/LocationRoomTimelineView.swift
@@ -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
 }

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Timeline/NoticeRoomTimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/Timeline/NoticeRoomTimelineView.swift
@@ -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