From a3ad5af9b5ce7573ac5bdd4595ef492fd9a1516d Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Fri, 21 Jul 2023 17:55:24 +0200 Subject: [PATCH 01/21] swiftUI conversion + scroll to bottom --- .../xcshareddata/swiftpm/Package.resolved | 4 +- .../RoomScreen/RoomScreenViewModel.swift | 18 +- .../Screens/RoomScreen/View/RoomScreen.swift | 3 +- .../TimelineItemStatusView.swift | 5 + .../View/TimelineTableViewController.swift | 283 ------------------ .../RoomScreen/View/TimelineView.swift | 121 ++++---- .../TimelineItems/RoomTimelineItemView.swift | 14 +- .../RoomTimelineItemViewModel.swift | 29 +- 8 files changed, 107 insertions(+), 370 deletions(-) delete mode 100644 ElementX/Sources/Screens/RoomScreen/View/TimelineTableViewController.swift diff --git a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index ed6f308542..b78e373659 100644 --- a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -228,8 +228,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/pointfreeco/xctest-dynamic-overlay", "state" : { - "revision" : "245d527002f29c5a616c5b7131f6a50ac7f41cbf", - "version" : "0.8.6" + "revision" : "50843cbb8551db836adec2290bb4bc6bac5c1865", + "version" : "0.9.0" } } ], diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index b5a74d5c4d..89f453c2c4 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -259,19 +259,19 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol if itemGroup.count == 1 { if let firstItem = itemGroup.first { - timelineItemsDictionary.updateValue(updateViewModel(item: firstItem, groupStyle: .single), + timelineItemsDictionary.updateValue(.init(item: firstItem, groupStyle: .single), forKey: firstItem.id.timelineID) } } else { for (index, item) in itemGroup.enumerated() { if index == 0 { - timelineItemsDictionary.updateValue(updateViewModel(item: item, groupStyle: .first), + timelineItemsDictionary.updateValue(.init(item: item, groupStyle: .first), forKey: item.id.timelineID) } else if index == itemGroup.count - 1 { - timelineItemsDictionary.updateValue(updateViewModel(item: item, groupStyle: .last), + timelineItemsDictionary.updateValue(.init(item: item, groupStyle: .last), forKey: item.id.timelineID) } else { - timelineItemsDictionary.updateValue(updateViewModel(item: item, groupStyle: .middle), + timelineItemsDictionary.updateValue(.init(item: item, groupStyle: .middle), forKey: item.id.timelineID) } } @@ -281,16 +281,6 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol state.itemsDictionary = timelineItemsDictionary } - private func updateViewModel(item: RoomTimelineItemProtocol, groupStyle: TimelineGroupStyle) -> RoomTimelineItemViewModel { - if let timelineItemViewModel = state.itemsDictionary[item.id.timelineID] { - timelineItemViewModel.groupStyle = groupStyle - timelineItemViewModel.type = .init(item: item) - return timelineItemViewModel - } else { - return RoomTimelineItemViewModel(item: item, groupStyle: groupStyle) - } - } - private func canGroupItem(timelineItem: RoomTimelineItemProtocol, with otherTimelineItem: RoomTimelineItemProtocol) -> Bool { if timelineItem is CollapsibleTimelineItem || otherTimelineItem is CollapsibleTimelineItem { return false diff --git a/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift b/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift index 2e65dbb66d..bbdb224df9 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift @@ -24,7 +24,7 @@ struct RoomScreen: View { var body: some View { timeline - .background(Color.compound.bgCanvasDefault.ignoresSafeArea()) // Kills the toolbar translucency. + .background(Color.compound.bgCanvasDefault.ignoresSafeArea()) .safeAreaInset(edge: .bottom, spacing: 0) { HStack(alignment: .bottom, spacing: attachmentButtonPadding) { RoomAttachmentPicker(context: context) @@ -36,6 +36,7 @@ struct RoomScreen: View { .padding(.trailing, 12) .padding(.top, 8) .padding(.bottom) + .background(Color.compound.bgCanvasDefault.ignoresSafeArea()) } .navigationBarTitleDisplayMode(.inline) .toolbar { toolbar } diff --git a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift index cdfc615ed2..da54d3eb0d 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift @@ -28,6 +28,11 @@ struct TimelineItemStatusView: View { } var body: some View { + mainContent + } + + @ViewBuilder + private var mainContent: some View { if !timelineItem.properties.orderedReadReceipts.isEmpty, readReceiptsEnabled { readReceipts } else { diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineTableViewController.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineTableViewController.swift deleted file mode 100644 index 240e0e8661..0000000000 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineTableViewController.swift +++ /dev/null @@ -1,283 +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 Combine -import SwiftUI - -import OrderedCollections - -/// A table view cell that displays a timeline item in a room. The cell is intended -/// to be configured to display a SwiftUI view and not use any UIKit. -class TimelineItemCell: UITableViewCell { - static let reuseIdentifier = "TimelineItemCell" - - var item: RoomTimelineItemViewModel? - - override func prepareForReuse() { - item = nil - } -} - -/// A table view controller that displays the timeline of a room. -/// -/// This class subclasses `UIViewController` as `UITableViewController` adds some -/// extra keyboard handling magic that wasn't playing well with SwiftUI (as of iOS 16.1). -/// Also this TableViewController uses a **flipped tableview** -class TimelineTableViewController: UIViewController { - private let coordinator: TimelineView.Coordinator - private let tableView = UITableView(frame: .zero, style: .plain) - - var timelineStyle: TimelineStyle - var timelineItemsDictionary = OrderedDictionary() { - didSet { - applySnapshot() - - if timelineItemsDictionary.isEmpty { - paginateBackwardsPublisher.send() - } - } - } - - /// The mode of the message composer. This is used to render selected - /// items in the timeline when replying, editing etc. - var composerMode: RoomScreenComposerMode = .default - - /// Whether or not the timeline has more messages to back paginate. - var canBackPaginate = true - - /// Whether or not the timeline is waiting for more messages to be added to the top. - var isBackPaginating = false { - didSet { - // Paginate again if the threshold hasn't been satisfied. - paginateBackwardsPublisher.send(()) - } - } - - var contextMenuActionProvider: (@MainActor (_ itemID: TimelineItemIdentifier) -> TimelineItemMenuActions?)? - - @Binding private var scrollToBottomButtonVisible: Bool - - private var timelineItemsIDs: [String] { - timelineItemsDictionary.keys.elements.reversed() - } - - /// The table's diffable data source. - private var dataSource: UITableViewDiffableDataSource? - private var cancellables: Set = [] - - /// A publisher used to throttle back pagination requests. - /// - /// Our view actions get wrapped in a `Task` so it is possible that a second call in - /// quick succession can execute before ``isBackPaginating`` becomes `true`. - private let paginateBackwardsPublisher = PassthroughSubject() - /// Whether or not the view has been shown on screen yet. - private var hasAppearedOnce = false - /// Whether the scroll and the animations should happen - private var shouldAnimate = false - - init(coordinator: TimelineView.Coordinator, - timelineStyle: TimelineStyle, - scrollToBottomButtonVisible: Binding, - scrollToBottomPublisher: PassthroughSubject) { - self.coordinator = coordinator - self.timelineStyle = timelineStyle - _scrollToBottomButtonVisible = scrollToBottomButtonVisible - - super.init(nibName: nil, bundle: nil) - - tableView.register(TimelineItemCell.self, forCellReuseIdentifier: TimelineItemCell.reuseIdentifier) - tableView.separatorStyle = .none - tableView.allowsSelection = false - tableView.keyboardDismissMode = .onDrag - tableView.backgroundColor = UIColor(.compound.bgCanvasDefault) - tableView.transform = CGAffineTransform(scaleX: 1, y: -1) - view.addSubview(tableView) - - // Prevents XCUITest from invoking the diffable dataSource's cellProvider - // for each possible cell, causing layout issues - tableView.accessibilityElementsHidden = Tests.shouldDisableTimelineAccessibility - - scrollToBottomPublisher - .sink { [weak self] _ in - self?.scrollToBottom(animated: true) - } - .store(in: &cancellables) - - paginateBackwardsPublisher - .collect(.byTime(DispatchQueue.main, 0.1)) - .sink { [weak self] _ in - self?.paginateBackwardsIfNeeded() - } - .store(in: &cancellables) - - configureDataSource() - } - - @available(*, unavailable) - required init?(coder: NSCoder) { fatalError("init(coder:) is not available.") } - - override func viewWillAppear(_ animated: Bool) { - super.viewWillAppear(animated) - - guard !hasAppearedOnce else { return } - tableView.contentOffset.y = -1 - hasAppearedOnce = true - paginateBackwardsPublisher.send() - - DispatchQueue.main.asyncAfter(deadline: .now() + 2) { - self.shouldAnimate = true - } - } - - override func viewWillLayoutSubviews() { - super.viewWillLayoutSubviews() - - guard tableView.frame.size != view.frame.size else { - return - } - - tableView.frame = CGRect(origin: .zero, size: view.frame.size) - } - - /// Configures a diffable data source for the timeline's table view. - private func configureDataSource() { - dataSource = .init(tableView: tableView) { [weak self] tableView, indexPath, id in - let cell = tableView.dequeueReusableCell(withIdentifier: TimelineItemCell.reuseIdentifier, for: indexPath) - guard let self, let cell = cell as? TimelineItemCell else { return cell } - - // A local reference to avoid capturing self in the cell configuration. - let coordinator = self.coordinator - - let viewModel = timelineItemsDictionary[id] - cell.item = viewModel - guard let viewModel else { - return cell - } - cell.contentConfiguration = UIHostingConfiguration { - RoomTimelineItemView(viewModel: viewModel) - .id(id) - .frame(maxWidth: .infinity, alignment: .leading) - .environmentObject(coordinator.context) // Attempted fix at a crash in TimelineItemContextMenu - .onAppear { - coordinator.send(viewAction: .itemAppeared(itemID: viewModel.id)) - } - .onDisappear { - coordinator.send(viewAction: .itemDisappeared(itemID: viewModel.id)) - } - .environment(\.openURL, OpenURLAction { url in - coordinator.send(viewAction: .linkClicked(url: url)) - return .systemAction - }) - } - .margins(.all, self.timelineStyle.rowInsets) - .minSize(height: 1) - .background(Color.clear) - - // Flipping the cell can create some issues with cell resizing, so flip the content View - cell.contentView.transform = CGAffineTransform(scaleX: 1, y: -1) - return cell - } - - dataSource?.defaultRowAnimation = .fade - tableView.delegate = self - } - - /// Updates the table view with the latest items from the ``timelineItems`` array. After - /// updating the data, the table will be scrolled to the bottom if it was visible otherwise - /// the scroll position will be updated to maintain the position of the last visible item. - private func applySnapshot() { - guard let dataSource else { return } - - var snapshot = NSDiffableDataSourceSnapshot() - snapshot.appendSections([.main]) - snapshot.appendItems(timelineItemsIDs) - - let currentSnapshot = dataSource.snapshot() - MXLog.verbose("DIFF: \(snapshot.itemIdentifiers.difference(from: currentSnapshot.itemIdentifiers))") - - // We only animate when new items come at the end of the timeline - let animated = shouldAnimate && - snapshot.itemIdentifiers.first != currentSnapshot.itemIdentifiers.first - dataSource.apply(snapshot, animatingDifferences: animated) - } - - /// Scrolls to the bottom of the timeline. - private func scrollToBottom(animated: Bool) { - guard !timelineItemsIDs.isEmpty else { - return - } - tableView.scrollToRow(at: IndexPath(item: 0, section: 0), at: .top, animated: animated) - } - - /// Scrolls to the top of the timeline. - private func scrollToTop(animated: Bool) { - guard !timelineItemsIDs.isEmpty else { - return - } - tableView.scrollToRow(at: IndexPath(item: timelineItemsIDs.count - 1, section: 0), at: .bottom, animated: animated) - } - - /// Checks whether or a backwards pagination is needed and requests one if so. - /// - /// Prefer not to call this directly, instead using ``paginateBackwardsPublisher`` to throttle requests. - private func paginateBackwardsIfNeeded() { - guard canBackPaginate, - !isBackPaginating, - tableView.contentOffset.y > tableView.contentSize.height - tableView.visibleSize.height * 2.0 - else { return } - - coordinator.send(viewAction: .paginateBackwards) - } -} - -// MARK: - UITableViewDelegate - -extension TimelineTableViewController: UITableViewDelegate { - func scrollViewDidScroll(_ scrollView: UIScrollView) { - paginateBackwardsPublisher.send(()) - - // Dispatch to fix runtime warning about making changes during a view update. - DispatchQueue.main.async { [weak self] in - guard let self else { return } - - let scrollToBottomButtonVisible = 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 - } - } - - // We never want the table view to be fully at the bottom to allow the status bar tap to work properly - if scrollView.contentOffset.y == 0 { - scrollView.contentOffset.y = -1 - } - } - - func scrollViewShouldScrollToTop(_ scrollView: UIScrollView) -> Bool { - scrollToTop(animated: true) - return false - } -} - -// MARK: - Layout Types - -extension TimelineTableViewController { - /// The sections of the table view used in the diffable data source. - enum TimelineSection { - case main - } -} diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift index 4aaf0274e7..35f51afd59 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift @@ -16,65 +16,80 @@ import SwiftUI -/// A table view wrapper that displays the timeline of a room. -struct TimelineView: UIViewControllerRepresentable { - @EnvironmentObject private var viewModelContext: RoomScreenViewModel.Context - @Environment(\.timelineStyle) private var timelineStyle - - func makeUIViewController(context: Context) -> TimelineTableViewController { - let tableViewController = TimelineTableViewController(coordinator: context.coordinator, - timelineStyle: timelineStyle, - scrollToBottomButtonVisible: $viewModelContext.scrollToBottomButtonVisible, - scrollToBottomPublisher: viewModelContext.viewState.scrollToBottomPublisher) - return tableViewController - } - - func updateUIViewController(_ uiViewController: TimelineTableViewController, context: Context) { - context.coordinator.update(tableViewController: uiViewController, timelineStyle: timelineStyle) +struct ScrollViewOffsetPreferenceKey: PreferenceKey { + static var defaultValue: CGFloat? + + static func reduce(value: inout CGFloat?, nextValue: () -> CGFloat?) { + value = value ?? nextValue() } - - func makeCoordinator() -> Coordinator { - Coordinator(viewModelContext: viewModelContext) +} + +struct HeightPreferenceKey: PreferenceKey { + static var defaultValue: CGFloat? + + static func reduce(value: inout CGFloat?, nextValue: () -> CGFloat?) { + value = value ?? nextValue() } - - // MARK: - Coordinator - - @MainActor - class Coordinator { - let context: RoomScreenViewModel.Context - - init(viewModelContext: RoomScreenViewModel.Context) { - context = viewModelContext - - if viewModelContext.viewState.itemViewModels.isEmpty { - viewModelContext.send(viewAction: .paginateBackwards) - } - } - - /// Updates the specified table view's properties from the current view state. - func update(tableViewController: TimelineTableViewController, timelineStyle: TimelineStyle) { - if tableViewController.timelineStyle != timelineStyle { - tableViewController.timelineStyle = timelineStyle - } - if tableViewController.timelineItemsDictionary != context.viewState.itemsDictionary { - tableViewController.timelineItemsDictionary = context.viewState.itemsDictionary +} + +struct TimelineView: View { + @EnvironmentObject private var context: RoomScreenViewModel.Context + @Environment(\.timelineStyle) private var timelineStyle + + private let scrollAreaId = "scrollArea" + private let bottomID = "bottomID" + + @State private var height: CGFloat? + @State private var offset: CGFloat? + + var body: some View { + ScrollViewReader { scrollView in + ScrollView { + GeometryReader { proxy in + let frame = proxy.frame(in: .named(scrollAreaId)) + // Since the scroll view is flipped the offset maxY is inverted + let offset = -frame.maxY + let height = frame.height + Color.clear.preference(key: ScrollViewOffsetPreferenceKey.self, value: offset) + Color.clear.preference(key: HeightPreferenceKey.self, value: height) + } + // It takes a little space so we give it 0 in height + .frame(height: 0) + .id(bottomID) + + LazyVStack(spacing: 0) { + ForEach(context.viewState.itemViewModels.reversed()) { viewModel in + RoomTimelineItemView(viewModel: viewModel) + .environmentObject(context) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(timelineStyle.rowInsets) + .scaleEffect(x: 1, y: -1) + } + } } - if tableViewController.canBackPaginate != context.viewState.canBackPaginate { - tableViewController.canBackPaginate = context.viewState.canBackPaginate + .coordinateSpace(name: scrollAreaId) + .scaleEffect(x: 1, y: -1) + .animation(.default, value: context.viewState.itemViewModels) + .onPreferenceChange(HeightPreferenceKey.self) { value in + guard let value, value != height else { + return + } + height = value } - if tableViewController.isBackPaginating != context.viewState.isBackPaginating { - tableViewController.isBackPaginating = context.viewState.isBackPaginating + .onPreferenceChange(ScrollViewOffsetPreferenceKey.self) { value in + guard let value else { + return + } + context.scrollToBottomButtonVisible = value > 0 } - if tableViewController.composerMode != context.viewState.composerMode { - tableViewController.composerMode = context.viewState.composerMode + .onReceive(context.viewState.scrollToBottomPublisher) { + guard let last = context.viewState.timelineIDs.last else { + return + } + withAnimation { + scrollView.scrollTo(bottomID) + } } - - // Doesn't have an equatable conformance :( - tableViewController.contextMenuActionProvider = context.viewState.timelineItemMenuActionProvider - } - - func send(viewAction: RoomScreenViewAction) { - context.send(viewAction: viewAction) } } } diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift index 2c8dbf9554..866c7f3081 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift @@ -16,13 +16,25 @@ import SwiftUI struct RoomTimelineItemView: View { - @ObservedObject var viewModel: RoomTimelineItemViewModel + @EnvironmentObject private var context: RoomScreenViewModel.Context + let viewModel: RoomTimelineItemViewModel var body: some View { timelineView + .environmentObject(context) .environment(\.timelineGroupStyle, viewModel.groupStyle) .animation(.elementDefault, value: viewModel.type) .animation(.elementDefault, value: viewModel.groupStyle) + .onAppear { + context.send(viewAction: .itemAppeared(itemID: viewModel.identifiers)) + } + .onDisappear { + context.send(viewAction: .itemDisappeared(itemID: viewModel.identifiers)) + } + .environment(\.openURL, OpenURLAction { url in + context.send(viewAction: .linkClicked(url: url)) + return .systemAction + }) } @ViewBuilder private var timelineView: some View { diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewModel.swift b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewModel.swift index 1ed1ba1e86..4cb81f3f4d 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewModel.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewModel.swift @@ -16,25 +16,16 @@ import Foundation -final class RoomTimelineItemViewModel: Identifiable, Equatable, ObservableObject { - static func == (lhs: RoomTimelineItemViewModel, rhs: RoomTimelineItemViewModel) -> Bool { - lhs.type == rhs.type && lhs.groupStyle == rhs.groupStyle - } - - @Published var type: RoomTimelineItemType - @Published var groupStyle: TimelineGroupStyle - - var id: TimelineItemIdentifier { - type.id - } +struct RoomTimelineItemViewModel: Identifiable, Equatable { + let type: RoomTimelineItemType + let groupStyle: TimelineGroupStyle - convenience init(item: RoomTimelineItemProtocol, groupStyle: TimelineGroupStyle) { - self.init(type: .init(item: item), groupStyle: groupStyle) + var id: String { + identifiers.timelineID } - init(type: RoomTimelineItemType, groupStyle: TimelineGroupStyle) { - self.type = type - self.groupStyle = groupStyle + var identifiers: TimelineItemIdentifier { + type.id } var isReactable: Bool { @@ -42,6 +33,12 @@ final class RoomTimelineItemViewModel: Identifiable, Equatable, ObservableObject } } +extension RoomTimelineItemViewModel { + init(item: RoomTimelineItemProtocol, groupStyle: TimelineGroupStyle) { + self.init(type: .init(item: item), groupStyle: groupStyle) + } +} + enum RoomTimelineItemType: Equatable { case text(TextRoomTimelineItem) case separator(SeparatorRoomTimelineItem) From af13ce48c837d9efc9e574f3f9282b14826667ac Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Fri, 21 Jul 2023 18:02:48 +0200 Subject: [PATCH 02/21] remove unused --- ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift | 3 --- 1 file changed, 3 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift index 35f51afd59..bd4286e721 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift @@ -83,9 +83,6 @@ struct TimelineView: View { context.scrollToBottomButtonVisible = value > 0 } .onReceive(context.viewState.scrollToBottomPublisher) { - guard let last = context.viewState.timelineIDs.last else { - return - } withAnimation { scrollView.scrollTo(bottomID) } From 48f5f9d7037a0777b64be5c95b2f439aac359afd Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Fri, 21 Jul 2023 19:24:12 +0200 Subject: [PATCH 03/21] back pagination implemented, however when a lot of elements are in the scroll the perfomance diminishes not sure what may be causing it --- .../RoomScreen/View/TimelineView.swift | 100 +++++++++++++----- 1 file changed, 73 insertions(+), 27 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift index bd4286e721..d3caa74305 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift @@ -14,46 +14,35 @@ // limitations under the License. // +import Combine import SwiftUI -struct ScrollViewOffsetPreferenceKey: PreferenceKey { - static var defaultValue: CGFloat? - - static func reduce(value: inout CGFloat?, nextValue: () -> CGFloat?) { - value = value ?? nextValue() - } -} - -struct HeightPreferenceKey: PreferenceKey { - static var defaultValue: CGFloat? - - static func reduce(value: inout CGFloat?, nextValue: () -> CGFloat?) { - value = value ?? nextValue() - } -} - struct TimelineView: View { @EnvironmentObject private var context: RoomScreenViewModel.Context @Environment(\.timelineStyle) private var timelineStyle - private let scrollAreaId = "scrollArea" + private let visibleArea = "visibleArea" + private let scrollAreaID = "scrollArea" private let bottomID = "bottomID" - @State private var height: CGFloat? + @State private var contentHeight: CGFloat? @State private var offset: CGFloat? + @State private var visibleHeight: CGFloat? + @State private var paginateBackwardsPublisher = PassthroughSubject() var body: some View { ScrollViewReader { scrollView in ScrollView { + // This is used as a pointer to the bottom of the view + // both for scrolling purposes and to understand the + // current content offset GeometryReader { proxy in - let frame = proxy.frame(in: .named(scrollAreaId)) + let frame = proxy.frame(in: .named(scrollAreaID)) // Since the scroll view is flipped the offset maxY is inverted let offset = -frame.maxY - let height = frame.height Color.clear.preference(key: ScrollViewOffsetPreferenceKey.self, value: offset) - Color.clear.preference(key: HeightPreferenceKey.self, value: height) } - // It takes a little space so we give it 0 in height + // It takes a little bit of space so we give it 0 in height .frame(height: 0) .id(bottomID) @@ -66,31 +55,88 @@ struct TimelineView: View { .scaleEffect(x: 1, y: -1) } } + .background( + GeometryReader { proxy in + Color.clear.preference(key: ContentHeightPreferenceKey.self, value: proxy.size.height) + } + ) } - .coordinateSpace(name: scrollAreaId) + .coordinateSpace(name: scrollAreaID) .scaleEffect(x: 1, y: -1) .animation(.default, value: context.viewState.itemViewModels) - .onPreferenceChange(HeightPreferenceKey.self) { value in - guard let value, value != height else { + .onPreferenceChange(ContentHeightPreferenceKey.self) { value in + guard let value, value != contentHeight else { return } - height = value + + contentHeight = value } .onPreferenceChange(ScrollViewOffsetPreferenceKey.self) { value in - guard let value else { + guard let value, value != offset else { return } + + offset = value context.scrollToBottomButtonVisible = value > 0 + + paginateBackwardsPublisher.send() } .onReceive(context.viewState.scrollToBottomPublisher) { withAnimation { scrollView.scrollTo(bottomID) } } + .background( + GeometryReader { proxy in + Color.clear.preference(key: VisibleHeightPreferenceKey.self, value: proxy.size.height) + } + ) + .onPreferenceChange(VisibleHeightPreferenceKey.self) { value in + guard let value, value != visibleHeight else { + return + } + visibleHeight = value + } + .onReceive(paginateBackwardsPublisher.collect(.byTime(DispatchQueue.main, 0.1))) { _ in + guard let offset, + let contentHeight, + let visibleHeight, + context.viewState.canBackPaginate, + !context.viewState.isBackPaginating, + offset > contentHeight - visibleHeight * 2.0 else { + return + } + + context.send(viewAction: .paginateBackwards) + } } } } +private struct ScrollViewOffsetPreferenceKey: PreferenceKey { + static var defaultValue: CGFloat? + + static func reduce(value: inout CGFloat?, nextValue: () -> CGFloat?) { + value = value ?? nextValue() + } +} + +private struct ContentHeightPreferenceKey: PreferenceKey { + static var defaultValue: CGFloat? + + static func reduce(value: inout CGFloat?, nextValue: () -> CGFloat?) { + value = value ?? nextValue() + } +} + +private struct VisibleHeightPreferenceKey: PreferenceKey { + static var defaultValue: CGFloat? + + static func reduce(value: inout CGFloat?, nextValue: () -> CGFloat?) { + value = value ?? nextValue() + } +} + // MARK: - Previews struct TimelineTableView_Previews: PreviewProvider { From fc4b0552aeea08ff3324c6b2ec76db04e4981217 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Fri, 21 Jul 2023 20:31:37 +0200 Subject: [PATCH 04/21] scroll adapter solution --- .../RoomScreen/View/TimelineView.swift | 97 ++++++++----------- 1 file changed, 38 insertions(+), 59 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift index d3caa74305..9c512bd1cc 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift @@ -17,34 +17,25 @@ import Combine import SwiftUI +import Introspect + struct TimelineView: View { @EnvironmentObject private var context: RoomScreenViewModel.Context @Environment(\.timelineStyle) private var timelineStyle - private let visibleArea = "visibleArea" - private let scrollAreaID = "scrollArea" private let bottomID = "bottomID" - @State private var contentHeight: CGFloat? - @State private var offset: CGFloat? - @State private var visibleHeight: CGFloat? + @State private var scrollViewAdapter = ScrollViewAdapter() @State private var paginateBackwardsPublisher = PassthroughSubject() var body: some View { ScrollViewReader { scrollView in ScrollView { - // This is used as a pointer to the bottom of the view - // both for scrolling purposes and to understand the - // current content offset - GeometryReader { proxy in - let frame = proxy.frame(in: .named(scrollAreaID)) - // Since the scroll view is flipped the offset maxY is inverted - let offset = -frame.maxY - Color.clear.preference(key: ScrollViewOffsetPreferenceKey.self, value: offset) - } - // It takes a little bit of space so we give it 0 in height - .frame(height: 0) - .id(bottomID) + // Only used ton get to the bottom of the scroll view + Divider() + .id(bottomID) + .hidden() + .frame(height: 0) LazyVStack(spacing: 0) { ForEach(context.viewState.itemViewModels.reversed()) { viewModel in @@ -55,62 +46,50 @@ struct TimelineView: View { .scaleEffect(x: 1, y: -1) } } - .background( - GeometryReader { proxy in - Color.clear.preference(key: ContentHeightPreferenceKey.self, value: proxy.size.height) - } - ) } - .coordinateSpace(name: scrollAreaID) - .scaleEffect(x: 1, y: -1) - .animation(.default, value: context.viewState.itemViewModels) - .onPreferenceChange(ContentHeightPreferenceKey.self) { value in - guard let value, value != contentHeight else { - return - } - - contentHeight = value + .introspect(.scrollView, on: .iOS(.v16)) { scrollView in + guard scrollView != scrollViewAdapter.scrollView else { return } + scrollViewAdapter.scrollView = scrollView } - .onPreferenceChange(ScrollViewOffsetPreferenceKey.self) { value in - guard let value, value != offset else { + .scaleEffect(x: 1, y: -1) + .animation(.elementDefault, value: context.viewState.itemViewModels) + .onReceive(scrollViewAdapter.didScroll) { _ in + guard let scrollView = scrollViewAdapter.scrollView else { return } - - offset = value - context.scrollToBottomButtonVisible = value > 0 - + let offset = scrollView.contentOffset.y + scrollView.contentInset.top + context.scrollToBottomButtonVisible = offset > 0 paginateBackwardsPublisher.send() } - .onReceive(context.viewState.scrollToBottomPublisher) { + .onReceive(context.viewState.scrollToBottomPublisher) { _ in withAnimation { scrollView.scrollTo(bottomID) } } - .background( - GeometryReader { proxy in - Color.clear.preference(key: VisibleHeightPreferenceKey.self, value: proxy.size.height) - } - ) - .onPreferenceChange(VisibleHeightPreferenceKey.self) { value in - guard let value, value != visibleHeight else { - return - } - visibleHeight = value - } .onReceive(paginateBackwardsPublisher.collect(.byTime(DispatchQueue.main, 0.1))) { _ in - guard let offset, - let contentHeight, - let visibleHeight, - context.viewState.canBackPaginate, - !context.viewState.isBackPaginating, - offset > contentHeight - visibleHeight * 2.0 else { - return - } - - context.send(viewAction: .paginateBackwards) + tryPaginateBackwards() } } } + + private func tryPaginateBackwards() { + guard let scrollView = scrollViewAdapter.scrollView, + context.viewState.canBackPaginate, + !context.viewState.isBackPaginating else { + return + } + + let visibleHeight = scrollView.visibleSize.height + let contentHeight = scrollView.contentSize.height + let offset = scrollView.contentOffset.y + scrollView.contentInset.top + let threshold = contentHeight - visibleHeight * 2 + + guard offset > threshold else { + return + } + + context.send(viewAction: .paginateBackwards) + } } private struct ScrollViewOffsetPreferenceKey: PreferenceKey { From 44359e5d04322d3d7c394bc1e9577e09e7264e10 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Mon, 24 Jul 2023 18:22:09 +0200 Subject: [PATCH 05/21] way better --- .../Screens/RoomScreen/RoomScreenModels.swift | 29 ++++--- .../RoomScreen/RoomScreenViewModel.swift | 26 ++++-- .../Screens/RoomScreen/View/RoomScreen.swift | 27 +----- .../Style/TimelineItemBubbledStylerView.swift | 2 +- .../View/Style/TimelineStyler.swift | 4 +- .../TimelineItemStatusView.swift | 2 +- .../RoomScreen/View/TimelineView.swift | 86 ++++++++++--------- 7 files changed, 92 insertions(+), 84 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift index 990e154947..003475c1ab 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift @@ -86,14 +86,13 @@ struct RoomScreenViewState: BindableState { var roomId: String var roomTitle = "" var roomAvatarURL: URL? - var itemsDictionary = OrderedDictionary() var members: [String: RoomMemberState] = [:] - var canBackPaginate = true - var isBackPaginating = false var showLoading = false var timelineStyle: TimelineStyle var readReceiptsEnabled: Bool var isEncryptedOneToOneRoom = false + let timelineViewState: TimelineViewState + var composerMode: RoomScreenComposerMode = .default let scrollToBottomPublisher = PassthroughSubject() @@ -106,14 +105,6 @@ struct RoomScreenViewState: BindableState { var sendButtonDisabled: Bool { bindings.composerText.count == 0 } - - var timelineIDs: [String] { - itemsDictionary.keys.elements - } - - var itemViewModels: [RoomTimelineItemViewModel] { - itemsDictionary.values.elements - } } struct RoomScreenViewStateBindings { @@ -184,3 +175,19 @@ struct RoomMemberState { let displayName: String? let avatarURL: URL? } + +final class TimelineViewState: ObservableObject { + @Published var canBackPaginate = false + @Published var isBackPaginating = false + @Published var itemsDictionary = OrderedDictionary() + + var timelineIDs: [String] { + itemsDictionary.keys.elements + } + + var itemViewModels: [RoomTimelineItemViewModel] { + itemsDictionary.values.elements + } + + var paginateAction: (() -> Void)? +} diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index 89f453c2c4..78d9348199 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -36,6 +36,8 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol private let notificationCenterProtocol: NotificationCenterProtocol private var canCurrentUserRedact = false + + private var paginateBackwards: Task? init(timelineController: RoomTimelineControllerProtocol, mediaProvider: MediaProviderProtocol, @@ -50,16 +52,27 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol self.analytics = analytics self.userIndicatorController = userIndicatorController self.notificationCenterProtocol = notificationCenterProtocol - + super.init(initialViewState: RoomScreenViewState(roomId: timelineController.roomID, roomTitle: roomProxy.roomTitle, roomAvatarURL: roomProxy.avatarURL, timelineStyle: appSettings.timelineStyle, readReceiptsEnabled: appSettings.readReceiptsEnabled, isEncryptedOneToOneRoom: roomProxy.isEncryptedOneToOneRoom, + timelineViewState: .init(), bindings: .init(composerText: "", composerFocused: false, reactionsCollapsed: [:])), imageProvider: mediaProvider) + state.timelineViewState.paginateAction = { [weak self] in + guard let self, + paginateBackwards == nil else { + return + } + paginateBackwards = Task { + await self.paginateBackwards() + } + } + setupSubscriptions() state.timelineItemMenuActionProvider = { [weak self] itemId -> TimelineItemMenuActions? in @@ -151,12 +164,12 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol case .updatedTimelineItems: self.buildTimelineViews() case .canBackPaginate(let canBackPaginate): - if self.state.canBackPaginate != canBackPaginate { - self.state.canBackPaginate = canBackPaginate + if self.state.timelineViewState.canBackPaginate != canBackPaginate { + self.state.timelineViewState.canBackPaginate = canBackPaginate } case .isBackPaginating(let isBackPaginating): - if self.state.isBackPaginating != isBackPaginating { - self.state.isBackPaginating = isBackPaginating + if self.state.timelineViewState.isBackPaginating != isBackPaginating { + self.state.timelineViewState.isBackPaginating = isBackPaginating } } } @@ -222,6 +235,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol default: break } + paginateBackwards = nil } private func markRoomAsRead() async { @@ -278,7 +292,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol } } - state.itemsDictionary = timelineItemsDictionary + state.timelineViewState.itemsDictionary = timelineItemsDictionary } private func canGroupItem(timelineItem: RoomTimelineItemProtocol, with otherTimelineItem: RoomTimelineItemProtocol) -> Bool { diff --git a/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift b/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift index bbdb224df9..3019418a61 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift @@ -81,14 +81,14 @@ struct RoomScreen: View { } } } - + + @ViewBuilder private var timeline: some View { - TimelineView() + TimelineView(viewState: context.viewState.timelineViewState) .id(context.viewState.roomId) .environmentObject(context) .environment(\.timelineStyle, context.viewState.timelineStyle) .environment(\.readReceiptsEnabled, context.viewState.readReceiptsEnabled) - .overlay(alignment: .bottomTrailing) { scrollToBottomButton } } private var messageComposer: some View { @@ -109,27 +109,6 @@ struct RoomScreen: View { } } - private var scrollToBottomButton: some View { - Button { context.viewState.scrollToBottomPublisher.send(()) } label: { - Image(systemName: "chevron.down") - .font(.compound.bodyLG) - .fontWeight(.semibold) - .foregroundColor(.compound.iconSecondary) - .padding(13) - .offset(y: 1) - .background { - Circle() - .fill(Color.compound.iconOnSolidPrimary) - // Intentionally using system primary colour to get white/black. - .shadow(color: .primary.opacity(0.33), radius: 2.0) - } - .padding() - } - .opacity(context.scrollToBottomButtonVisible ? 1.0 : 0.0) - .accessibilityHidden(!context.scrollToBottomButtonVisible) - .animation(.elementDefault, value: context.scrollToBottomButtonVisible) - } - @ViewBuilder private var loadingIndicator: some View { if context.viewState.showLoading { diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift index 6fea8e8e61..d61b1e895c 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift @@ -328,7 +328,7 @@ struct TimelineItemBubbledStylerView_Previews: PreviewProvider { static var mockTimeline: some View { ScrollView { VStack(alignment: .leading, spacing: 0) { - ForEach(viewModel.state.itemViewModels) { itemViewModel in + ForEach(viewModel.state.timelineViewState.itemViewModels) { itemViewModel in RoomTimelineItemView(viewModel: itemViewModel) .padding(TimelineStyle.bubbles.rowInsets) // Insets added in the table view cells diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineStyler.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineStyler.swift index 875921ab49..9dfcac87e4 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineStyler.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineStyler.swift @@ -53,7 +53,7 @@ struct TimelineItemStyler_Previews: PreviewProvider { }() static let sendingLast: TextRoomTimelineItem = { - let id = viewModel.state.timelineIDs.last ?? UUID().uuidString + let id = viewModel.state.timelineViewState.timelineIDs.last ?? UUID().uuidString var result = TextRoomTimelineItem(id: .init(timelineID: id), timestamp: "Now", isOutgoing: true, isEditable: false, sender: .init(id: UUID().uuidString), content: .init(body: "Test")) result.properties.deliveryStatus = .sending return result @@ -66,7 +66,7 @@ struct TimelineItemStyler_Previews: PreviewProvider { }() static let sentLast: TextRoomTimelineItem = { - let id = viewModel.state.timelineIDs.last ?? UUID().uuidString + let id = viewModel.state.timelineViewState.timelineIDs.last ?? UUID().uuidString let result = TextRoomTimelineItem(id: .init(timelineID: id), timestamp: "Now", isOutgoing: true, isEditable: false, sender: .init(id: UUID().uuidString), content: .init(body: "Test")) return result }() diff --git a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift index da54d3eb0d..33686690e9 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift @@ -23,7 +23,7 @@ struct TimelineItemStatusView: View { @EnvironmentObject private var context: RoomScreenViewModel.Context private var isLastOutgoingMessage: Bool { - context.viewState.timelineIDs.last == timelineItem.id.timelineID && + context.viewState.timelineViewState.timelineIDs.last == timelineItem.id.timelineID && timelineItem.isOutgoing } diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift index 9c512bd1cc..7b43e02b16 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift @@ -17,16 +17,19 @@ import Combine import SwiftUI -import Introspect +import OrderedCollections +import SwiftUIIntrospect struct TimelineView: View { - @EnvironmentObject private var context: RoomScreenViewModel.Context + @ObservedObject var viewState: TimelineViewState @Environment(\.timelineStyle) private var timelineStyle private let bottomID = "bottomID" @State private var scrollViewAdapter = ScrollViewAdapter() @State private var paginateBackwardsPublisher = PassthroughSubject() + @State private var scrollToBottomPublisher = PassthroughSubject() + @State private var scrollToBottomButtonVisible = false var body: some View { ScrollViewReader { scrollView in @@ -38,12 +41,13 @@ struct TimelineView: View { .frame(height: 0) LazyVStack(spacing: 0) { - ForEach(context.viewState.itemViewModels.reversed()) { viewModel in - RoomTimelineItemView(viewModel: viewModel) - .environmentObject(context) - .frame(maxWidth: .infinity, alignment: .leading) - .padding(timelineStyle.rowInsets) - .scaleEffect(x: 1, y: -1) + ForEach(viewState.timelineIDs.reversed(), id: \.self) { id in + if let viewModel = viewState.itemsDictionary[id] { + RoomTimelineItemView(viewModel: viewModel) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(timelineStyle.rowInsets) + .scaleEffect(x: 1, y: -1) + } } } } @@ -52,16 +56,19 @@ struct TimelineView: View { scrollViewAdapter.scrollView = scrollView } .scaleEffect(x: 1, y: -1) - .animation(.elementDefault, value: context.viewState.itemViewModels) + .animation(.elementDefault, value: viewState.itemsDictionary) .onReceive(scrollViewAdapter.didScroll) { _ in guard let scrollView = scrollViewAdapter.scrollView else { return } let offset = scrollView.contentOffset.y + scrollView.contentInset.top - context.scrollToBottomButtonVisible = offset > 0 + let scrollToBottomButtonVisibleValue = offset > 0 + if scrollToBottomButtonVisibleValue != scrollToBottomButtonVisible { + scrollToBottomButtonVisible = scrollToBottomButtonVisibleValue + } paginateBackwardsPublisher.send() } - .onReceive(context.viewState.scrollToBottomPublisher) { _ in + .onReceive(scrollToBottomPublisher) { _ in withAnimation { scrollView.scrollTo(bottomID) } @@ -70,12 +77,37 @@ struct TimelineView: View { tryPaginateBackwards() } } + .overlay(scrollToBottomButton, alignment: .bottomTrailing) + } + + private var scrollToBottomButton: some View { + Button { + scrollToBottomPublisher.send() + } label: { + Image(systemName: "chevron.down") + .font(.compound.bodyLG) + .fontWeight(.semibold) + .foregroundColor(.compound.iconSecondary) + .padding(13) + .offset(y: 1) + .background { + Circle() + .fill(Color.compound.iconOnSolidPrimary) + // Intentionally using system primary colour to get white/black. + .shadow(color: .primary.opacity(0.33), radius: 2.0) + } + .padding() + } + .opacity(scrollToBottomButtonVisible ? 1.0 : 0.0) + .accessibilityHidden(!scrollToBottomButtonVisible) + .animation(.elementDefault, value: scrollToBottomButtonVisible) } private func tryPaginateBackwards() { - guard let scrollView = scrollViewAdapter.scrollView, - context.viewState.canBackPaginate, - !context.viewState.isBackPaginating else { + guard let paginateAction = viewState.paginateAction, + let scrollView = scrollViewAdapter.scrollView, + viewState.canBackPaginate, + !viewState.isBackPaginating else { return } @@ -88,31 +120,7 @@ struct TimelineView: View { return } - context.send(viewAction: .paginateBackwards) - } -} - -private struct ScrollViewOffsetPreferenceKey: PreferenceKey { - static var defaultValue: CGFloat? - - static func reduce(value: inout CGFloat?, nextValue: () -> CGFloat?) { - value = value ?? nextValue() - } -} - -private struct ContentHeightPreferenceKey: PreferenceKey { - static var defaultValue: CGFloat? - - static func reduce(value: inout CGFloat?, nextValue: () -> CGFloat?) { - value = value ?? nextValue() - } -} - -private struct VisibleHeightPreferenceKey: PreferenceKey { - static var defaultValue: CGFloat? - - static func reduce(value: inout CGFloat?, nextValue: () -> CGFloat?) { - value = value ?? nextValue() + paginateAction() } } From 2c4a95ecf2bbbaa84b75709ed9e581de8a85c050 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Mon, 24 Jul 2023 19:06:27 +0200 Subject: [PATCH 06/21] works but height animation in cells are broken --- .../RoomScreen/RoomScreenViewModel.swift | 32 ++++++++++--------- .../RoomScreen/View/TimelineView.swift | 6 ++-- .../TimelineItems/RoomTimelineItemView.swift | 6 ++-- .../RoomTimelineItemViewModel.swift | 31 +++++++++++------- 4 files changed, 42 insertions(+), 33 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index 78d9348199..d58c8852d8 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -36,8 +36,6 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol private let notificationCenterProtocol: NotificationCenterProtocol private var canCurrentUserRedact = false - - private var paginateBackwards: Task? init(timelineController: RoomTimelineControllerProtocol, mediaProvider: MediaProviderProtocol, @@ -52,7 +50,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol self.analytics = analytics self.userIndicatorController = userIndicatorController self.notificationCenterProtocol = notificationCenterProtocol - + super.init(initialViewState: RoomScreenViewState(roomId: timelineController.roomID, roomTitle: roomProxy.roomTitle, roomAvatarURL: roomProxy.avatarURL, @@ -64,15 +62,10 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol imageProvider: mediaProvider) state.timelineViewState.paginateAction = { [weak self] in - guard let self, - paginateBackwards == nil else { - return - } - paginateBackwards = Task { - await self.paginateBackwards() + Task { + await self?.paginateBackwards() } } - setupSubscriptions() state.timelineItemMenuActionProvider = { [weak self] itemId -> TimelineItemMenuActions? in @@ -235,7 +228,6 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol default: break } - paginateBackwards = nil } private func markRoomAsRead() async { @@ -273,19 +265,19 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol if itemGroup.count == 1 { if let firstItem = itemGroup.first { - timelineItemsDictionary.updateValue(.init(item: firstItem, groupStyle: .single), + timelineItemsDictionary.updateValue(updateViewModel(item: firstItem, groupStyle: .single), forKey: firstItem.id.timelineID) } } else { for (index, item) in itemGroup.enumerated() { if index == 0 { - timelineItemsDictionary.updateValue(.init(item: item, groupStyle: .first), + timelineItemsDictionary.updateValue(updateViewModel(item: item, groupStyle: .first), forKey: item.id.timelineID) } else if index == itemGroup.count - 1 { - timelineItemsDictionary.updateValue(.init(item: item, groupStyle: .last), + timelineItemsDictionary.updateValue(updateViewModel(item: item, groupStyle: .last), forKey: item.id.timelineID) } else { - timelineItemsDictionary.updateValue(.init(item: item, groupStyle: .middle), + timelineItemsDictionary.updateValue(updateViewModel(item: item, groupStyle: .middle), forKey: item.id.timelineID) } } @@ -295,6 +287,16 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol state.timelineViewState.itemsDictionary = timelineItemsDictionary } + private func updateViewModel(item: RoomTimelineItemProtocol, groupStyle: TimelineGroupStyle) -> RoomTimelineItemViewModel { + if let timelineItemViewModel = state.timelineViewState.itemsDictionary[item.id.timelineID] { + timelineItemViewModel.groupStyle = groupStyle + timelineItemViewModel.type = .init(item: item) + return timelineItemViewModel + } else { + return RoomTimelineItemViewModel(item: item, groupStyle: groupStyle) + } + } + private func canGroupItem(timelineItem: RoomTimelineItemProtocol, with otherTimelineItem: RoomTimelineItemProtocol) -> Bool { if timelineItem is CollapsibleTimelineItem || otherTimelineItem is CollapsibleTimelineItem { return false diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift index 7b43e02b16..db1fa3028c 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift @@ -40,13 +40,13 @@ struct TimelineView: View { .hidden() .frame(height: 0) - LazyVStack(spacing: 0) { + VStack(spacing: 0) { ForEach(viewState.timelineIDs.reversed(), id: \.self) { id in if let viewModel = viewState.itemsDictionary[id] { RoomTimelineItemView(viewModel: viewModel) .frame(maxWidth: .infinity, alignment: .leading) - .padding(timelineStyle.rowInsets) .scaleEffect(x: 1, y: -1) + .padding(timelineStyle.rowInsets) } } } @@ -55,8 +55,8 @@ struct TimelineView: View { guard scrollView != scrollViewAdapter.scrollView else { return } scrollViewAdapter.scrollView = scrollView } + .animation(.elementDefault, value: viewState.timelineIDs) .scaleEffect(x: 1, y: -1) - .animation(.elementDefault, value: viewState.itemsDictionary) .onReceive(scrollViewAdapter.didScroll) { _ in guard let scrollView = scrollViewAdapter.scrollView else { return diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift index 866c7f3081..a7d2dd1904 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift @@ -17,7 +17,7 @@ import SwiftUI struct RoomTimelineItemView: View { @EnvironmentObject private var context: RoomScreenViewModel.Context - let viewModel: RoomTimelineItemViewModel + @ObservedObject var viewModel: RoomTimelineItemViewModel var body: some View { timelineView @@ -26,10 +26,10 @@ struct RoomTimelineItemView: View { .animation(.elementDefault, value: viewModel.type) .animation(.elementDefault, value: viewModel.groupStyle) .onAppear { - context.send(viewAction: .itemAppeared(itemID: viewModel.identifiers)) + context.send(viewAction: .itemAppeared(itemID: viewModel.identifier)) } .onDisappear { - context.send(viewAction: .itemDisappeared(itemID: viewModel.identifiers)) + context.send(viewAction: .itemDisappeared(itemID: viewModel.identifier)) } .environment(\.openURL, OpenURLAction { url in context.send(viewAction: .linkClicked(url: url)) diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewModel.swift b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewModel.swift index 4cb81f3f4d..c3cc12e9b6 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewModel.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewModel.swift @@ -16,27 +16,34 @@ import Foundation -struct RoomTimelineItemViewModel: Identifiable, Equatable { - let type: RoomTimelineItemType - let groupStyle: TimelineGroupStyle - - var id: String { - identifiers.timelineID +final class RoomTimelineItemViewModel: Identifiable, Equatable, ObservableObject { + static func == (lhs: RoomTimelineItemViewModel, rhs: RoomTimelineItemViewModel) -> Bool { + lhs.type == rhs.type && lhs.groupStyle == rhs.groupStyle } - var identifiers: TimelineItemIdentifier { + @Published var type: RoomTimelineItemType + @Published var groupStyle: TimelineGroupStyle + + var identifier: TimelineItemIdentifier { type.id } - var isReactable: Bool { - type.isReactable + var id: String { + identifier.timelineID } -} -extension RoomTimelineItemViewModel { - init(item: RoomTimelineItemProtocol, groupStyle: TimelineGroupStyle) { + convenience init(item: RoomTimelineItemProtocol, groupStyle: TimelineGroupStyle) { self.init(type: .init(item: item), groupStyle: groupStyle) } + + init(type: RoomTimelineItemType, groupStyle: TimelineGroupStyle) { + self.type = type + self.groupStyle = groupStyle + } + + var isReactable: Bool { + type.isReactable + } } enum RoomTimelineItemType: Equatable { From b8a96fff0d7e8eedf2da57a7d3f7fab05596a444 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Mon, 24 Jul 2023 20:48:24 +0200 Subject: [PATCH 07/21] code improvement --- .../Screens/RoomScreen/View/TimelineView.swift | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift index db1fa3028c..717ff5edb6 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift @@ -17,7 +17,6 @@ import Combine import SwiftUI -import OrderedCollections import SwiftUIIntrospect struct TimelineView: View { @@ -40,14 +39,12 @@ struct TimelineView: View { .hidden() .frame(height: 0) - VStack(spacing: 0) { - ForEach(viewState.timelineIDs.reversed(), id: \.self) { id in - if let viewModel = viewState.itemsDictionary[id] { - RoomTimelineItemView(viewModel: viewModel) - .frame(maxWidth: .infinity, alignment: .leading) - .scaleEffect(x: 1, y: -1) - .padding(timelineStyle.rowInsets) - } + LazyVStack(spacing: 0) { + ForEach(viewState.itemViewModels.reversed()) { viewModel in + RoomTimelineItemView(viewModel: viewModel) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(timelineStyle.rowInsets) + .scaleEffect(x: 1, y: -1) } } } @@ -55,7 +52,7 @@ struct TimelineView: View { guard scrollView != scrollViewAdapter.scrollView else { return } scrollViewAdapter.scrollView = scrollView } - .animation(.elementDefault, value: viewState.timelineIDs) + .animation(.elementDefault, value: viewState.itemViewModels) .scaleEffect(x: 1, y: -1) .onReceive(scrollViewAdapter.didScroll) { _ in guard let scrollView = scrollViewAdapter.scrollView else { From cb49a7e0dbedfd79e7a19e9d332c604fc8a70ce4 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Mon, 24 Jul 2023 22:15:08 +0200 Subject: [PATCH 08/21] everything implemented --- .../Sources/Other/ScrollViewAdapter.swift | 12 ++- .../Screens/RoomScreen/RoomScreenModels.swift | 1 - .../RoomScreen/RoomScreenViewModel.swift | 46 +++++------ .../RoomScreen/View/TimelineView.swift | 80 +++++++++++++------ .../TimelineItems/RoomTimelineItemView.swift | 4 +- .../RoomTimelineItemViewModel.swift | 25 +++--- 6 files changed, 99 insertions(+), 69 deletions(-) diff --git a/ElementX/Sources/Other/ScrollViewAdapter.swift b/ElementX/Sources/Other/ScrollViewAdapter.swift index bdb8d395e0..1256d3ea57 100644 --- a/ElementX/Sources/Other/ScrollViewAdapter.swift +++ b/ElementX/Sources/Other/ScrollViewAdapter.swift @@ -29,7 +29,9 @@ class ScrollViewAdapter: NSObject, UIScrollViewDelegate { scrollView?.delegate = self } } - + + var shouldScrollToTopClosure: ((UIScrollView) -> Bool)? + private let didScrollSubject = PassthroughSubject() var didScroll: AnyPublisher { didScrollSubject.eraseToAnyPublisher() @@ -73,6 +75,14 @@ class ScrollViewAdapter: NSObject, UIScrollViewDelegate { func scrollViewDidScrollToTop(_ scrollView: UIScrollView) { updateDidScroll(scrollView) } + + func scrollViewShouldScrollToTop(_ scrollView: UIScrollView) -> Bool { + guard let shouldScrollToTopClosure else { + // Default behaviour + return true + } + return shouldScrollToTopClosure(scrollView) + } // MARK: - Private diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift index 003475c1ab..b446e9f906 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift @@ -51,7 +51,6 @@ enum RoomScreenComposerMode: Equatable { enum RoomScreenViewAction { case displayRoomDetails - case paginateBackwards case itemAppeared(itemID: TimelineItemIdentifier) case itemDisappeared(itemID: TimelineItemIdentifier) case itemTapped(itemID: TimelineItemIdentifier) diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index d58c8852d8..bb013654f2 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -62,10 +62,9 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol imageProvider: mediaProvider) state.timelineViewState.paginateAction = { [weak self] in - Task { - await self?.paginateBackwards() - } + self?.paginateBackwards() } + setupSubscriptions() state.timelineItemMenuActionProvider = { [weak self] itemId -> TimelineItemMenuActions? in @@ -90,8 +89,6 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol switch viewAction { case .displayRoomDetails: callback?(.displayRoomDetails) - case .paginateBackwards: - Task { await paginateBackwards() } case .itemAppeared(let id): Task { await timelineController.processItemAppearance(id) } case .itemDisappeared(let id): @@ -221,12 +218,21 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol .store(in: &cancellables) } - private func paginateBackwards() async { - switch await timelineController.paginateBackwards(requestSize: Constants.backPaginationEventLimit, untilNumberOfItems: Constants.backPaginationPageSize) { - case .failure: - displayError(.toast(L10n.errorFailedLoadingMessages)) - default: - break + private var paginateBackwardsTask: Task? + + func paginateBackwards() { + guard paginateBackwardsTask == nil else { + return + } + + paginateBackwardsTask = Task { + switch await timelineController.paginateBackwards(requestSize: Constants.backPaginationEventLimit, untilNumberOfItems: Constants.backPaginationPageSize) { + case .failure: + displayError(.toast(L10n.errorFailedLoadingMessages)) + default: + break + } + paginateBackwardsTask = nil } } @@ -265,19 +271,19 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol if itemGroup.count == 1 { if let firstItem = itemGroup.first { - timelineItemsDictionary.updateValue(updateViewModel(item: firstItem, groupStyle: .single), + timelineItemsDictionary.updateValue(RoomTimelineItemViewModel(item: firstItem, groupStyle: .single), forKey: firstItem.id.timelineID) } } else { for (index, item) in itemGroup.enumerated() { if index == 0 { - timelineItemsDictionary.updateValue(updateViewModel(item: item, groupStyle: .first), + timelineItemsDictionary.updateValue(RoomTimelineItemViewModel(item: item, groupStyle: .first), forKey: item.id.timelineID) } else if index == itemGroup.count - 1 { - timelineItemsDictionary.updateValue(updateViewModel(item: item, groupStyle: .last), + timelineItemsDictionary.updateValue(RoomTimelineItemViewModel(item: item, groupStyle: .last), forKey: item.id.timelineID) } else { - timelineItemsDictionary.updateValue(updateViewModel(item: item, groupStyle: .middle), + timelineItemsDictionary.updateValue(RoomTimelineItemViewModel(item: item, groupStyle: .middle), forKey: item.id.timelineID) } } @@ -287,16 +293,6 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol state.timelineViewState.itemsDictionary = timelineItemsDictionary } - private func updateViewModel(item: RoomTimelineItemProtocol, groupStyle: TimelineGroupStyle) -> RoomTimelineItemViewModel { - if let timelineItemViewModel = state.timelineViewState.itemsDictionary[item.id.timelineID] { - timelineItemViewModel.groupStyle = groupStyle - timelineItemViewModel.type = .init(item: item) - return timelineItemViewModel - } else { - return RoomTimelineItemViewModel(item: item, groupStyle: groupStyle) - } - } - private func canGroupItem(timelineItem: RoomTimelineItemProtocol, with otherTimelineItem: RoomTimelineItemProtocol) -> Bool { if timelineItem is CollapsibleTimelineItem || otherTimelineItem is CollapsibleTimelineItem { return false diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift index 717ff5edb6..16f2570b64 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift @@ -24,6 +24,7 @@ struct TimelineView: View { @Environment(\.timelineStyle) private var timelineStyle private let bottomID = "bottomID" + private let topID = "topID" @State private var scrollViewAdapter = ScrollViewAdapter() @State private var paginateBackwardsPublisher = PassthroughSubject() @@ -33,11 +34,7 @@ struct TimelineView: View { var body: some View { ScrollViewReader { scrollView in ScrollView { - // Only used ton get to the bottom of the scroll view - Divider() - .id(bottomID) - .hidden() - .frame(height: 0) + bottomPin LazyVStack(spacing: 0) { ForEach(viewState.itemViewModels.reversed()) { viewModel in @@ -47,34 +44,71 @@ struct TimelineView: View { .scaleEffect(x: 1, y: -1) } } + + topPin } - .introspect(.scrollView, on: .iOS(.v16)) { scrollView in - guard scrollView != scrollViewAdapter.scrollView else { return } - scrollViewAdapter.scrollView = scrollView - } - .animation(.elementDefault, value: viewState.itemViewModels) - .scaleEffect(x: 1, y: -1) - .onReceive(scrollViewAdapter.didScroll) { _ in - guard let scrollView = scrollViewAdapter.scrollView else { - return - } - let offset = scrollView.contentOffset.y + scrollView.contentInset.top - let scrollToBottomButtonVisibleValue = offset > 0 - if scrollToBottomButtonVisibleValue != scrollToBottomButtonVisible { - scrollToBottomButtonVisible = scrollToBottomButtonVisibleValue + .introspect(.scrollView, on: .iOS(.v16)) { uiScrollView in + guard uiScrollView != scrollViewAdapter.scrollView else { return } + scrollViewAdapter.scrollView = uiScrollView + scrollViewAdapter.shouldScrollToTopClosure = { _ in + withAnimation { + scrollView.scrollTo(topID) + } + return false } - paginateBackwardsPublisher.send() + + // Allows the scroll to top to work properly + uiScrollView.contentOffset.y -= 1 } + .scaleEffect(x: 1, y: -1) .onReceive(scrollToBottomPublisher) { _ in withAnimation { scrollView.scrollTo(bottomID) } } - .onReceive(paginateBackwardsPublisher.collect(.byTime(DispatchQueue.main, 0.1))) { _ in - tryPaginateBackwards() - } + .scrollDismissesKeyboard(.interactively) } .overlay(scrollToBottomButton, alignment: .bottomTrailing) + .animation(.elementDefault, value: viewState.itemViewModels) + .onReceive(scrollViewAdapter.didScroll) { _ in + guard let scrollView = scrollViewAdapter.scrollView else { + return + } + let offset = scrollView.contentOffset.y + scrollView.contentInset.top + let scrollToBottomButtonVisibleValue = offset > 0 + if scrollToBottomButtonVisibleValue != scrollToBottomButtonVisible { + scrollToBottomButtonVisible = scrollToBottomButtonVisibleValue + } + paginateBackwardsPublisher.send() + + // Allows the scroll to top to work properly + if offset == 0 { + scrollView.contentOffset.y -= 1 + } + } + .onReceive(paginateBackwardsPublisher.collect(.byTime(DispatchQueue.main, 0.1))) { _ in + tryPaginateBackwards() + } + .onAppear { + paginateBackwardsPublisher.send() + guard let scrollView = scrollViewAdapter.scrollView else { + return + } + } + } + + private var topPin: some View { + Divider() + .id(topID) + .hidden() + .frame(height: 0) + } + + private var bottomPin: some View { + Divider() + .id(bottomID) + .hidden() + .frame(height: 0) } private var scrollToBottomButton: some View { diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift index a7d2dd1904..6ff505711f 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemView.swift @@ -17,14 +17,12 @@ import SwiftUI struct RoomTimelineItemView: View { @EnvironmentObject private var context: RoomScreenViewModel.Context - @ObservedObject var viewModel: RoomTimelineItemViewModel + var viewModel: RoomTimelineItemViewModel var body: some View { timelineView .environmentObject(context) .environment(\.timelineGroupStyle, viewModel.groupStyle) - .animation(.elementDefault, value: viewModel.type) - .animation(.elementDefault, value: viewModel.groupStyle) .onAppear { context.send(viewAction: .itemAppeared(itemID: viewModel.identifier)) } diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewModel.swift b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewModel.swift index c3cc12e9b6..1c5e9a9462 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewModel.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewModel.swift @@ -16,13 +16,9 @@ import Foundation -final class RoomTimelineItemViewModel: Identifiable, Equatable, ObservableObject { - static func == (lhs: RoomTimelineItemViewModel, rhs: RoomTimelineItemViewModel) -> Bool { - lhs.type == rhs.type && lhs.groupStyle == rhs.groupStyle - } - - @Published var type: RoomTimelineItemType - @Published var groupStyle: TimelineGroupStyle +struct RoomTimelineItemViewModel: Identifiable, Equatable { + var type: RoomTimelineItemType + var groupStyle: TimelineGroupStyle var identifier: TimelineItemIdentifier { type.id @@ -32,20 +28,17 @@ final class RoomTimelineItemViewModel: Identifiable, Equatable, ObservableObject identifier.timelineID } - convenience init(item: RoomTimelineItemProtocol, groupStyle: TimelineGroupStyle) { - self.init(type: .init(item: item), groupStyle: groupStyle) - } - - init(type: RoomTimelineItemType, groupStyle: TimelineGroupStyle) { - self.type = type - self.groupStyle = groupStyle - } - var isReactable: Bool { type.isReactable } } +extension RoomTimelineItemViewModel { + init(item: RoomTimelineItemProtocol, groupStyle: TimelineGroupStyle) { + self.init(type: .init(item: item), groupStyle: groupStyle) + } +} + enum RoomTimelineItemType: Equatable { case text(TextRoomTimelineItem) case separator(SeparatorRoomTimelineItem) From 3f1db81351e8bed787018d330ba751056403b7b8 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Mon, 24 Jul 2023 22:20:35 +0200 Subject: [PATCH 09/21] rebase fix --- ElementX.xcodeproj/project.pbxproj | 6 +----- ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift | 1 - .../Sources/Screens/RoomScreen/RoomScreenViewModel.swift | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj index 0017b88085..d0962e0d20 100644 --- a/ElementX.xcodeproj/project.pbxproj +++ b/ElementX.xcodeproj/project.pbxproj @@ -296,7 +296,6 @@ 6F2D5D4F2590310DFAE973E4 /* WaitingDialog.swift in Sources */ = {isa = PBXBuildFile; fileRef = F6D698BFD68B061350553930 /* WaitingDialog.swift */; }; 6FC10A00D268FCD48B631E37 /* ViewFrameReader.swift in Sources */ = {isa = PBXBuildFile; fileRef = EFF7BF82A950B91BC5469E91 /* ViewFrameReader.swift */; }; 6FF51EB400DBA0668FC38B97 /* TimelineStartRoomTimelineView.swift in Sources */ = {isa = PBXBuildFile; fileRef = F9ED8E731E21055F728E5FED /* TimelineStartRoomTimelineView.swift */; }; - 702694459B649B9D3A3C34F8 /* TimelineTableViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = F9212AE02CBDD692C56A879F /* TimelineTableViewController.swift */; }; 70394ECD2DCC70741538620D /* AccessibilityIdentifiers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 04BB8DDE245ED86C489BA983 /* AccessibilityIdentifiers.swift */; }; 70558528EF68CAAEF09972D5 /* RoomTimelineItemFixtures.swift in Sources */ = {isa = PBXBuildFile; fileRef = E96ED747FF90332EA1333C22 /* RoomTimelineItemFixtures.swift */; }; 706289B086B0A6B0C211763F /* UITestsSignalling.swift in Sources */ = {isa = PBXBuildFile; fileRef = B7F0192CE2F891141A25B49F /* UITestsSignalling.swift */; }; @@ -1472,7 +1471,6 @@ F875D71347DC81EAE7687446 /* NavigationRootCoordinatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NavigationRootCoordinatorTests.swift; sourceTree = ""; }; F899D02CF26EA7675EEBE74C /* UserSessionScreenTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserSessionScreenTests.swift; sourceTree = ""; }; F8CEB4634C0DD7779C4AB504 /* CreateRoomScreenUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreateRoomScreenUITests.swift; sourceTree = ""; }; - F9212AE02CBDD692C56A879F /* TimelineTableViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineTableViewController.swift; sourceTree = ""; }; F9E785D5137510481733A3E8 /* TextRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TextRoomTimelineView.swift; sourceTree = ""; }; F9ED8E731E21055F728E5FED /* TimelineStartRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineStartRoomTimelineView.swift; sourceTree = ""; }; FA7D3C686C214470F4911618 /* RoomTimelineItemViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomTimelineItemViewModel.swift; sourceTree = ""; }; @@ -2590,7 +2588,6 @@ 4A4AD793D50748F8997E5B15 /* TimelineItemMacContextMenu.swift */, A1BF12A5E7C76777C4BF0F2B /* TimelineItemMenu.swift */, 0BC588051E6572A1AF51D738 /* TimelineSenderAvatarView.swift */, - F9212AE02CBDD692C56A879F /* TimelineTableViewController.swift */, 874A1842477895F199567BD7 /* TimelineView.swift */, 1D8572B713A11CFDBF009B2F /* Replies */, A312471EA62EFB0FD94E60DC /* Style */, @@ -3627,7 +3624,7 @@ path = Timeline; sourceTree = ""; }; - "TEMP_62CE5EF1-8768-4933-9547-E5F60DAC11F4" /* element-x-ios */ = { + "TEMP_3919F537-58A5-4700-B869-24AAD8D8DD61" /* element-x-ios */ = { isa = PBXGroup; children = ( 41553551C55AD59885840F0E /* secrets.xcconfig */, @@ -4631,7 +4628,6 @@ 6FF51EB400DBA0668FC38B97 /* TimelineStartRoomTimelineView.swift in Sources */, 69BCBB4FB2DC3D61A28D3FD8 /* TimelineStyle.swift in Sources */, FFD3E4FF948E06C7585317FC /* TimelineStyler.swift in Sources */, - 702694459B649B9D3A3C34F8 /* TimelineTableViewController.swift in Sources */, 500CB65ED116B81DA52FDAEE /* TimelineView.swift in Sources */, 36AC963F2F04069B7FF1AA0C /* UIConstants.swift in Sources */, A37EED79941AD3B7140B3822 /* UIDevice.swift in Sources */, diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift index b446e9f906..25d259fc67 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift @@ -92,7 +92,6 @@ struct RoomScreenViewState: BindableState { var isEncryptedOneToOneRoom = false let timelineViewState: TimelineViewState - var composerMode: RoomScreenComposerMode = .default let scrollToBottomPublisher = PassthroughSubject() diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index bb013654f2..4b06f71a91 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -668,7 +668,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol // MARK: - Reactions private func showEmojiPicker(for itemID: TimelineItemIdentifier) { - guard let item = state.itemsDictionary[itemID.timelineID], item.isReactable else { return } + guard let item = state.timelineViewState.itemsDictionary[itemID.timelineID], item.isReactable else { return } callback?(.displayEmojiPicker(itemID: itemID)) } From 2646230a24986613490e4c30fd83f0a751fba479 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Mon, 24 Jul 2023 22:38:37 +0200 Subject: [PATCH 10/21] fix --- .../RoomScreen/View/Style/TimelineItemBubbledStylerView.swift | 2 +- .../RoomScreen/View/Style/TimelineItemPlainStylerView.swift | 2 +- .../View/Supplementary/TimelineItemStatusView.swift | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift index d61b1e895c..fb0720e884 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift @@ -70,7 +70,7 @@ struct TimelineItemBubbledStylerView: View { if !timelineItem.isOutgoing { Spacer() } - TimelineItemStatusView(timelineItem: timelineItem) + TimelineItemStatusView(timelineItem: timelineItem, timelineViewState: context.viewState.timelineViewState) .environmentObject(context) .padding(.top, 8) .padding(.bottom, 3) diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift index 9474b466c2..03d31ce94e 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift @@ -42,7 +42,7 @@ struct TimelineItemPlainStylerView: View { supplementaryViews } } - TimelineItemStatusView(timelineItem: timelineItem) + TimelineItemStatusView(timelineItem: timelineItem, timelineViewState: context.viewState.timelineViewState) .environmentObject(context) } } diff --git a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift index 33686690e9..8e8625a817 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift @@ -21,10 +21,10 @@ struct TimelineItemStatusView: View { @Environment(\.timelineStyle) private var style @Environment(\.readReceiptsEnabled) private var readReceiptsEnabled @EnvironmentObject private var context: RoomScreenViewModel.Context + @ObservedObject var timelineViewState: TimelineViewState private var isLastOutgoingMessage: Bool { - context.viewState.timelineViewState.timelineIDs.last == timelineItem.id.timelineID && - timelineItem.isOutgoing + timelineItem.isOutgoing && timelineViewState.timelineIDs.last == timelineItem.id.timelineID } var body: some View { From c9f3f4eddd6bb924645b865c5f9b48fcaf7a55dc Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Mon, 24 Jul 2023 22:40:22 +0200 Subject: [PATCH 11/21] doc --- .../RoomScreen/View/Supplementary/TimelineItemStatusView.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift index 8e8625a817..8e022c126d 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift @@ -21,6 +21,8 @@ struct TimelineItemStatusView: View { @Environment(\.timelineStyle) private var style @Environment(\.readReceiptsEnabled) private var readReceiptsEnabled @EnvironmentObject private var context: RoomScreenViewModel.Context + + // Required since the timelineViewState is a reference and its changes are not observed by the context @ObservedObject var timelineViewState: TimelineViewState private var isLastOutgoingMessage: Bool { From 1a9350e5c6b515bb63bd69242f649e3f6dd95c24 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 25 Jul 2023 01:02:54 +0200 Subject: [PATCH 12/21] fix test compilation --- .../Sources/RoomScreenViewModelTests.swift | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/UnitTests/Sources/RoomScreenViewModelTests.swift b/UnitTests/Sources/RoomScreenViewModelTests.swift index 6cefb3e0a6..c0bf9754e1 100644 --- a/UnitTests/Sources/RoomScreenViewModelTests.swift +++ b/UnitTests/Sources/RoomScreenViewModelTests.swift @@ -51,9 +51,9 @@ class RoomScreenViewModelTests: XCTestCase { userIndicatorController: userIndicatorControllerMock) // Then the messages should be grouped together. - XCTAssertEqual(viewModel.state.itemViewModels[0].groupStyle, .first, "Nothing should prevent the first message from being grouped.") - XCTAssertEqual(viewModel.state.itemViewModels[1].groupStyle, .middle, "Nothing should prevent the middle message from being grouped.") - XCTAssertEqual(viewModel.state.itemViewModels[2].groupStyle, .last, "Nothing should prevent the last message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[0].groupStyle, .first, "Nothing should prevent the first message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[1].groupStyle, .middle, "Nothing should prevent the middle message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[2].groupStyle, .last, "Nothing should prevent the last message from being grouped.") } func testMessageGroupingMultipleSenders() { @@ -84,12 +84,12 @@ class RoomScreenViewModelTests: XCTestCase { userIndicatorController: userIndicatorControllerMock) // Then the messages should be grouped by sender. - XCTAssertEqual(viewModel.state.itemViewModels[0].groupStyle, .single, "A message should not be grouped when the sender changes.") - XCTAssertEqual(viewModel.state.itemViewModels[1].groupStyle, .single, "A message should not be grouped when the sender changes.") - XCTAssertEqual(viewModel.state.itemViewModels[2].groupStyle, .first, "A group should start with a new sender if there are more messages from that sender.") - XCTAssertEqual(viewModel.state.itemViewModels[3].groupStyle, .last, "A group should be ended when the sender changes in the next message.") - XCTAssertEqual(viewModel.state.itemViewModels[4].groupStyle, .first, "A group should start with a new sender if there are more messages from that sender.") - XCTAssertEqual(viewModel.state.itemViewModels[5].groupStyle, .last, "A group should be ended when the sender changes in the next message.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[0].groupStyle, .single, "A message should not be grouped when the sender changes.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[1].groupStyle, .single, "A message should not be grouped when the sender changes.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[2].groupStyle, .first, "A group should start with a new sender if there are more messages from that sender.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[3].groupStyle, .last, "A group should be ended when the sender changes in the next message.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[4].groupStyle, .first, "A group should start with a new sender if there are more messages from that sender.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[5].groupStyle, .last, "A group should be ended when the sender changes in the next message.") } func testMessageGroupingWithLeadingReactions() { @@ -115,9 +115,9 @@ class RoomScreenViewModelTests: XCTestCase { userIndicatorController: userIndicatorControllerMock) // Then the first message should not be grouped but the other two should. - XCTAssertEqual(viewModel.state.itemViewModels[0].groupStyle, .single, "When the first message has reactions it should not be grouped.") - XCTAssertEqual(viewModel.state.itemViewModels[1].groupStyle, .first, "A new group should be made when the preceding message has reactions.") - XCTAssertEqual(viewModel.state.itemViewModels[2].groupStyle, .last, "Nothing should prevent the last message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[0].groupStyle, .single, "When the first message has reactions it should not be grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[1].groupStyle, .first, "A new group should be made when the preceding message has reactions.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[2].groupStyle, .last, "Nothing should prevent the last message from being grouped.") } func testMessageGroupingWithInnerReactions() { @@ -143,9 +143,9 @@ class RoomScreenViewModelTests: XCTestCase { userIndicatorController: userIndicatorControllerMock) // Then the first and second messages should be grouped and the last one should not. - XCTAssertEqual(viewModel.state.itemViewModels[0].groupStyle, .first, "Nothing should prevent the first message from being grouped.") - XCTAssertEqual(viewModel.state.itemViewModels[1].groupStyle, .last, "When the message has reactions, the group should end here.") - XCTAssertEqual(viewModel.state.itemViewModels[2].groupStyle, .single, "The last message should not be grouped when the preceding message has reactions.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[0].groupStyle, .first, "Nothing should prevent the first message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[1].groupStyle, .last, "When the message has reactions, the group should end here.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[2].groupStyle, .single, "The last message should not be grouped when the preceding message has reactions.") } func testMessageGroupingWithTrailingReactions() { @@ -171,9 +171,9 @@ class RoomScreenViewModelTests: XCTestCase { userIndicatorController: userIndicatorControllerMock) // Then the messages should be grouped together. - XCTAssertEqual(viewModel.state.itemViewModels[0].groupStyle, .first, "Nothing should prevent the first message from being grouped.") - XCTAssertEqual(viewModel.state.itemViewModels[1].groupStyle, .middle, "Nothing should prevent the second message from being grouped.") - XCTAssertEqual(viewModel.state.itemViewModels[2].groupStyle, .last, "Reactions on the last message should not prevent it from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[0].groupStyle, .first, "Nothing should prevent the first message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[1].groupStyle, .middle, "Nothing should prevent the second message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[2].groupStyle, .last, "Reactions on the last message should not prevent it from being grouped.") } func testGoToUserDetailsSuccessNoDelay() async { From 66a3d1605c569bc97dc45387dba9cf1a92383fb5 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 25 Jul 2023 01:42:48 +0200 Subject: [PATCH 13/21] code improvement and animation improvement --- .../Sources/Screens/RoomScreen/RoomScreenModels.swift | 10 +++++----- .../View/Style/TimelineItemBubbledStylerView.swift | 2 +- .../View/Style/TimelineItemPlainStylerView.swift | 2 +- .../View/Supplementary/TimelineItemStatusView.swift | 5 +---- .../View/Timeline/CollapsibleRoomTimelineView.swift | 11 ++++++----- .../Screens/RoomScreen/View/TimelineView.swift | 2 +- 6 files changed, 15 insertions(+), 17 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift index 25d259fc67..8a12063855 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift @@ -90,7 +90,7 @@ struct RoomScreenViewState: BindableState { var timelineStyle: TimelineStyle var readReceiptsEnabled: Bool var isEncryptedOneToOneRoom = false - let timelineViewState: TimelineViewState + var timelineViewState: TimelineViewState var composerMode: RoomScreenComposerMode = .default let scrollToBottomPublisher = PassthroughSubject() @@ -174,10 +174,10 @@ struct RoomMemberState { let avatarURL: URL? } -final class TimelineViewState: ObservableObject { - @Published var canBackPaginate = false - @Published var isBackPaginating = false - @Published var itemsDictionary = OrderedDictionary() +struct TimelineViewState { + var canBackPaginate = false + var isBackPaginating = false + var itemsDictionary = OrderedDictionary() var timelineIDs: [String] { itemsDictionary.keys.elements diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift index fb0720e884..d61b1e895c 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift @@ -70,7 +70,7 @@ struct TimelineItemBubbledStylerView: View { if !timelineItem.isOutgoing { Spacer() } - TimelineItemStatusView(timelineItem: timelineItem, timelineViewState: context.viewState.timelineViewState) + TimelineItemStatusView(timelineItem: timelineItem) .environmentObject(context) .padding(.top, 8) .padding(.bottom, 3) diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift index 03d31ce94e..9474b466c2 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift @@ -42,7 +42,7 @@ struct TimelineItemPlainStylerView: View { supplementaryViews } } - TimelineItemStatusView(timelineItem: timelineItem, timelineViewState: context.viewState.timelineViewState) + TimelineItemStatusView(timelineItem: timelineItem) .environmentObject(context) } } diff --git a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift index 8e022c126d..56ebac1770 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineItemStatusView.swift @@ -21,12 +21,9 @@ struct TimelineItemStatusView: View { @Environment(\.timelineStyle) private var style @Environment(\.readReceiptsEnabled) private var readReceiptsEnabled @EnvironmentObject private var context: RoomScreenViewModel.Context - - // Required since the timelineViewState is a reference and its changes are not observed by the context - @ObservedObject var timelineViewState: TimelineViewState private var isLastOutgoingMessage: Bool { - timelineItem.isOutgoing && timelineViewState.timelineIDs.last == timelineItem.id.timelineID + timelineItem.isOutgoing && context.viewState.timelineViewState.timelineIDs.last == timelineItem.id.timelineID } var body: some View { diff --git a/ElementX/Sources/Screens/RoomScreen/View/Timeline/CollapsibleRoomTimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/Timeline/CollapsibleRoomTimelineView.swift index 65b1be12a2..069b802c23 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Timeline/CollapsibleRoomTimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Timeline/CollapsibleRoomTimelineView.swift @@ -33,19 +33,20 @@ struct CollapsibleRoomTimelineView: View { ForEach(timelineViewModels) { viewModel in RoomTimelineItemView(viewModel: viewModel) } - }.transition(.opacity.animation(.elementDefault)) + } } .disclosureGroupStyle(CollapsibleRoomTimelineItemDisclosureGroupStyle()) - .transaction { transaction in - transaction.animation = .noAnimation // Fixes weird animations on the disclosure indicator - } } } private struct CollapsibleRoomTimelineItemDisclosureGroupStyle: DisclosureGroupStyle { func makeBody(configuration: Configuration) -> some View { VStack(spacing: 0.0) { - Button { configuration.isExpanded.toggle() } label: { + Button { + withAnimation { + configuration.isExpanded.toggle() + } + } label: { HStack(alignment: .center) { configuration.label Text(Image(systemName: "chevron.forward")) diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift index 16f2570b64..106e745448 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift @@ -20,7 +20,7 @@ import SwiftUI import SwiftUIIntrospect struct TimelineView: View { - @ObservedObject var viewState: TimelineViewState + let viewState: TimelineViewState @Environment(\.timelineStyle) private var timelineStyle private let bottomID = "bottomID" From 72b3349949d6f8089ef88214e7e0265904dc57a8 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 25 Jul 2023 01:50:31 +0200 Subject: [PATCH 14/21] code improvement --- ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift | 2 +- ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift index 8a12063855..3e0a953a52 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift @@ -90,7 +90,7 @@ struct RoomScreenViewState: BindableState { var timelineStyle: TimelineStyle var readReceiptsEnabled: Bool var isEncryptedOneToOneRoom = false - var timelineViewState: TimelineViewState + var timelineViewState = TimelineViewState() var composerMode: RoomScreenComposerMode = .default let scrollToBottomPublisher = PassthroughSubject() diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index 4b06f71a91..6e1dfe3679 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -57,7 +57,6 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol timelineStyle: appSettings.timelineStyle, readReceiptsEnabled: appSettings.readReceiptsEnabled, isEncryptedOneToOneRoom: roomProxy.isEncryptedOneToOneRoom, - timelineViewState: .init(), bindings: .init(composerText: "", composerFocused: false, reactionsCollapsed: [:])), imageProvider: mediaProvider) From a3e5e3aa0202c166e135a280c846b8a2200d44f0 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 25 Jul 2023 02:13:30 +0200 Subject: [PATCH 15/21] code improvements --- ElementX.xcodeproj/project.pbxproj | 32 +++++++------------ .../Screens/RoomScreen/RoomScreenModels.swift | 4 +-- .../RoomScreen/RoomScreenViewModel.swift | 10 +++--- .../Style/TimelineItemBubbledStylerView.swift | 8 ++--- .../Style/TimelineItemPlainStylerView.swift | 2 +- .../CollapsibleRoomTimelineView.swift | 8 ++--- .../Timeline/ReadMarkerRoomTimelineView.swift | 8 ++--- .../RoomScreen/View/TimelineView.swift | 22 +++++++------ .../TimelineItems/RoomTimelineItemView.swift | 12 +++---- ....swift => RoomTimelineItemViewState.swift} | 8 ++--- 10 files changed, 54 insertions(+), 60 deletions(-) rename ElementX/Sources/Services/Timeline/TimelineItems/{RoomTimelineItemViewModel.swift => RoomTimelineItemViewState.swift} (96%) diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj index d0962e0d20..cd83482d1d 100644 --- a/ElementX.xcodeproj/project.pbxproj +++ b/ElementX.xcodeproj/project.pbxproj @@ -3,7 +3,7 @@ archiveVersion = 1; classes = { }; - objectVersion = 51; + objectVersion = 54; objects = { /* Begin PBXBuildFile section */ @@ -86,7 +86,7 @@ 206F0DBAB6AF042CA1FF2C0D /* SettingsViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D487C1185D658F8B15B8F55 /* SettingsViewModelTests.swift */; }; 208C19811613F9A10F8A7B75 /* MediaLoader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AFCE895ECFFA53FEE64D62B /* MediaLoader.swift */; }; 2185C1F6724C78FFF355D6FA /* WelcomeScreenScreenUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AB10FA6570DD08B3966C159 /* WelcomeScreenScreenUITests.swift */; }; - 2198B4458AFF69102BBCC370 /* RoomTimelineItemViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA7D3C686C214470F4911618 /* RoomTimelineItemViewModel.swift */; }; + 2198B4458AFF69102BBCC370 /* RoomTimelineItemViewState.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA7D3C686C214470F4911618 /* RoomTimelineItemViewState.swift */; }; 21BF2B7CEDFE3CA67C5355AD /* test_image.png in Resources */ = {isa = PBXBuildFile; fileRef = C733D11B421CFE3A657EF230 /* test_image.png */; }; 22882C710BC99EC34A5024A0 /* UITestsScreenIdentifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6CEBE5EA91E8691EDF364EC2 /* UITestsScreenIdentifier.swift */; }; 2352C541AF857241489756FF /* MockRoomSummaryProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8F7D42E66E939B709C1EC390 /* MockRoomSummaryProvider.swift */; }; @@ -857,7 +857,7 @@ 127C8472672A5BA09EF1ACF8 /* CurrentValuePublisher.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CurrentValuePublisher.swift; sourceTree = ""; }; 12EDAFB64FA5F6812D54F39A /* MigrationScreenViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MigrationScreenViewModel.swift; sourceTree = ""; }; 12F1E7F9C2BE8BB751037826 /* WaitlistScreenCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WaitlistScreenCoordinator.swift; sourceTree = ""; }; - 1304D9191300873EADA52D6E /* IntegrationTests.xctestplan */ = {isa = PBXFileReference; path = IntegrationTests.xctestplan; sourceTree = ""; }; + 1304D9191300873EADA52D6E /* IntegrationTests.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = IntegrationTests.xctestplan; sourceTree = ""; }; 130ED565A078F7E0B59D9D25 /* UNTextInputNotificationResponse+Creator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UNTextInputNotificationResponse+Creator.swift"; sourceTree = ""; }; 13802897C7AFA360EA74C0B0 /* en */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = en; path = en.lproj/Localizable.stringsdict; sourceTree = ""; }; 1423AB065857FA546444DB15 /* NotificationManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationManager.swift; sourceTree = ""; }; @@ -998,7 +998,7 @@ 47111410B6E659A697D472B5 /* RoomProxyProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomProxyProtocol.swift; sourceTree = ""; }; 471EB7D96AFEA8D787659686 /* EmoteRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EmoteRoomTimelineView.swift; sourceTree = ""; }; 47873756E45B46683D97DC32 /* LegalInformationScreenModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LegalInformationScreenModels.swift; sourceTree = ""; }; - 478BE8591BD13E908EF70C0C /* DesignKit */ = {isa = PBXFileReference; lastKnownFileType = folder; name = DesignKit; path = DesignKit; sourceTree = SOURCE_ROOT; }; + 478BE8591BD13E908EF70C0C /* DesignKit */ = {isa = PBXFileReference; lastKnownFileType = folder; path = DesignKit; sourceTree = SOURCE_ROOT; }; 4798B3B7A1E8AE3901CEE8C6 /* FramePreferenceKey.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FramePreferenceKey.swift; sourceTree = ""; }; 47EBB5D698CE9A25BB553A2D /* Strings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Strings.swift; sourceTree = ""; }; 47F29139BC2A804CE5E0757E /* MediaUploadPreviewScreenViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MediaUploadPreviewScreenViewModel.swift; sourceTree = ""; }; @@ -1175,7 +1175,7 @@ 8D55702474F279D910D2D162 /* RoomStateEventStringBuilder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomStateEventStringBuilder.swift; sourceTree = ""; }; 8D8169443E5AC5FF71BFB3DB /* cs */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = cs; path = cs.lproj/Localizable.strings; sourceTree = ""; }; 8DC2C9E0E15C79BBDA80F0A2 /* TimelineStyle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineStyle.swift; sourceTree = ""; }; - 8E088F2A1B9EC529D3221931 /* UITests.xctestplan */ = {isa = PBXFileReference; path = UITests.xctestplan; sourceTree = ""; }; + 8E088F2A1B9EC529D3221931 /* UITests.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = UITests.xctestplan; sourceTree = ""; }; 8E1BBA73B611EDEEA6E20E05 /* InvitesScreenModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InvitesScreenModels.swift; sourceTree = ""; }; 8EC57A32ABC80D774CC663DB /* SettingsScreenUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsScreenUITests.swift; sourceTree = ""; }; 8F21ED7205048668BEB44A38 /* AppActivityView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppActivityView.swift; sourceTree = ""; }; @@ -1285,7 +1285,7 @@ B4CFE236419E830E8946639C /* Analytics+SwiftUI.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Analytics+SwiftUI.swift"; sourceTree = ""; }; B590BD4507D4F0A377FDE01A /* LoadableAvatarImage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoadableAvatarImage.swift; sourceTree = ""; }; B5B243E7818E5E9F6A4EDC7A /* NoticeRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NoticeRoomTimelineView.swift; sourceTree = ""; }; - B61C339A2FDDBD067FF6635C /* ConfettiScene.scn */ = {isa = PBXFileReference; path = ConfettiScene.scn; sourceTree = ""; }; + B61C339A2FDDBD067FF6635C /* ConfettiScene.scn */ = {isa = PBXFileReference; lastKnownFileType = file.bplist; path = ConfettiScene.scn; sourceTree = ""; }; B6311F21F911E23BE4DF51B4 /* ReadMarkerRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReadMarkerRoomTimelineView.swift; sourceTree = ""; }; B697816AF93DA06EC58C5D70 /* WaitlistScreenViewModelProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WaitlistScreenViewModelProtocol.swift; sourceTree = ""; }; B6E89E530A8E92EC44301CA1 /* Bundle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Bundle.swift; sourceTree = ""; }; @@ -1365,7 +1365,7 @@ CD6B0C4639E066915B5E6463 /* target.yml */ = {isa = PBXFileReference; lastKnownFileType = text.yaml; path = target.yml; sourceTree = ""; }; CDB3227C7A74B734924942E9 /* RoomSummaryProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomSummaryProvider.swift; sourceTree = ""; }; CEE0E6043EFCF6FD2A341861 /* TimelineReplyView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineReplyView.swift; sourceTree = ""; }; - CEE41494C837AA403A06A5D9 /* UnitTests.xctestplan */ = {isa = PBXFileReference; path = UnitTests.xctestplan; sourceTree = ""; }; + CEE41494C837AA403A06A5D9 /* UnitTests.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = UnitTests.xctestplan; sourceTree = ""; }; CF48AF076424DBC1615C74AD /* AuthenticationServiceProxy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AuthenticationServiceProxy.swift; sourceTree = ""; }; D0140615D2232612C813FD6C /* EncryptedHistoryRoomTimelineItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EncryptedHistoryRoomTimelineItem.swift; sourceTree = ""; }; D071F86CD47582B9196C9D16 /* UserDiscoverySection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserDiscoverySection.swift; sourceTree = ""; }; @@ -1437,7 +1437,7 @@ ECF79FB25E2D4BD6F50CE7C9 /* RoomMembersListScreenViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomMembersListScreenViewModel.swift; sourceTree = ""; }; ED044D00F2176681CC02CD54 /* HomeScreenRoomCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HomeScreenRoomCell.swift; sourceTree = ""; }; ED1D792EB82506A19A72C8DE /* RoomTimelineItemProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomTimelineItemProtocol.swift; sourceTree = ""; }; - ED482057AE39D5C6D9C5F3D8 /* message.caf */ = {isa = PBXFileReference; path = message.caf; sourceTree = ""; }; + ED482057AE39D5C6D9C5F3D8 /* message.caf */ = {isa = PBXFileReference; lastKnownFileType = file; path = message.caf; sourceTree = ""; }; ED983D4DCA5AFA6E1ED96099 /* StateRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StateRoomTimelineView.swift; sourceTree = ""; }; EDAA4472821985BF868CC21C /* ServerSelectionViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ServerSelectionViewModelTests.swift; sourceTree = ""; }; EE378083653EF0C9B5E9D580 /* EmoteRoomTimelineItemContent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EmoteRoomTimelineItemContent.swift; sourceTree = ""; }; @@ -1451,7 +1451,7 @@ F174A5627CDB3CAF280D1880 /* EmojiPickerScreenModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EmojiPickerScreenModels.swift; sourceTree = ""; }; F17EFA1D3D09FC2F9C5E1CB2 /* MediaProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MediaProvider.swift; sourceTree = ""; }; F1B8500C152BC59445647DA8 /* UnsupportedRoomTimelineItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UnsupportedRoomTimelineItem.swift; sourceTree = ""; }; - F2D513D2477B57F90E98EEC0 /* portrait_test_video.mp4 */ = {isa = PBXFileReference; path = portrait_test_video.mp4; sourceTree = ""; }; + F2D513D2477B57F90E98EEC0 /* portrait_test_video.mp4 */ = {isa = PBXFileReference; lastKnownFileType = file; path = portrait_test_video.mp4; sourceTree = ""; }; F31F59030205A6F65B057E1A /* MatrixEntityRegexTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MatrixEntityRegexTests.swift; sourceTree = ""; }; F348B5F2C12F9D4F4B4D3884 /* VideoRoomTimelineItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VideoRoomTimelineItem.swift; sourceTree = ""; }; F36C0A6D59717193F49EA986 /* UserSessionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserSessionTests.swift; sourceTree = ""; }; @@ -1473,7 +1473,7 @@ F8CEB4634C0DD7779C4AB504 /* CreateRoomScreenUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreateRoomScreenUITests.swift; sourceTree = ""; }; F9E785D5137510481733A3E8 /* TextRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TextRoomTimelineView.swift; sourceTree = ""; }; F9ED8E731E21055F728E5FED /* TimelineStartRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineStartRoomTimelineView.swift; sourceTree = ""; }; - FA7D3C686C214470F4911618 /* RoomTimelineItemViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomTimelineItemViewModel.swift; sourceTree = ""; }; + FA7D3C686C214470F4911618 /* RoomTimelineItemViewState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomTimelineItemViewState.swift; sourceTree = ""; }; FB0D6CB491777E7FC6B5BA12 /* CreateRoomScreen.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreateRoomScreen.swift; sourceTree = ""; }; FB7BAD55A4E2B8E5828CD64C /* SoftLogoutScreenViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SoftLogoutScreenViewModel.swift; sourceTree = ""; }; FBC776F301D374A3298C69DA /* AppCoordinatorProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppCoordinatorProtocol.swift; sourceTree = ""; }; @@ -2889,7 +2889,7 @@ 7D25A35764C7B3DB78954AB5 /* RoomTimelineItemFactoryProtocol.swift */, ED1D792EB82506A19A72C8DE /* RoomTimelineItemProtocol.swift */, 90F2F8998E5632668B0AD848 /* RoomTimelineItemView.swift */, - FA7D3C686C214470F4911618 /* RoomTimelineItemViewModel.swift */, + FA7D3C686C214470F4911618 /* RoomTimelineItemViewState.swift */, 75D1D02F7F3AC1122FCFB4F3 /* Items */, ); path = TimelineItems; @@ -3624,14 +3624,6 @@ path = Timeline; sourceTree = ""; }; - "TEMP_3919F537-58A5-4700-B869-24AAD8D8DD61" /* element-x-ios */ = { - isa = PBXGroup; - children = ( - 41553551C55AD59885840F0E /* secrets.xcconfig */, - ); - path = "element-x-ios"; - sourceTree = ""; - }; /* End PBXGroup section */ /* Begin PBXNativeTarget section */ @@ -4529,7 +4521,7 @@ 878070573C7BF19E735707B4 /* RoomTimelineItemProperties.swift in Sources */, 1AE4AEA0FA8DEF52671832E0 /* RoomTimelineItemProtocol.swift in Sources */, AD55E245FE686D7DB4C86406 /* RoomTimelineItemView.swift in Sources */, - 2198B4458AFF69102BBCC370 /* RoomTimelineItemViewModel.swift in Sources */, + 2198B4458AFF69102BBCC370 /* RoomTimelineItemViewState.swift in Sources */, 9BD3A773186291560DF92B62 /* RoomTimelineProvider.swift in Sources */, 77D7DAA41AAB36800C1F2E2D /* RoomTimelineProviderProtocol.swift in Sources */, B2F8E01ABA1BA30265B4ECBE /* RoundedCornerShape.swift in Sources */, diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift index 3e0a953a52..c3f607e365 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift @@ -177,13 +177,13 @@ struct RoomMemberState { struct TimelineViewState { var canBackPaginate = false var isBackPaginating = false - var itemsDictionary = OrderedDictionary() + var itemsDictionary = OrderedDictionary() var timelineIDs: [String] { itemsDictionary.keys.elements } - var itemViewModels: [RoomTimelineItemViewModel] { + var itemViewStates: [RoomTimelineItemViewState] { itemsDictionary.values.elements } diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index 6e1dfe3679..83a756b335 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -256,7 +256,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol } private func buildTimelineViews() { - var timelineItemsDictionary = OrderedDictionary() + var timelineItemsDictionary = OrderedDictionary() let itemsGroupedByTimelineDisplayStyle = timelineController.timelineItems.chunked { current, next in canGroupItem(timelineItem: current, with: next) @@ -270,19 +270,19 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol if itemGroup.count == 1 { if let firstItem = itemGroup.first { - timelineItemsDictionary.updateValue(RoomTimelineItemViewModel(item: firstItem, groupStyle: .single), + timelineItemsDictionary.updateValue(RoomTimelineItemViewState(item: firstItem, groupStyle: .single), forKey: firstItem.id.timelineID) } } else { for (index, item) in itemGroup.enumerated() { if index == 0 { - timelineItemsDictionary.updateValue(RoomTimelineItemViewModel(item: item, groupStyle: .first), + timelineItemsDictionary.updateValue(RoomTimelineItemViewState(item: item, groupStyle: .first), forKey: item.id.timelineID) } else if index == itemGroup.count - 1 { - timelineItemsDictionary.updateValue(RoomTimelineItemViewModel(item: item, groupStyle: .last), + timelineItemsDictionary.updateValue(RoomTimelineItemViewState(item: item, groupStyle: .last), forKey: item.id.timelineID) } else { - timelineItemsDictionary.updateValue(RoomTimelineItemViewModel(item: item, groupStyle: .middle), + timelineItemsDictionary.updateValue(RoomTimelineItemViewState(item: item, groupStyle: .middle), forKey: item.id.timelineID) } } diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift index d61b1e895c..30ded2fd4a 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift @@ -328,8 +328,8 @@ struct TimelineItemBubbledStylerView_Previews: PreviewProvider { static var mockTimeline: some View { ScrollView { VStack(alignment: .leading, spacing: 0) { - ForEach(viewModel.state.timelineViewState.itemViewModels) { itemViewModel in - RoomTimelineItemView(viewModel: itemViewModel) + ForEach(viewModel.state.timelineViewState.itemViewStates) { viewState in + RoomTimelineItemView(viewState: viewState) .padding(TimelineStyle.bubbles.rowInsets) // Insets added in the table view cells } @@ -341,7 +341,7 @@ struct TimelineItemBubbledStylerView_Previews: PreviewProvider { static var replies: some View { VStack { - RoomTimelineItemView(viewModel: .init(item: TextRoomTimelineItem(id: .init(timelineID: ""), + RoomTimelineItemView(viewState: .init(item: TextRoomTimelineItem(id: .init(timelineID: ""), timestamp: "10:42", isOutgoing: true, isEditable: false, @@ -350,7 +350,7 @@ struct TimelineItemBubbledStylerView_Previews: PreviewProvider { replyDetails: .loaded(sender: .init(id: "", displayName: "Alice"), contentType: .text(.init(body: "Short")))), groupStyle: .single)) - RoomTimelineItemView(viewModel: .init(item: TextRoomTimelineItem(id: .init(timelineID: ""), + RoomTimelineItemView(viewState: .init(item: TextRoomTimelineItem(id: .init(timelineID: ""), timestamp: "10:42", isOutgoing: true, isEditable: false, diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift index 9474b466c2..b6bca5adc9 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift @@ -144,7 +144,7 @@ struct TimelineItemPlainStylerView_Previews: PreviewProvider { VStack(alignment: .leading, spacing: 0) { ForEach(1.. Date: Tue, 25 Jul 2023 02:15:30 +0200 Subject: [PATCH 16/21] tests updated. --- .../Sources/RoomScreenViewModelTests.swift | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/UnitTests/Sources/RoomScreenViewModelTests.swift b/UnitTests/Sources/RoomScreenViewModelTests.swift index c0bf9754e1..e7a455faa1 100644 --- a/UnitTests/Sources/RoomScreenViewModelTests.swift +++ b/UnitTests/Sources/RoomScreenViewModelTests.swift @@ -51,9 +51,9 @@ class RoomScreenViewModelTests: XCTestCase { userIndicatorController: userIndicatorControllerMock) // Then the messages should be grouped together. - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[0].groupStyle, .first, "Nothing should prevent the first message from being grouped.") - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[1].groupStyle, .middle, "Nothing should prevent the middle message from being grouped.") - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[2].groupStyle, .last, "Nothing should prevent the last message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[0].groupStyle, .first, "Nothing should prevent the first message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[1].groupStyle, .middle, "Nothing should prevent the middle message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[2].groupStyle, .last, "Nothing should prevent the last message from being grouped.") } func testMessageGroupingMultipleSenders() { @@ -84,12 +84,12 @@ class RoomScreenViewModelTests: XCTestCase { userIndicatorController: userIndicatorControllerMock) // Then the messages should be grouped by sender. - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[0].groupStyle, .single, "A message should not be grouped when the sender changes.") - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[1].groupStyle, .single, "A message should not be grouped when the sender changes.") - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[2].groupStyle, .first, "A group should start with a new sender if there are more messages from that sender.") - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[3].groupStyle, .last, "A group should be ended when the sender changes in the next message.") - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[4].groupStyle, .first, "A group should start with a new sender if there are more messages from that sender.") - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[5].groupStyle, .last, "A group should be ended when the sender changes in the next message.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[0].groupStyle, .single, "A message should not be grouped when the sender changes.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[1].groupStyle, .single, "A message should not be grouped when the sender changes.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[2].groupStyle, .first, "A group should start with a new sender if there are more messages from that sender.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[3].groupStyle, .last, "A group should be ended when the sender changes in the next message.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[4].groupStyle, .first, "A group should start with a new sender if there are more messages from that sender.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[5].groupStyle, .last, "A group should be ended when the sender changes in the next message.") } func testMessageGroupingWithLeadingReactions() { @@ -115,9 +115,9 @@ class RoomScreenViewModelTests: XCTestCase { userIndicatorController: userIndicatorControllerMock) // Then the first message should not be grouped but the other two should. - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[0].groupStyle, .single, "When the first message has reactions it should not be grouped.") - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[1].groupStyle, .first, "A new group should be made when the preceding message has reactions.") - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[2].groupStyle, .last, "Nothing should prevent the last message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[0].groupStyle, .single, "When the first message has reactions it should not be grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[1].groupStyle, .first, "A new group should be made when the preceding message has reactions.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[2].groupStyle, .last, "Nothing should prevent the last message from being grouped.") } func testMessageGroupingWithInnerReactions() { @@ -143,9 +143,9 @@ class RoomScreenViewModelTests: XCTestCase { userIndicatorController: userIndicatorControllerMock) // Then the first and second messages should be grouped and the last one should not. - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[0].groupStyle, .first, "Nothing should prevent the first message from being grouped.") - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[1].groupStyle, .last, "When the message has reactions, the group should end here.") - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[2].groupStyle, .single, "The last message should not be grouped when the preceding message has reactions.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[0].groupStyle, .first, "Nothing should prevent the first message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[1].groupStyle, .last, "When the message has reactions, the group should end here.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[2].groupStyle, .single, "The last message should not be grouped when the preceding message has reactions.") } func testMessageGroupingWithTrailingReactions() { @@ -171,9 +171,9 @@ class RoomScreenViewModelTests: XCTestCase { userIndicatorController: userIndicatorControllerMock) // Then the messages should be grouped together. - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[0].groupStyle, .first, "Nothing should prevent the first message from being grouped.") - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[1].groupStyle, .middle, "Nothing should prevent the second message from being grouped.") - XCTAssertEqual(viewModel.state.timelineViewState.itemViewModels[2].groupStyle, .last, "Reactions on the last message should not prevent it from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[0].groupStyle, .first, "Nothing should prevent the first message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[1].groupStyle, .middle, "Nothing should prevent the second message from being grouped.") + XCTAssertEqual(viewModel.state.timelineViewState.itemViewStates[2].groupStyle, .last, "Reactions on the last message should not prevent it from being grouped.") } func testGoToUserDetailsSuccessNoDelay() async { From 3b9f64f5d7fabdb3d2236cc8eaade52c71acf89e Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 25 Jul 2023 10:36:28 +0200 Subject: [PATCH 17/21] pr comments --- ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift | 2 +- ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift index c3f607e365..9414a697f7 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift @@ -175,7 +175,7 @@ struct RoomMemberState { } struct TimelineViewState { - var canBackPaginate = false + var canBackPaginate = true var isBackPaginating = false var itemsDictionary = OrderedDictionary() diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index 83a756b335..a8a34da8ec 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -219,7 +219,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol private var paginateBackwardsTask: Task? - func paginateBackwards() { + private func paginateBackwards() { guard paginateBackwardsTask == nil else { return } From 998a654e73fd24e288de0bbf5c0a1e0776c84a5b Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 25 Jul 2023 11:11:50 +0200 Subject: [PATCH 18/21] better identifiers --- ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift index 7ed6728cfd..0723772f35 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift @@ -23,8 +23,8 @@ struct TimelineView: View { let viewState: TimelineViewState @Environment(\.timelineStyle) private var timelineStyle - private let bottomID = "bottomID" - private let topID = "topID" + private let bottomID = "RoomTimelineBottomPinIdentifier" + private let topID = "RoomTimelineTopPinIdentifier" @State private var scrollViewAdapter = ScrollViewAdapter() @State private var paginateBackwardsPublisher = PassthroughSubject() From 8067410ea7fcf5a7a01cfb1ad795b7fd55a04144 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 25 Jul 2023 12:10:09 +0200 Subject: [PATCH 19/21] fix --- ElementX.xcodeproj/project.pbxproj | 32 +++++++++++++++++++----------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj index cd83482d1d..57fa307a28 100644 --- a/ElementX.xcodeproj/project.pbxproj +++ b/ElementX.xcodeproj/project.pbxproj @@ -3,7 +3,7 @@ archiveVersion = 1; classes = { }; - objectVersion = 54; + objectVersion = 51; objects = { /* Begin PBXBuildFile section */ @@ -86,7 +86,6 @@ 206F0DBAB6AF042CA1FF2C0D /* SettingsViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D487C1185D658F8B15B8F55 /* SettingsViewModelTests.swift */; }; 208C19811613F9A10F8A7B75 /* MediaLoader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AFCE895ECFFA53FEE64D62B /* MediaLoader.swift */; }; 2185C1F6724C78FFF355D6FA /* WelcomeScreenScreenUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AB10FA6570DD08B3966C159 /* WelcomeScreenScreenUITests.swift */; }; - 2198B4458AFF69102BBCC370 /* RoomTimelineItemViewState.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA7D3C686C214470F4911618 /* RoomTimelineItemViewState.swift */; }; 21BF2B7CEDFE3CA67C5355AD /* test_image.png in Resources */ = {isa = PBXBuildFile; fileRef = C733D11B421CFE3A657EF230 /* test_image.png */; }; 22882C710BC99EC34A5024A0 /* UITestsScreenIdentifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6CEBE5EA91E8691EDF364EC2 /* UITestsScreenIdentifier.swift */; }; 2352C541AF857241489756FF /* MockRoomSummaryProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8F7D42E66E939B709C1EC390 /* MockRoomSummaryProvider.swift */; }; @@ -172,6 +171,7 @@ 40B79D20A873620F7F128A2C /* UserPreference.swift in Sources */ = {isa = PBXBuildFile; fileRef = 35FA991289149D31F4286747 /* UserPreference.swift */; }; 414F50CFCFEEE2611127DCFB /* RestorationToken.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3558A15CFB934F9229301527 /* RestorationToken.swift */; }; 4166A7DD2A4E2EFF0EB9369B /* FormRowLabelStyle.swift in Sources */ = {isa = PBXBuildFile; fileRef = D1897720266C036471AD9D1B /* FormRowLabelStyle.swift */; }; + 41CE5E1289C8768FC5B6490C /* RoomTimelineItemViewState.swift in Sources */ = {isa = PBXBuildFile; fileRef = D0C2D52E36AD614B3C003EF6 /* RoomTimelineItemViewState.swift */; }; 41DFDD212D1BE57CA50D783B /* Kingfisher in Frameworks */ = {isa = PBXBuildFile; productRef = 0DD568A494247444A4B56031 /* Kingfisher */; }; 41F553349AF44567184822D8 /* APNSPayload.swift in Sources */ = {isa = PBXBuildFile; fileRef = 94D670124FC3E84F23A62CCF /* APNSPayload.swift */; }; 4219391CD2351E410554B3E8 /* AggregratedReaction.swift in Sources */ = {isa = PBXBuildFile; fileRef = B858A61F2A570DFB8DE570A7 /* AggregratedReaction.swift */; }; @@ -857,7 +857,7 @@ 127C8472672A5BA09EF1ACF8 /* CurrentValuePublisher.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CurrentValuePublisher.swift; sourceTree = ""; }; 12EDAFB64FA5F6812D54F39A /* MigrationScreenViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MigrationScreenViewModel.swift; sourceTree = ""; }; 12F1E7F9C2BE8BB751037826 /* WaitlistScreenCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WaitlistScreenCoordinator.swift; sourceTree = ""; }; - 1304D9191300873EADA52D6E /* IntegrationTests.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = IntegrationTests.xctestplan; sourceTree = ""; }; + 1304D9191300873EADA52D6E /* IntegrationTests.xctestplan */ = {isa = PBXFileReference; path = IntegrationTests.xctestplan; sourceTree = ""; }; 130ED565A078F7E0B59D9D25 /* UNTextInputNotificationResponse+Creator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UNTextInputNotificationResponse+Creator.swift"; sourceTree = ""; }; 13802897C7AFA360EA74C0B0 /* en */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = en; path = en.lproj/Localizable.stringsdict; sourceTree = ""; }; 1423AB065857FA546444DB15 /* NotificationManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationManager.swift; sourceTree = ""; }; @@ -998,7 +998,7 @@ 47111410B6E659A697D472B5 /* RoomProxyProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomProxyProtocol.swift; sourceTree = ""; }; 471EB7D96AFEA8D787659686 /* EmoteRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EmoteRoomTimelineView.swift; sourceTree = ""; }; 47873756E45B46683D97DC32 /* LegalInformationScreenModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LegalInformationScreenModels.swift; sourceTree = ""; }; - 478BE8591BD13E908EF70C0C /* DesignKit */ = {isa = PBXFileReference; lastKnownFileType = folder; path = DesignKit; sourceTree = SOURCE_ROOT; }; + 478BE8591BD13E908EF70C0C /* DesignKit */ = {isa = PBXFileReference; lastKnownFileType = folder; name = DesignKit; path = DesignKit; sourceTree = SOURCE_ROOT; }; 4798B3B7A1E8AE3901CEE8C6 /* FramePreferenceKey.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FramePreferenceKey.swift; sourceTree = ""; }; 47EBB5D698CE9A25BB553A2D /* Strings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Strings.swift; sourceTree = ""; }; 47F29139BC2A804CE5E0757E /* MediaUploadPreviewScreenViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MediaUploadPreviewScreenViewModel.swift; sourceTree = ""; }; @@ -1175,7 +1175,7 @@ 8D55702474F279D910D2D162 /* RoomStateEventStringBuilder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomStateEventStringBuilder.swift; sourceTree = ""; }; 8D8169443E5AC5FF71BFB3DB /* cs */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = cs; path = cs.lproj/Localizable.strings; sourceTree = ""; }; 8DC2C9E0E15C79BBDA80F0A2 /* TimelineStyle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineStyle.swift; sourceTree = ""; }; - 8E088F2A1B9EC529D3221931 /* UITests.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = UITests.xctestplan; sourceTree = ""; }; + 8E088F2A1B9EC529D3221931 /* UITests.xctestplan */ = {isa = PBXFileReference; path = UITests.xctestplan; sourceTree = ""; }; 8E1BBA73B611EDEEA6E20E05 /* InvitesScreenModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InvitesScreenModels.swift; sourceTree = ""; }; 8EC57A32ABC80D774CC663DB /* SettingsScreenUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsScreenUITests.swift; sourceTree = ""; }; 8F21ED7205048668BEB44A38 /* AppActivityView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppActivityView.swift; sourceTree = ""; }; @@ -1285,7 +1285,7 @@ B4CFE236419E830E8946639C /* Analytics+SwiftUI.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Analytics+SwiftUI.swift"; sourceTree = ""; }; B590BD4507D4F0A377FDE01A /* LoadableAvatarImage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoadableAvatarImage.swift; sourceTree = ""; }; B5B243E7818E5E9F6A4EDC7A /* NoticeRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NoticeRoomTimelineView.swift; sourceTree = ""; }; - B61C339A2FDDBD067FF6635C /* ConfettiScene.scn */ = {isa = PBXFileReference; lastKnownFileType = file.bplist; path = ConfettiScene.scn; sourceTree = ""; }; + B61C339A2FDDBD067FF6635C /* ConfettiScene.scn */ = {isa = PBXFileReference; path = ConfettiScene.scn; sourceTree = ""; }; B6311F21F911E23BE4DF51B4 /* ReadMarkerRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReadMarkerRoomTimelineView.swift; sourceTree = ""; }; B697816AF93DA06EC58C5D70 /* WaitlistScreenViewModelProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WaitlistScreenViewModelProtocol.swift; sourceTree = ""; }; B6E89E530A8E92EC44301CA1 /* Bundle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Bundle.swift; sourceTree = ""; }; @@ -1365,12 +1365,13 @@ CD6B0C4639E066915B5E6463 /* target.yml */ = {isa = PBXFileReference; lastKnownFileType = text.yaml; path = target.yml; sourceTree = ""; }; CDB3227C7A74B734924942E9 /* RoomSummaryProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomSummaryProvider.swift; sourceTree = ""; }; CEE0E6043EFCF6FD2A341861 /* TimelineReplyView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineReplyView.swift; sourceTree = ""; }; - CEE41494C837AA403A06A5D9 /* UnitTests.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = UnitTests.xctestplan; sourceTree = ""; }; + CEE41494C837AA403A06A5D9 /* UnitTests.xctestplan */ = {isa = PBXFileReference; path = UnitTests.xctestplan; sourceTree = ""; }; CF48AF076424DBC1615C74AD /* AuthenticationServiceProxy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AuthenticationServiceProxy.swift; sourceTree = ""; }; D0140615D2232612C813FD6C /* EncryptedHistoryRoomTimelineItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EncryptedHistoryRoomTimelineItem.swift; sourceTree = ""; }; D071F86CD47582B9196C9D16 /* UserDiscoverySection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserDiscoverySection.swift; sourceTree = ""; }; D09A267106B9585D3D0CFC0D /* ClientError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClientError.swift; sourceTree = ""; }; D0A45283CF1DB96E583BECA6 /* ImageRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageRoomTimelineView.swift; sourceTree = ""; }; + D0C2D52E36AD614B3C003EF6 /* RoomTimelineItemViewState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomTimelineItemViewState.swift; sourceTree = ""; }; D1897720266C036471AD9D1B /* FormRowLabelStyle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FormRowLabelStyle.swift; sourceTree = ""; }; D263254AFE5B7993FFBBF324 /* NSE.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = NSE.entitlements; sourceTree = ""; }; D316BB02636AF2174F2580E6 /* SoftLogoutScreenViewModelProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SoftLogoutScreenViewModelProtocol.swift; sourceTree = ""; }; @@ -1437,7 +1438,7 @@ ECF79FB25E2D4BD6F50CE7C9 /* RoomMembersListScreenViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomMembersListScreenViewModel.swift; sourceTree = ""; }; ED044D00F2176681CC02CD54 /* HomeScreenRoomCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HomeScreenRoomCell.swift; sourceTree = ""; }; ED1D792EB82506A19A72C8DE /* RoomTimelineItemProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomTimelineItemProtocol.swift; sourceTree = ""; }; - ED482057AE39D5C6D9C5F3D8 /* message.caf */ = {isa = PBXFileReference; lastKnownFileType = file; path = message.caf; sourceTree = ""; }; + ED482057AE39D5C6D9C5F3D8 /* message.caf */ = {isa = PBXFileReference; path = message.caf; sourceTree = ""; }; ED983D4DCA5AFA6E1ED96099 /* StateRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StateRoomTimelineView.swift; sourceTree = ""; }; EDAA4472821985BF868CC21C /* ServerSelectionViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ServerSelectionViewModelTests.swift; sourceTree = ""; }; EE378083653EF0C9B5E9D580 /* EmoteRoomTimelineItemContent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EmoteRoomTimelineItemContent.swift; sourceTree = ""; }; @@ -1451,7 +1452,7 @@ F174A5627CDB3CAF280D1880 /* EmojiPickerScreenModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EmojiPickerScreenModels.swift; sourceTree = ""; }; F17EFA1D3D09FC2F9C5E1CB2 /* MediaProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MediaProvider.swift; sourceTree = ""; }; F1B8500C152BC59445647DA8 /* UnsupportedRoomTimelineItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UnsupportedRoomTimelineItem.swift; sourceTree = ""; }; - F2D513D2477B57F90E98EEC0 /* portrait_test_video.mp4 */ = {isa = PBXFileReference; lastKnownFileType = file; path = portrait_test_video.mp4; sourceTree = ""; }; + F2D513D2477B57F90E98EEC0 /* portrait_test_video.mp4 */ = {isa = PBXFileReference; path = portrait_test_video.mp4; sourceTree = ""; }; F31F59030205A6F65B057E1A /* MatrixEntityRegexTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MatrixEntityRegexTests.swift; sourceTree = ""; }; F348B5F2C12F9D4F4B4D3884 /* VideoRoomTimelineItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VideoRoomTimelineItem.swift; sourceTree = ""; }; F36C0A6D59717193F49EA986 /* UserSessionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserSessionTests.swift; sourceTree = ""; }; @@ -1473,7 +1474,6 @@ F8CEB4634C0DD7779C4AB504 /* CreateRoomScreenUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreateRoomScreenUITests.swift; sourceTree = ""; }; F9E785D5137510481733A3E8 /* TextRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TextRoomTimelineView.swift; sourceTree = ""; }; F9ED8E731E21055F728E5FED /* TimelineStartRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineStartRoomTimelineView.swift; sourceTree = ""; }; - FA7D3C686C214470F4911618 /* RoomTimelineItemViewState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomTimelineItemViewState.swift; sourceTree = ""; }; FB0D6CB491777E7FC6B5BA12 /* CreateRoomScreen.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreateRoomScreen.swift; sourceTree = ""; }; FB7BAD55A4E2B8E5828CD64C /* SoftLogoutScreenViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SoftLogoutScreenViewModel.swift; sourceTree = ""; }; FBC776F301D374A3298C69DA /* AppCoordinatorProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppCoordinatorProtocol.swift; sourceTree = ""; }; @@ -2889,7 +2889,7 @@ 7D25A35764C7B3DB78954AB5 /* RoomTimelineItemFactoryProtocol.swift */, ED1D792EB82506A19A72C8DE /* RoomTimelineItemProtocol.swift */, 90F2F8998E5632668B0AD848 /* RoomTimelineItemView.swift */, - FA7D3C686C214470F4911618 /* RoomTimelineItemViewState.swift */, + D0C2D52E36AD614B3C003EF6 /* RoomTimelineItemViewState.swift */, 75D1D02F7F3AC1122FCFB4F3 /* Items */, ); path = TimelineItems; @@ -3624,6 +3624,14 @@ path = Timeline; sourceTree = ""; }; + "TEMP_43147A32-A06C-4EEF-8C45-FDEA43A25C1E" /* element-x-ios */ = { + isa = PBXGroup; + children = ( + 41553551C55AD59885840F0E /* secrets.xcconfig */, + ); + path = "element-x-ios"; + sourceTree = ""; + }; /* End PBXGroup section */ /* Begin PBXNativeTarget section */ @@ -4521,7 +4529,7 @@ 878070573C7BF19E735707B4 /* RoomTimelineItemProperties.swift in Sources */, 1AE4AEA0FA8DEF52671832E0 /* RoomTimelineItemProtocol.swift in Sources */, AD55E245FE686D7DB4C86406 /* RoomTimelineItemView.swift in Sources */, - 2198B4458AFF69102BBCC370 /* RoomTimelineItemViewState.swift in Sources */, + 41CE5E1289C8768FC5B6490C /* RoomTimelineItemViewState.swift in Sources */, 9BD3A773186291560DF92B62 /* RoomTimelineProvider.swift in Sources */, 77D7DAA41AAB36800C1F2E2D /* RoomTimelineProviderProtocol.swift in Sources */, B2F8E01ABA1BA30265B4ECBE /* RoundedCornerShape.swift in Sources */, From bd3920f8316107fd2281b54a1134210a9f23952d Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 25 Jul 2023 12:44:51 +0200 Subject: [PATCH 20/21] some PR comments --- .../Screens/RoomScreen/View/TimelineView.swift | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift index 0723772f35..c18eff22c9 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift @@ -47,12 +47,12 @@ struct TimelineView: View { topPin } - .introspect(.scrollView, on: .iOS(.v16)) { uiKitScrollView in - guard uiKitScrollView != scrollViewAdapter.scrollView else { + .introspect(.scrollView, on: .iOS(.v16)) { uiScrollView in + guard uiScrollView != scrollViewAdapter.scrollView else { return } - scrollViewAdapter.scrollView = uiKitScrollView + scrollViewAdapter.scrollView = uiScrollView scrollViewAdapter.shouldScrollToTopClosure = { _ in withAnimation { scrollView.scrollTo(topID) @@ -61,7 +61,7 @@ struct TimelineView: View { } // Allows the scroll to top to work properly - uiKitScrollView.contentOffset.y -= 1 + uiScrollView.contentOffset.y -= 1 } .scaleEffect(x: 1, y: -1) .onReceive(scrollToBottomPublisher) { _ in @@ -90,14 +90,14 @@ struct TimelineView: View { } } .onReceive(paginateBackwardsPublisher.collect(.byTime(DispatchQueue.main, 0.1))) { _ in - tryPaginateBackwards() + paginateBackwardsIfNeeded() } .onAppear { paginateBackwardsPublisher.send() } } - // Used to mark the top of the scroll view and easily scroll to it + /// Used to mark the top of the scroll view and easily scroll to it private var topPin: some View { Divider() .id(topID) @@ -105,7 +105,7 @@ struct TimelineView: View { .frame(height: 0) } - // Used to mark the bottom of the scroll view and easily scroll to it + /// Used to mark the bottom of the scroll view and easily scroll to it private var bottomPin: some View { Divider() .id(bottomID) @@ -136,7 +136,7 @@ struct TimelineView: View { .animation(.elementDefault, value: scrollToBottomButtonVisible) } - private func tryPaginateBackwards() { + private func paginateBackwardsIfNeeded() { guard let paginateAction = viewState.paginateAction, let scrollView = scrollViewAdapter.scrollView, viewState.canBackPaginate, From b1950b79b729e5d97ceef8beeb03a00313742375 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 25 Jul 2023 13:29:54 +0200 Subject: [PATCH 21/21] pr comments --- .../Sources/Screens/RoomScreen/RoomScreenModels.swift | 8 ++++---- .../Screens/RoomScreen/RoomScreenViewModel.swift | 10 +++++++--- .../Sources/Screens/RoomScreen/View/RoomScreen.swift | 1 - .../View/Timeline/CollapsibleRoomTimelineView.swift | 2 +- .../Sources/Screens/RoomScreen/View/TimelineView.swift | 4 ++-- .../TimelineItems/RoomTimelineItemViewState.swift | 2 ++ 6 files changed, 16 insertions(+), 11 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift index 9414a697f7..fff188ed96 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift @@ -90,11 +90,9 @@ struct RoomScreenViewState: BindableState { var timelineStyle: TimelineStyle var readReceiptsEnabled: Bool var isEncryptedOneToOneRoom = false - var timelineViewState = TimelineViewState() - + var timelineViewState = TimelineViewState() // check the doc before changing this var composerMode: RoomScreenComposerMode = .default - let scrollToBottomPublisher = PassthroughSubject() - + var bindings: RoomScreenViewStateBindings /// A closure providing the actions to show when long pressing on an item in the timeline. @@ -174,6 +172,8 @@ struct RoomMemberState { let avatarURL: URL? } +/// Used as the state for the TimelineView, to avoid having the context continuously refresh the list of items on each small change. +/// Is also nice to have this as a wrapper for any state that is directly connected to the timeline. struct TimelineViewState { var canBackPaginate = true var isBackPaginating = false diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index a8a34da8ec..169dafcb41 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -37,6 +37,8 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol private var canCurrentUserRedact = false + private var paginateBackwardsTask: Task? + init(timelineController: RoomTimelineControllerProtocol, mediaProvider: MediaProviderProtocol, roomProxy: RoomProxyProtocol, @@ -217,14 +219,16 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol .store(in: &cancellables) } - private var paginateBackwardsTask: Task? - private func paginateBackwards() { guard paginateBackwardsTask == nil else { return } - paginateBackwardsTask = Task { + paginateBackwardsTask = Task { [weak self] in + guard let self else { + return + } + switch await timelineController.paginateBackwards(requestSize: Constants.backPaginationEventLimit, untilNumberOfItems: Constants.backPaginationPageSize) { case .failure: displayError(.toast(L10n.errorFailedLoadingMessages)) diff --git a/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift b/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift index 3019418a61..6882aa703d 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift @@ -82,7 +82,6 @@ struct RoomScreen: View { } } - @ViewBuilder private var timeline: some View { TimelineView(viewState: context.viewState.timelineViewState) .id(context.viewState.roomId) diff --git a/ElementX/Sources/Screens/RoomScreen/View/Timeline/CollapsibleRoomTimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/Timeline/CollapsibleRoomTimelineView.swift index 2919299be9..75e067d4e2 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Timeline/CollapsibleRoomTimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Timeline/CollapsibleRoomTimelineView.swift @@ -43,7 +43,7 @@ private struct CollapsibleRoomTimelineItemDisclosureGroupStyle: DisclosureGroupS func makeBody(configuration: Configuration) -> some View { VStack(spacing: 0.0) { Button { - withAnimation { + withElementAnimation { configuration.isExpanded.toggle() } } label: { diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift index c18eff22c9..c1979eab7f 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift @@ -54,7 +54,7 @@ struct TimelineView: View { scrollViewAdapter.scrollView = uiScrollView scrollViewAdapter.shouldScrollToTopClosure = { _ in - withAnimation { + withElementAnimation { scrollView.scrollTo(topID) } return false @@ -65,7 +65,7 @@ struct TimelineView: View { } .scaleEffect(x: 1, y: -1) .onReceive(scrollToBottomPublisher) { _ in - withAnimation { + withElementAnimation { scrollView.scrollTo(bottomID) } } diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewState.swift b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewState.swift index a36919d817..aa8cf50633 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewState.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemViewState.swift @@ -20,10 +20,12 @@ struct RoomTimelineItemViewState: Identifiable, Equatable { let type: RoomTimelineItemType let groupStyle: TimelineGroupStyle + /// Contains all the identification info of the item, `timelineID`, `eventID` and `transactionID` var identifier: TimelineItemIdentifier { type.id } + /// The `timelineID` of the item, used for the timeline view level identification, do not use for any business logic use `identifier` instead var id: String { identifier.timelineID }