Collapsible timeline item tweaks (#646)

* Fix collapsible timeline items grouping just one item, remove duplicate timeline item checking, clean up the logic

* Cleanup collapsible, separator and state room timeline view paddings

* Add changelog

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

@@ -53,7 +53,7 @@ struct CollapsibleRoomTimelineView_Previews: PreviewProvider {
 
 private struct CollapsibleRoomTimelineItemDisclosureGroupStyle: DisclosureGroupStyle {
     func makeBody(configuration: Configuration) -> some View {
-        VStack {
+        VStack(spacing: 0.0) {
             HStack(alignment: .center) {
                 configuration.label
                 Text(Image(systemName: "chevron.forward"))
@@ -63,8 +63,10 @@ private struct CollapsibleRoomTimelineItemDisclosureGroupStyle: DisclosureGroupS
             .frame(maxWidth: .infinity)
             .font(.element.footnote)
             .foregroundColor(.element.secondaryContent)
-            .padding(.horizontal, 36)
+            .padding(.horizontal, 36.0)
-            .padding(.vertical, 8)
+            .padding(.top, 20.0)
+            .padding(.bottom, 12.0)
+
             .onTapGesture {
                 configuration.isExpanded.toggle()
             }

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

@@ -23,9 +23,9 @@ struct SeparatorRoomTimelineView: View {
         Text(timelineItem.text)
             .font(.element.footnote)
             .foregroundColor(.element.secondaryContent)
-            .padding(.top, 12)
-            .padding(.bottom, 8)
             .frame(maxWidth: .infinity)
+            .padding(.horizontal, 36.0)
+            .padding(.vertical, 8.0)
     }
 }
 

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

@@ -25,8 +25,8 @@ struct StateRoomTimelineView: View {
             .multilineTextAlignment(.center)
             .foregroundColor(.element.secondaryContent)
             .frame(maxWidth: .infinity, alignment: .center)
-            .padding(.horizontal, 36)
+            .padding(.horizontal, 36.0)
-            .padding(.vertical, 8)
+            .padding(.vertical, 8.0)
     }
 }
 

ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift

@@ -235,24 +235,37 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
         var canBackPaginate = true
         var isBackPaginating = false
         
-        var createdIdentifiers = [String: Bool]()
-        
         let collapsibleChunks = timelineProvider.itemsPublisher.value.groupBy { isItemCollapsible($0) }
         
-        for (index, itemGroup) in collapsibleChunks.enumerated() {
+        for (index, collapsibleChunk) in collapsibleChunks.enumerated() {
-            if itemGroup.count == 1, let itemProxy = itemGroup.first {
+            let isLastItem = index == collapsibleChunks.indices.last
-                let isLastItem = index == collapsibleChunks.indices.last
+            
-                if let item = buildTimelineItemFor(itemProxy: itemProxy, isLastItem: isLastItem, createdIdentifiers: &createdIdentifiers, isBackPaginating: &isBackPaginating, canBackPaginate: &canBackPaginate) {
+            let items = collapsibleChunk.compactMap { itemProxy in
-                    newTimelineItems.append(item)
+                let timelineItem = buildTimelineItemFor(itemProxy: itemProxy)
-                }
+                
-            } else {
+                if timelineItem is PaginationIndicatorRoomTimelineItem {
-                let items = itemGroup.compactMap { itemProxy in
+                    isBackPaginating = true
-                    buildTimelineItemFor(itemProxy: itemProxy, isLastItem: false, createdIdentifiers: &createdIdentifiers, isBackPaginating: &isBackPaginating, canBackPaginate: &canBackPaginate)
+                } else if timelineItem is TimelineStartRoomTimelineItem {
+                    canBackPaginate = false
                 }
                 
-                if !items.isEmpty {
+                return timelineItem
-                    newTimelineItems.append(CollapsibleTimelineItem(items: items))
+            }
+            
+            if items.isEmpty {
+                continue
+            }
+            
+            if items.count == 1, let timelineItem = items.first {
+                // Don't show the read marker if it's the last item in the timeline
+                // https://github.com/matrix-org/matrix-rust-sdk/issues/1546
+                guard !(timelineItem is ReadMarkerRoomTimelineItem && isLastItem) else {
+                    continue
                 }
+                
+                newTimelineItems.append(timelineItem)
+            } else {
+                newTimelineItems.append(CollapsibleTimelineItem(items: items))
             }
         }
 
@@ -263,22 +276,10 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
         callbacks.send(.isBackPaginating(isBackPaginating))
     }
     
-    private func buildTimelineItemFor(itemProxy: TimelineItemProxy,
+    private func buildTimelineItemFor(itemProxy: TimelineItemProxy) -> RoomTimelineItemProtocol? {
-                                      isLastItem: Bool,
-                                      createdIdentifiers: inout [String: Bool],
-                                      isBackPaginating: inout Bool,
-                                      canBackPaginate: inout Bool) -> RoomTimelineItemProtocol? {
         switch itemProxy {
         case .event(let eventItemProxy):
-            if let timelineItem = timelineItemFactory.buildTimelineItemFor(eventItemProxy: eventItemProxy) {
+            return timelineItemFactory.buildTimelineItemFor(eventItemProxy: eventItemProxy)
-                #warning("This works around duplicated items coming out of the SDK, remove once fixed")
-                if createdIdentifiers[timelineItem.id] == nil {
-                    createdIdentifiers[timelineItem.id] = true
-                    return timelineItem
-                } else {
-                    MXLog.error("Found duplicated timeline item, ignoring")
-                }
-            }
         case .virtual(let virtualItem):
             switch virtualItem {
             case .dayDivider(let timestamp):
@@ -287,15 +288,10 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
                 let dateString = date.formatted(date: .complete, time: .omitted)
                 return SeparatorRoomTimelineItem(text: dateString)
             case .readMarker:
-                // Don't show the read marker if it's the last item in the timeline
+                return ReadMarkerRoomTimelineItem()
-                if !isLastItem {
-                    return ReadMarkerRoomTimelineItem()
-                }
             case .loadingIndicator:
-                isBackPaginating = true
                 return PaginationIndicatorRoomTimelineItem()
             case .timelineStart:
-                canBackPaginate = false
                 return TimelineStartRoomTimelineItem(name: roomProxy.displayName ?? roomProxy.name)
             }
         case .unknown:

changelog.d/631.bugfix

@@ -0,0 +1 @@
+Prevent creating collapsible groups for one single event. Increase their padding and touch area.