From 76b0cede77ab025401573b09fc81cadd8a6c174c Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Thu, 20 Jul 2023 13:29:45 +0200 Subject: [PATCH] Flipping the tableView (#1366) * I can't believe this works so well. * code improvements * we probably just need to change the TODO * better naming * done, there is only one small bug to fix * fix * this needs to be decided * fixed the backpagination issue and improved the scroll to top gesture triggered by tapping on the clock (but won't work when I am at the bottom) * fix --- .../View/TimelineTableViewController.swift | 199 +++--------------- .../RoomTimelineController.swift | 8 +- 2 files changed, 33 insertions(+), 174 deletions(-) diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineTableViewController.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineTableViewController.swift index b0b38dacf..5dbade690 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineTableViewController.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineTableViewController.swift @@ -35,6 +35,7 @@ class TimelineItemCell: UITableViewCell { /// /// 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) @@ -42,13 +43,6 @@ class TimelineTableViewController: UIViewController { var timelineStyle: TimelineStyle var timelineItemsDictionary = OrderedDictionary() { didSet { - guard !scrollAdapter.isScrolling.value else { - // Delay updating until scrolling has stopped as programatic - // changes to the scroll position kills any inertia. - hasPendingUpdates = true - return - } - applySnapshot() if timelineItemsDictionary.isEmpty { @@ -77,7 +71,7 @@ class TimelineTableViewController: UIViewController { @Binding private var scrollToBottomButtonVisible: Bool private var timelineItemsIDs: [String] { - timelineItemsDictionary.keys.elements + timelineItemsDictionary.keys.elements.reversed() } /// The table's diffable data source. @@ -91,10 +85,6 @@ class TimelineTableViewController: UIViewController { /// 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 ``timelineItems`` value should be applied when scrolling stops. - private var hasPendingUpdates = false - /// We need to store the previous layout as computing it on the fly leads to problems. - private var previousLayout: LayoutDescriptor? /// Whether or not the view has been shown on screen yet. private var hasAppearedOnce = false /// Whether the scroll and the animations should happen @@ -115,48 +105,25 @@ class TimelineTableViewController: UIViewController { 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 - tableView.publisher(for: \.contentSize) - .removeDuplicates() - .sink { [weak self] _ in - self?.updateTopPadding() - } - .store(in: &cancellables) - scrollToBottomPublisher .sink { [weak self] _ in self?.scrollToBottom(animated: true) } .store(in: &cancellables) - scrollAdapter.isScrolling - .sink { [weak self] isScrolling in - guard !isScrolling, let self, self.hasPendingUpdates else { return } - // When scrolling has stopped, apply any pending updates. - self.applySnapshot() - self.hasPendingUpdates = false - self.paginateBackwardsPublisher.send(()) - } - .store(in: &cancellables) - paginateBackwardsPublisher .collect(.byTime(DispatchQueue.main, 0.1)) .sink { [weak self] _ in self?.paginateBackwardsIfNeeded() } .store(in: &cancellables) - - NotificationCenter.default.publisher(for: UIResponder.keyboardDidShowNotification) - .sink { [weak self] _ in - guard let self, let layout = self.previousLayout, layout.isBottomVisible else { return } - self.scrollToBottom(animated: false) // Force the bottom to be visible as some timelines misbehave. - } - .store(in: &cancellables) ServiceLocator.shared.settings.$timelineDiffableAnimationsEnabled .weakAssign(to: \.shouldAnimate, on: self) @@ -174,13 +141,9 @@ class TimelineTableViewController: UIViewController { guard !hasAppearedOnce else { return } scrollToBottom(animated: false) hasAppearedOnce = true - } - - override func didMove(toParent parent: UIViewController?) { - super.didMove(toParent: parent) - - // Ensure the padding is correct before display. - updateTopPadding() + paginateBackwardsPublisher.send() + // This allows the reversed table view never fully be considered at the bottom + tableView.contentOffset.y = 1 } override func viewWillLayoutSubviews() { @@ -191,13 +154,6 @@ class TimelineTableViewController: UIViewController { } tableView.frame = CGRect(origin: .zero, size: view.frame.size) - - // Update the table's layout if necessary after the frame changed. - updateTopPadding() - - if let previousLayout, previousLayout.isBottomVisible { - scrollToBottom(animated: false) - } } /// Configures a diffable data source for the timeline's table view. @@ -233,7 +189,9 @@ class TimelineTableViewController: UIViewController { .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 } @@ -246,10 +204,7 @@ class TimelineTableViewController: UIViewController { /// the scroll position will be updated to maintain the position of the last visible item. private func applySnapshot() { guard let dataSource else { return } - - let previousLayout = layout() - self.previousLayout = previousLayout - + var snapshot = NSDiffableDataSourceSnapshot() snapshot.appendSections([.main]) snapshot.appendItems(timelineItemsIDs) @@ -257,90 +212,14 @@ class TimelineTableViewController: UIViewController { let currentSnapshot = dataSource.snapshot() MXLog.verbose("DIFF: \(snapshot.itemIdentifiers.difference(from: currentSnapshot.itemIdentifiers))") - // We only animate if the last item has changed - // We don't care to animate backpagination since we want to keep the scrolling position when that happens - let animated = shouldAnimate && snapshot.itemIdentifiers.last != currentSnapshot.itemIdentifiers.last + // 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) - - if previousLayout.isBottomVisible { - scrollToBottom(animated: false) - } else if let pinnedItem = previousLayout.pinnedItem { - restoreScrollPosition(using: pinnedItem, and: snapshot) - } - } - - /// Returns a description of the current layout in order to update the - /// scroll position after adding/updating items to the timeline. - private func layout() -> LayoutDescriptor { - guard let dataSource else { return LayoutDescriptor() } - - let snapshot = dataSource.snapshot() - var layout = LayoutDescriptor(numberOfItems: snapshot.numberOfItems) - - guard !snapshot.itemIdentifiers.isEmpty else { - layout.isBottomVisible = true - return layout - } - - guard let bottomItemIndexPath = tableView.indexPathsForVisibleRows?.last, - let bottomID = dataSource.itemIdentifier(for: bottomItemIndexPath) - else { return layout } - - let bottomCellFrame = tableView.cellFrame(for: bottomID) - layout.pinnedItem = PinnedItem(id: bottomID, position: .bottom, frame: bottomCellFrame) - layout.isBottomVisible = bottomID == snapshot.itemIdentifiers.last - - return layout - } - - /// Updates the additional padding added to the top of the table (via a header) - /// in order to fill the timeline from the bottom of the view upwards. - private func updateTopPadding() { - let headerHeight = tableView.tableHeaderView?.frame.height ?? 0 - let contentHeight = tableView.contentSize.height - headerHeight - let newHeight = max(0, tableView.visibleSize.height - contentHeight) - - // Round the check to account floating point accuracy during keyboard appearance. - guard newHeight.rounded() != headerHeight.rounded() else { return } - - if newHeight > 0 { - let frame = CGRect(origin: .zero, size: CGSize(width: tableView.contentSize.width, height: newHeight)) - tableView.tableHeaderView = UIView(frame: frame) // Updating an existing view's height doesn't move the cells. - } else { - tableView.tableHeaderView = nil - } - } - - /// Whether or not the bottom of the scroll view is visible (with some small tolerance added). - private func isAtBottom() -> Bool { - tableView.contentOffset.y < (tableView.contentSize.height - tableView.visibleSize.height - 15) } /// Scrolls to the bottom of the timeline. private func scrollToBottom(animated: Bool) { - guard let lastItemID = timelineItemsIDs.last, - let lastIndexPath = dataSource?.indexPath(for: lastItemID) - else { return } - - tableView.scrollToRow(at: lastIndexPath, at: .bottom, animated: animated) - } - - /// Restores the position of the timeline using the supplied item and snapshot. - private func restoreScrollPosition(using pinnedItem: PinnedItem, and snapshot: NSDiffableDataSourceSnapshot) { - guard let item = snapshot.itemIdentifiers.first(where: { $0 == pinnedItem.id }), - let indexPath = dataSource?.indexPath(for: item) - else { return } - - // Scroll the item into view. - tableView.scrollToRow(at: indexPath, at: pinnedItem.position, animated: false) - - guard let oldFrame = pinnedItem.frame, let newFrame = tableView.cellFrame(for: item) else { return } - - // Remove any unwanted offset that was added by scrollToRow. - let deltaY = newFrame.maxY - oldFrame.maxY - if deltaY != 0 { - tableView.contentOffset.y += deltaY - } + tableView.scrollToRow(at: IndexPath(item: 0, section: 0), at: .top, animated: animated) } /// Checks whether or a backwards pagination is needed and requests one if so. @@ -349,12 +228,16 @@ class TimelineTableViewController: UIViewController { private func paginateBackwardsIfNeeded() { guard canBackPaginate, !isBackPaginating, - !hasPendingUpdates, - tableView.contentOffset.y < tableView.visibleSize.height * 2.0 + tableView.contentOffset.y > tableView.contentSize.height - tableView.visibleSize.height * 2.0 else { return } coordinator.send(viewAction: .paginateBackwards) } + + private func scrollToTop(animated: Bool) { + tableView.scrollToRow(at: IndexPath(item: timelineItemsIDs.count - 1, section: 0), at: .bottom, animated: animated) + scrollAdapter.scrollViewDidScrollToTop(tableView) + } } // MARK: - UITableViewDelegate @@ -367,13 +250,17 @@ extension TimelineTableViewController: UITableViewDelegate { DispatchQueue.main.async { [weak self] in guard let self else { return } - let isAtBottom = self.isAtBottom() + let scrollToBottomButtonVisible = scrollView.contentOffset.y > 15 // Only update the binding on changes to avoid needlessly recomputing the hierarchy when scrolling. - if self.scrollToBottomButtonVisible != isAtBottom { - self.scrollToBottomButtonVisible = isAtBottom + if self.scrollToBottomButtonVisible != scrollToBottomButtonVisible { + self.scrollToBottomButtonVisible = scrollToBottomButtonVisible } } + + if scrollView.contentOffset.y == 0 { + scrollView.contentOffset.y = 1 + } } // MARK: ScrollViewAdapter Methods @@ -395,9 +282,10 @@ extension TimelineTableViewController: UITableViewDelegate { func scrollViewDidEndDecelerating(_ scrollView: UIScrollView) { scrollAdapter.scrollViewDidEndDecelerating(scrollView) } - - func scrollViewDidScrollToTop(_ scrollView: UIScrollView) { - scrollAdapter.scrollViewDidScrollToTop(scrollView) + + func scrollViewShouldScrollToTop(_ scrollView: UIScrollView) -> Bool { + scrollToTop(animated: true) + return false } } @@ -408,31 +296,4 @@ extension TimelineTableViewController { enum TimelineSection { case main } - - /// A description of the timeline's layout. - struct LayoutDescriptor { - var numberOfItems = 0 - var pinnedItem: PinnedItem? - var isBottomVisible = false - } - - /// An item that should have its position pinned after updates. - struct PinnedItem { - let id: String - let position: UITableView.ScrollPosition - let frame: CGRect? - } -} - -// MARK: - Cell Layout - -private extension UITableView { - /// Returns the frame of the cell for a particular timeline item. - func cellFrame(for id: String) -> CGRect? { - guard let timelineCell = visibleCells.last(where: { ($0 as? TimelineItemCell)?.item?.id.timelineID == id }) else { - return nil - } - - return convert(timelineCell.frame, to: superview) - } } diff --git a/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift b/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift index b3e1d249d..a9f6f0c0c 100644 --- a/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift +++ b/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift @@ -277,11 +277,7 @@ class RoomTimelineController: RoomTimelineControllerProtocol { let timelineItem = buildTimelineItem(for: itemProxy, chunkIndex: reversedIndex) - if timelineItem is PaginationIndicatorRoomTimelineItem { - isBackPaginating = true - } else if timelineItem is TimelineStartRoomTimelineItem { - canBackPaginate = false - } else if timelineItem is EncryptedHistoryRoomTimelineItem { + if timelineItem is EncryptedHistoryRoomTimelineItem { canBackPaginate = false } @@ -319,8 +315,10 @@ class RoomTimelineController: RoomTimelineControllerProtocol { case .timelineStartReached: let timelineStart = TimelineStartRoomTimelineItem(name: roomProxy.displayName ?? roomProxy.name) newTimelineItems.insert(timelineStart, at: 0) + canBackPaginate = false case .paginating: newTimelineItems.insert(PaginationIndicatorRoomTimelineItem(), at: 0) + isBackPaginating = true case .idle: break }