Use a VStack on the timeline (#332)

* Use a VStack for the timeline. Replace edge publishers with a binding. * Allow both top and bottom edges to be detected. * Fix scrolling with frame changes.
2026-11-03 06:55:59 +00:00 · 2022-11-22 13:28:35 +00:00
parent 6c2ea61585
commit 5c4ca74426
8 changed files with 160 additions and 193 deletions
@@ -1,82 +0,0 @@
-//
-// 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.
-//
-
-import SwiftUI
-
-/// A SwiftUI scroll view that lays out its content starting at the bottom or trailing
-/// https://www.thirdrocktechkno.com/blog/implementing-reversed-scrolling-behaviour-in-swiftui/
-struct ReversedScrollView<Content: View>: View {
-    private let axis: Axis.Set
-    private let leadingSpace: CGFloat
-    private let content: Content
-    
-    init(_ axis: Axis.Set = .horizontal, leadingSpace: CGFloat = 0, @ViewBuilder builder: () -> Content) {
-        self.axis = axis
-        self.leadingSpace = leadingSpace
-        content = builder()
-    }
-    
-    var body: some View {
-        GeometryReader { proxy in
-            ScrollView(axis, showsIndicators: false) {
-                Stack(axis) {
-                    Spacer(minLength: leadingSpace)
-                    content
-                }
-                .frame(
-                    minWidth: minWidth(in: proxy, for: axis),
-                    minHeight: minHeight(in: proxy, for: axis)
-                )
-            }
-        }
-    }
-    
-    private func minWidth(in proxy: GeometryProxy, for axis: Axis.Set) -> CGFloat? {
-        axis.contains(.horizontal) ? proxy.size.width : nil
-    }
-    
-    private func minHeight(in proxy: GeometryProxy, for axis: Axis.Set) -> CGFloat? {
-        axis.contains(.vertical) ? proxy.size.height : nil
-    }
-}
-
-private struct Stack<Content: View>: View {
-    var axis: Axis.Set
-    var content: Content
-    
-    init(_ axis: Axis.Set = .vertical, @ViewBuilder builder: () -> Content) {
-        self.axis = axis
-        
-        content = builder()
-    }
-    
-    var body: some View {
-        switch axis {
-        case .horizontal:
-            HStack {
-                content
-            }
-        case .vertical:
-            VStack {
-                content
-            }
-        default:
-            VStack {
-                content
-            }
-        }
-    }
-}
@@ -38,7 +38,7 @@ struct ImageRoomTimelineView: View {
                        .foregroundColor(.element.systemGray6)
                        .opacity(0.3)
                    
-                    ProgressView("Loading")
+                    ProgressView(ElementL10n.loading)
                        .frame(maxWidth: .infinity)
                }
                .aspectRatio(timelineItem.aspectRatio, contentMode: .fit)
@@ -46,7 +46,6 @@ struct ImageRoomTimelineView: View {
        }
        .id(timelineItem.id)
        .animation(.elementDefault, value: timelineItem.image)
-        .frame(maxHeight: 1000.0)
    }
 }

@@ -34,7 +34,7 @@ struct VideoRoomTimelineView: View {
                        .foregroundColor(.element.systemGray6)
                        .opacity(0.3)
                    
-                    ProgressView("Loading")
+                    ProgressView(ElementL10n.loading)
                        .frame(maxWidth: .infinity)
                }
                .aspectRatio(timelineItem.aspectRatio, contentMode: .fit)
@@ -42,7 +42,6 @@ struct VideoRoomTimelineView: View {
        }
        .id(timelineItem.id)
        .animation(.elementDefault, value: timelineItem.image)
-        .frame(maxHeight: 1000.0)
    }

    @ViewBuilder
