From 3c8daebfe1501d58938765ffe1f8cc005a411c90 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 31 Jul 2023 13:34:54 +0300 Subject: [PATCH] Delay rendering bottom timeline items when scrolled up and avoid content shifts - builds on top of langleyd/timeline_defer_new_items - improve animations --- .../Screens/RoomScreen/RoomScreenModels.swift | 11 +++- .../RoomScreen/RoomScreenViewModel.swift | 56 +++++++++++++++++-- .../Screens/RoomScreen/View/RoomScreen.swift | 13 +++-- .../Style/TimelineItemPlainStylerView.swift | 3 +- .../TimelineTableViewController.swift | 12 ++-- .../View/Timeline/UITimelineView.swift | 2 +- .../RoomScreen/View/TimelineView.swift | 19 ++++--- .../RoomSummary/RoomEventStringBuilder.swift | 2 +- .../TimelineItems/RoomTimelineItemView.swift | 2 +- 9 files changed, 87 insertions(+), 33 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift index 6e5fd71ba..2f045f2e4 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift @@ -80,6 +80,8 @@ enum RoomScreenViewAction { case retrySend(itemID: TimelineItemIdentifier) case cancelSend(itemID: TimelineItemIdentifier) + + case scrolledToBottom } struct RoomScreenViewState: BindableState { @@ -94,7 +96,7 @@ struct RoomScreenViewState: BindableState { var timelineViewState = TimelineViewState() // check the doc before changing this var composerMode: RoomScreenComposerMode = .default var swiftUITimelineEnabled = false - + var bindings: RoomScreenViewStateBindings /// A closure providing the actions to show when long pressing on an item in the timeline. @@ -109,7 +111,7 @@ struct RoomScreenViewStateBindings { var composerText: String var composerFocused: Bool - var scrollToBottomButtonVisible = false + var isScrolledToBottom = true var showAttachmentPopover = false { didSet { composerFocused = false @@ -184,12 +186,15 @@ struct TimelineViewState { var scrollToBottomPublisher = PassthroughSubject() var itemsDictionary = OrderedDictionary() + + var renderedTimelineIDs = [String]() + var pendingTimelineIDs = [String]() var timelineIDs: [String] { itemsDictionary.keys.elements } var itemViewStates: [RoomTimelineItemViewState] { - itemsDictionary.values.elements + renderedTimelineIDs.compactMap { itemsDictionary[$0] } } } diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index ef504f0d6..eab2c7fdd 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -138,6 +138,10 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol Task { await handleCancelSend(itemID: itemID) } case .paginateBackwards: paginateBackwards() + case .scrolledToBottom: + if state.swiftUITimelineEnabled { + renderPendingTimelineItems() + } } } @@ -276,29 +280,34 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol if itemGroup.count == 1 { if let firstItem = itemGroup.first { - timelineItemsDictionary.updateValue(updateViewstate(item: firstItem, groupStyle: .single), + timelineItemsDictionary.updateValue(updateViewState(item: firstItem, groupStyle: .single), forKey: firstItem.id.timelineID) } } else { for (index, item) in itemGroup.enumerated() { if index == 0 { - timelineItemsDictionary.updateValue(updateViewstate(item: item, groupStyle: .first), + timelineItemsDictionary.updateValue(updateViewState(item: item, groupStyle: .first), forKey: item.id.timelineID) } else if index == itemGroup.count - 1 { - timelineItemsDictionary.updateValue(updateViewstate(item: item, groupStyle: .last), + timelineItemsDictionary.updateValue(updateViewState(item: item, groupStyle: .last), forKey: item.id.timelineID) } else { - timelineItemsDictionary.updateValue(updateViewstate(item: item, groupStyle: .middle), + timelineItemsDictionary.updateValue(updateViewState(item: item, groupStyle: .middle), forKey: item.id.timelineID) } } } } - state.timelineViewState.itemsDictionary = timelineItemsDictionary + // The SwiftUI scroll view needs special handling, see `selectivellyUpdateTimelineItems` + if state.swiftUITimelineEnabled { + selectivelyUpdateTimelineItems(timelineItemsDictionary: timelineItemsDictionary) + } else { + state.timelineViewState.itemsDictionary = timelineItemsDictionary + } } - private func updateViewstate(item: RoomTimelineItemProtocol, groupStyle: TimelineGroupStyle) -> RoomTimelineItemViewState { + private func updateViewState(item: RoomTimelineItemProtocol, groupStyle: TimelineGroupStyle) -> RoomTimelineItemViewState { if let timelineItemViewState = state.timelineViewState.itemsDictionary[item.id.timelineID] { timelineItemViewState.groupStyle = groupStyle timelineItemViewState.type = .init(item: item) @@ -307,6 +316,41 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol return RoomTimelineItemViewState(item: item, groupStyle: groupStyle) } } + + /// With the timeline scroll being reversed, introducing items at it's top (i.e. bottom now) will make the content move upwards, which is unwanted when + /// reading history. Delay rendering new items until it reaches the bottom again. + private func selectivelyUpdateTimelineItems(timelineItemsDictionary: OrderedDictionary) { + var timelineViewState = state.timelineViewState + + let newItemIdentifiers = Array(timelineItemsDictionary.keys) + + if !state.bindings.isScrolledToBottom, + let lastItemIdentifier = state.timelineViewState.renderedTimelineIDs.last, + let newLastItemIdentifierIndex = newItemIdentifiers.firstIndex(where: { $0 == lastItemIdentifier }) { + timelineViewState.pendingTimelineIDs = Array(newItemIdentifiers.dropFirst(newLastItemIdentifierIndex + 1)) + timelineViewState.renderedTimelineIDs = Array(newItemIdentifiers.dropLast(newItemIdentifiers.count - (newLastItemIdentifierIndex + 1))) + } else { + // Otherwise just render everything normally + timelineViewState.renderedTimelineIDs = Array(timelineItemsDictionary.keys) + } + + timelineViewState.itemsDictionary = timelineItemsDictionary + + state.timelineViewState = timelineViewState + } + + private func renderPendingTimelineItems() { + // Render pending timeline items when the scroll view reaches the bottom again + guard state.bindings.isScrolledToBottom, + state.timelineViewState.pendingTimelineIDs.count > 0 else { + return + } + + var newTimelineViewState = state.timelineViewState + newTimelineViewState.renderedTimelineIDs = state.timelineViewState.renderedTimelineIDs + state.timelineViewState.pendingTimelineIDs + newTimelineViewState.pendingTimelineIDs = [] + state.timelineViewState = newTimelineViewState + } private func canGroupItem(timelineItem: RoomTimelineItemProtocol, with otherTimelineItem: RoomTimelineItemProtocol) -> Bool { if timelineItem is CollapsibleTimelineItem || otherTimelineItem is CollapsibleTimelineItem { diff --git a/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift b/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift index 064382255..794476352 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift @@ -80,6 +80,11 @@ struct RoomScreen: View { context.send(viewAction: .cancelSend(itemID: info.itemID)) } } + .onChange(of: context.isScrolledToBottom) { isScrolledToBottom in + if isScrolledToBottom { + context.send(viewAction: .scrolledToBottom) + } + } } private var timeline: some View { @@ -94,7 +99,7 @@ struct RoomScreen: View { private var timelineSwitch: some View { if context.viewState.swiftUITimelineEnabled { TimelineView(viewState: context.viewState.timelineViewState, - scrollToBottomButtonVisible: $context.scrollToBottomButtonVisible) { + isScrolledToBottom: $context.isScrolledToBottom) { context.send(viewAction: .paginateBackwards) } } else { @@ -121,9 +126,9 @@ struct RoomScreen: View { } .padding() } - .opacity(context.scrollToBottomButtonVisible ? 1.0 : 0.0) - .accessibilityHidden(!context.scrollToBottomButtonVisible) - .animation(.elementDefault, value: context.scrollToBottomButtonVisible) + .opacity(context.isScrolledToBottom ? 0.0 : 1.0) + .accessibilityHidden(context.isScrolledToBottom) + .animation(.elementDefault, value: context.isScrolledToBottom) } private var messageComposer: some View { diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift index d799f3a2b..2a1b6cf2e 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift @@ -54,8 +54,7 @@ struct TimelineItemPlainStylerView: View { Rectangle() .foregroundColor(.global.melon) .frame(width: 4.0) - TimelineReplyView(placement: .timeline, - timelineItemReplyDetails: replyDetails) + TimelineReplyView(placement: .timeline, timelineItemReplyDetails: replyDetails) } } diff --git a/ElementX/Sources/Screens/RoomScreen/View/Timeline/TimelineTableViewController.swift b/ElementX/Sources/Screens/RoomScreen/View/Timeline/TimelineTableViewController.swift index 4185637ae..a3544fff0 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Timeline/TimelineTableViewController.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Timeline/TimelineTableViewController.swift @@ -68,7 +68,7 @@ class TimelineTableViewController: UIViewController { var contextMenuActionProvider: (@MainActor (_ itemID: TimelineItemIdentifier) -> TimelineItemMenuActions?)? - @Binding private var scrollToBottomButtonVisible: Bool + @Binding private var isScrolledToBottom: Bool private var timelineItemsIDs: [String] { timelineItemsDictionary.keys.elements.reversed() @@ -90,11 +90,11 @@ class TimelineTableViewController: UIViewController { init(coordinator: UITimelineView.Coordinator, timelineStyle: TimelineStyle, - scrollToBottomButtonVisible: Binding, + isScrolledToBottom: Binding, scrollToBottomPublisher: PassthroughSubject) { self.coordinator = coordinator self.timelineStyle = timelineStyle - _scrollToBottomButtonVisible = scrollToBottomButtonVisible + _isScrolledToBottom = isScrolledToBottom super.init(nibName: nil, bundle: nil) @@ -253,11 +253,11 @@ extension TimelineTableViewController: UITableViewDelegate { DispatchQueue.main.async { [weak self] in guard let self else { return } - let scrollToBottomButtonVisible = scrollView.contentOffset.y > 0 + let isScrolledToBottom = scrollView.contentOffset.y <= 0 // Only update the binding on changes to avoid needlessly recomputing the hierarchy when scrolling. - if self.scrollToBottomButtonVisible != scrollToBottomButtonVisible { - self.scrollToBottomButtonVisible = scrollToBottomButtonVisible + if self.isScrolledToBottom != isScrolledToBottom { + self.isScrolledToBottom = isScrolledToBottom } } diff --git a/ElementX/Sources/Screens/RoomScreen/View/Timeline/UITimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/Timeline/UITimelineView.swift index 27c223b0e..58e695e1e 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Timeline/UITimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Timeline/UITimelineView.swift @@ -24,7 +24,7 @@ struct UITimelineView: UIViewControllerRepresentable { func makeUIViewController(context: Context) -> TimelineTableViewController { let tableViewController = TimelineTableViewController(coordinator: context.coordinator, timelineStyle: timelineStyle, - scrollToBottomButtonVisible: $viewModelContext.scrollToBottomButtonVisible, + isScrolledToBottom: $viewModelContext.isScrolledToBottom, scrollToBottomPublisher: viewModelContext.viewState.timelineViewState.scrollToBottomPublisher) return tableViewController } diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift index 5b490fe4c..3b582e769 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift @@ -21,7 +21,7 @@ import SwiftUIIntrospect struct TimelineView: View { let viewState: TimelineViewState - @Binding var scrollToBottomButtonVisible: Bool + @Binding var isScrolledToBottom: Bool let paginationAction: () -> Void @Environment(\.timelineStyle) private var timelineStyle @@ -61,17 +61,17 @@ struct TimelineView: View { .scrollDismissesKeyboard(.immediately) } .overlay(scrollToBottomButton, alignment: .bottomTrailing) - .animation(.elementDefault, value: viewState.timelineIDs) .onReceive(scrollViewAdapter.didScroll) { _ in guard let scrollView = scrollViewAdapter.scrollView else { return } let offset = scrollView.contentOffset.y + scrollView.contentInset.top - // We give it a bit of tollerance which solves the issue when of it being displayed when the keyboard appears - let scrollToBottomButtonVisibleValue = offset > 5 - if scrollToBottomButtonVisibleValue != scrollToBottomButtonVisible { - scrollToBottomButtonVisible = scrollToBottomButtonVisibleValue + let isScrolledToBottom = offset <= 0 + + // Only update the binding on changes to avoid needlessly recomputing the hierarchy when scrolling. + if self.isScrolledToBottom != isScrolledToBottom { + self.isScrolledToBottom = isScrolledToBottom } // Allows the scroll to top to work properly @@ -100,6 +100,7 @@ struct TimelineView: View { .scaleEffect(x: 1, y: -1) } } + .animation(.elementDefault, value: viewState.itemViewStates) topPin } } @@ -138,9 +139,9 @@ struct TimelineView: View { } .padding() } - .opacity(scrollToBottomButtonVisible ? 1.0 : 0.0) - .accessibilityHidden(!scrollToBottomButtonVisible) - .animation(.elementDefault, value: scrollToBottomButtonVisible) + .opacity(isScrolledToBottom ? 0.0 : 1.0) + .accessibilityHidden(isScrolledToBottom) + .animation(.elementDefault, value: isScrolledToBottom) } private func paginateBackwardsIfNeeded() { diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomEventStringBuilder.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomEventStringBuilder.swift index 2cff2804f..936ac0f64 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/RoomEventStringBuilder.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/RoomEventStringBuilder.swift @@ -23,7 +23,7 @@ struct RoomEventStringBuilder { self.stateEventStringBuilder = stateEventStringBuilder } - // swiftlint:disable:next cyclomatic_complexity function_body_length + // swiftlint:disable:next cyclomatic_complexity func buildAttributedString(for eventItemProxy: EventTimelineItemProxy) -> AttributedString? { let sender = eventItemProxy.sender let isOutgoing = eventItemProxy.isOwn diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift index 8ab3307cb..8f6760bda 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift @@ -18,7 +18,7 @@ import SwiftUI struct RoomTimelineItemView: View { @EnvironmentObject private var context: RoomScreenViewModel.Context @ObservedObject var viewState: RoomTimelineItemViewState - + var body: some View { timelineView .animation(.elementDefault, value: viewState.groupStyle)