@@ -23,76 +23,74 @@ struct TimelineItemList: View {
    @State private var timelineItems: [RoomTimelineViewProvider] = []
    @State private var viewFrame: CGRect = .zero
    @State private var pinnedItem: PinnedItem?
-    @State private var visibleItemIdentifiers: Set<String> = []
-    @State private var topVisiblePublisher = CurrentValueSubject<Bool, Never>(false)
+    
+    @Binding var visibleEdges: [VerticalEdge]
+    /// The last known value of the visible edges. This is stored because `visibleEdges`
+    /// updates at the same time as the `viewFrame` but we need to know the previous
+    /// value when the keyboard appears to determine whether to scroll to the bottom.
+    @State private var cachedVisibleEdges: [VerticalEdge] = []
    
    @EnvironmentObject var context: RoomScreenViewModel.Context
    
-    let bottomVisiblePublisher: CurrentValueSubject<Bool, Never>
    let scrollToBottomPublisher: PassthroughSubject<Void, Never>

    var body: some View {
-        ScrollViewReader { proxy in
-            ReversedScrollView(.vertical) {
+        ScrollViewReader { scrollView in
+            TimelineScrollView(visibleEdges: $visibleEdges) {
+                // The scroll view already contains a VStack so simply provide the content to fill it.
+                
                ProgressView()
                    .frame(maxWidth: .infinity)
                    .opacity(context.viewState.isBackPaginating ? 1.0 : 0.0)
                    .animation(.elementDefault, value: context.viewState.isBackPaginating)
                
-                LazyVStack(alignment: .leading, spacing: 0.0) {
-                    ForEach(isRunningPreviews ? context.viewState.items : timelineItems) { item in
-                        item
-                            .contextMenu {
-                                context.viewState.contextMenuBuilder?(item.id)
-                                    .id(item.id)
-                            }
-                            .opacity(opacityForItem(item))
-                            .padding(settings.timelineStyle.rowInsets)
-                            .onAppear {
-                                context.send(viewAction: .itemAppeared(id: item.id))
-                                visibleItemIdentifiers.insert(item.id)
-                                
-                                if timelineItems.first == item {
-                                    topVisiblePublisher.send(true)
-                                }
-                                
-                                if timelineItems.last == item {
-                                    bottomVisiblePublisher.send(true)
-                                }
-                            }
-                            .onDisappear {
-                                context.send(viewAction: .itemDisappeared(id: item.id))
-                                visibleItemIdentifiers.remove(item.id)
-                                
-                                if timelineItems.first == item {
-                                    topVisiblePublisher.send(false)
-                                }
-                                
-                                if timelineItems.last == item {
-                                    bottomVisiblePublisher.send(false)
-                                }
-                            }
-                            .environment(\.openURL, OpenURLAction { url in
-                                context.send(viewAction: .linkClicked(url: url))
-                                return .systemAction
-                            })
-                            .onTapGesture {
-                                context.send(viewAction: .itemTapped(id: item.id))
-                            }
-                    }
+                ForEach(isRunningPreviews ? context.viewState.items : timelineItems) { item in
+                    item
+                        .contextMenu {
+                            context.viewState.contextMenuBuilder?(item.id)
+                                .id(item.id)
+                        }
+                        .opacity(opacityForItem(item))
+                        .padding(settings.timelineStyle.rowInsets)
+                        .onAppear {
+                            context.send(viewAction: .itemAppeared(id: item.id))
+                        }
+                        .onDisappear {
+                            context.send(viewAction: .itemDisappeared(id: item.id))
+                        }
+                        .environment(\.openURL, OpenURLAction { url in
+                            context.send(viewAction: .linkClicked(url: url))
+                            return .systemAction
+                        })
+                        .onTapGesture {
+                            context.send(viewAction: .itemTapped(id: item.id))
+                        }
+                }
+            }
+            .onChange(of: visibleEdges) { edges in
+                cachedVisibleEdges = edges
+                // Paginate when the top becomes visible
+                guard edges.contains(.top) else { return }
+                requestBackPagination()
+            }
+            .onChange(of: context.viewState.isBackPaginating) { isBackPaginating in
+                guard !isBackPaginating else { return }
+                
+                // Repeat the pagination if the top edge is still visible.
+                DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(1)) {
+                    guard visibleEdges.contains(.top) else { return }
+                    requestBackPagination()
                }
            }
            .onChange(of: pinnedItem) { item in
-                guard let item else {
-                    return
-                }
+                guard let item else { return }
                
                if item.animated {
                    withAnimation(Animation.elementDefault) {
-                        proxy.scrollTo(item.id, anchor: item.anchor)
+                        scrollView.scrollTo(item.id, anchor: item.anchor)
                    }
                } else {
-                    proxy.scrollTo(item.id, anchor: item.anchor)
+                    scrollView.scrollTo(item.id, anchor: item.anchor)
                }
                
                pinnedItem = nil
@@ -103,20 +101,17 @@ struct TimelineItemList: View {
        .timelineStyle(settings.timelineStyle)
        .onAppear {
            timelineItems = context.viewState.items
-            requestBackPagination()
-        }
-        // Allow SwiftUI to layout the views properly before checking if the top is visible
-        .onReceive(topVisiblePublisher.collect(.byTime(DispatchQueue.main, 0.5))) { values in
-            if values.last == true {
-                requestBackPagination()
-            }
        }
        .onReceive(scrollToBottomPublisher) {
            scrollToBottom(animated: true)
        }
-        .onChange(of: context.viewState.items.count) { _ in
-            guard !context.viewState.items.isEmpty,
-                  context.viewState.items.count != timelineItems.count else {
+        .onChange(of: context.viewState.items) { items in
+            guard
+                !context.viewState.items.isEmpty,
+                context.viewState.items.count != timelineItems.count
+            else {
+                // Update the items, but don't worry about scrolling if the count is unchanged.
+                timelineItems = items
                return
            }
            
@@ -132,9 +127,7 @@ struct TimelineItemList: View {
            }
            
            // Pin to the new bottom if visible
-            if let currentLastItem = timelineItems.last,
-               visibleItemIdentifiers.contains(currentLastItem.id),
-               let newLastItem = context.viewState.items.last {
+            if visibleEdges.contains(.bottom), let newLastItem = context.viewState.items.last {
                let pinnedItem = PinnedItem(id: newLastItem.id, anchor: .bottom, animated: false)
                timelineItems = context.viewState.items
                self.pinnedItem = pinnedItem
@@ -143,8 +136,7 @@ struct TimelineItemList: View {
            }
            
            // Pin to the old topmost visible
-            if let currentFirstItem = timelineItems.first,
-               visibleItemIdentifiers.contains(currentFirstItem.id) {
+            if visibleEdges.contains(.top), let currentFirstItem = timelineItems.first {
                let pinnedItem = PinnedItem(id: currentFirstItem.id, anchor: .top, animated: false)
                timelineItems = context.viewState.items
                self.pinnedItem = pinnedItem
@@ -155,18 +147,10 @@ struct TimelineItemList: View {
            // Otherwise just update the items
            timelineItems = context.viewState.items
        }
-        .onChange(of: context.viewState.items, perform: { items in
-            if timelineItems != items {
-                timelineItems = items
-            }
-        })
-        .background(GeometryReader { geo in
-            Color.clear.preference(key: ViewFramePreferenceKey.self, value: [geo.frame(in: .global)])
-        })
-        .onPreferenceChange(ViewFramePreferenceKey.self) { _ in
-            guard bottomVisiblePublisher.value == true else {
-                return
-            }
+        .onChange(of: viewFrame) { _ in
+            // Use the cached version as visibleEdges will already have changed
+            // (but its onChange handler is yet to be called - possible race condition?)
+            guard cachedVisibleEdges.contains(.bottom) else { return }
            
            // Pin the timeline to the bottom if was there on the frame change
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) {
@@ -213,14 +197,6 @@ private struct PinnedItem: Equatable {
    let animated: Bool
 }

-private struct ViewFramePreferenceKey: PreferenceKey {
-    static var defaultValue = [CGRect]() // Doesn't work with plain CGRects
-    
-    static func reduce(value: inout [CGRect], nextValue: () -> [CGRect]) {
-        value += nextValue()
-    }
-}
-
 struct TimelineItemList_Previews: PreviewProvider {
    static var previews: some View {
        body.preferredColorScheme(.light)
@@ -234,7 +210,7 @@ struct TimelineItemList_Previews: PreviewProvider {
                                            mediaProvider: MockMediaProvider(),
                                            roomName: nil)
        
-        TimelineItemList(bottomVisiblePublisher: CurrentValueSubject(false), scrollToBottomPublisher: PassthroughSubject())
+        TimelineItemList(visibleEdges: .constant([]), scrollToBottomPublisher: PassthroughSubject())
            .environmentObject(viewModel.context)
    }
 }
@@ -0,0 +1,76 @@
+//
+// 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.
+//
+
+import SwiftUI
+
+struct VisibleEdgesKey: PreferenceKey {
+    static var defaultValue: [VerticalEdge] = []
+
+    static func reduce(value: inout [VerticalEdge], nextValue: () -> [VerticalEdge]) {
+        value = nextValue()
+    }
+}
+
+/// A SwiftUI scroll view with the following customisations for a room timeline
+/// - The content is laid out starting at the bottom.
+/// - Top and bottom edge visibility detection for triggering other behaviours.
+struct TimelineScrollView<Content: View>: View {
+    @Binding var visibleEdges: [VerticalEdge]
+    
+    @ViewBuilder var content: () -> Content
+    
+    /// A small threshold added to the edge detection to allow a bit of leniency.
+    private let edgeDetectionThreshold: CGFloat = 15
+    
+    var body: some View {
+        GeometryReader { scrollViewGeometry in
+            ScrollView {
+                VStack(alignment: .leading, spacing: 0) {
+                    Spacer()
+                    content()
+                }
+                .frame(minHeight: scrollViewGeometry.size.height)
+                .background {
+                    GeometryReader { contentGeometry in
+                        Color.clear
+                            .preference(key: VisibleEdgesKey.self,
+                                        value: visibleEdges(of: contentGeometry, in: scrollViewGeometry))
+                    }
+                    .onPreferenceChange(VisibleEdgesKey.self) {
+                        visibleEdges = $0
+                    }
+                }
+            }
+        }
+    }
+    
+    func visibleEdges(of contentGeometry: GeometryProxy, in scrollViewGeometry: GeometryProxy) -> [VerticalEdge] {
+        let frame = contentGeometry.frame(in: .global)
+        let isTopVisible = scrollViewGeometry.frame(in: .global).contains(CGPoint(x: frame.midX, y: frame.minY + edgeDetectionThreshold))
+        let isBottomVisible = scrollViewGeometry.frame(in: .global).contains(CGPoint(x: frame.midX, y: frame.maxY - edgeDetectionThreshold))
+        
+        switch (isTopVisible, isBottomVisible) {
+        case (false, false):
+            return []
+        case (true, false):
+            return [.top]
+        case (false, true):
+            return [.bottom]
+        case (true, true):
+            return [.top, .bottom]
+        }
+    }
+}
@@ -21,31 +21,29 @@ import SwiftUI
 import Introspect

 struct TimelineView: View {
-    @State private var bottomVisiblePublisher = CurrentValueSubject<Bool, Never>(true)
+    @State private var visibleEdges: [VerticalEdge] = []
    @State private var scrollToBottomPublisher = PassthroughSubject<Void, Never>()
-    @State private var scollToBottomButtonVisible = false
+    @State private var scrollToBottomButtonVisible = false
    
    var body: some View {
        ZStack(alignment: .bottomTrailing) {
-            TimelineItemList(bottomVisiblePublisher: bottomVisiblePublisher, scrollToBottomPublisher: scrollToBottomPublisher)
+            TimelineItemList(visibleEdges: $visibleEdges, scrollToBottomPublisher: scrollToBottomPublisher)
            scrollToBottomButton
        }
    }
    
    @ViewBuilder
    private var scrollToBottomButton: some View {
-        Button(action: {
-            scrollToBottomPublisher.send(())
-        }, label: {
+        Button { scrollToBottomPublisher.send(()) } label: {
            Image(uiImage: Asset.Images.timelineScrollToBottom.image)
                .shadow(radius: 2.0)
                .padding()
-        })
-        .onReceive(bottomVisiblePublisher, perform: { visible in
-            scollToBottomButtonVisible = !visible
-        })
-        .opacity(scollToBottomButtonVisible ? 1.0 : 0.0)
-        .animation(.elementDefault, value: scollToBottomButtonVisible)
+        }
+        .onChange(of: visibleEdges) { edges in
+            scrollToBottomButtonVisible = !edges.contains(.bottom)
+        }
+        .opacity(scrollToBottomButtonVisible ? 1.0 : 0.0)
+        .animation(.elementDefault, value: scrollToBottomButtonVisible)
    }
 }