From cbfd2d1cca1d001e17585b9b475660b6ec2187f4 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Thu, 2 Feb 2023 15:17:13 +0200 Subject: [PATCH] Refactor the dynamic timeline modifiers and make sure they fire in the right order --- .../Screens/HomeScreen/HomeScreenModels.swift | 4 +- .../HomeScreen/HomeScreenViewModel.swift | 53 +++++++++++-------- .../Screens/HomeScreen/View/HomeScreen.swift | 25 ++++----- .../Client/SlidingSyncViewProxy.swift | 2 +- 4 files changed, 45 insertions(+), 39 deletions(-) diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift index f515e4246..1d3a3d396 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift @@ -39,7 +39,7 @@ enum HomeScreenViewAction { case userMenu(action: HomeScreenViewUserMenuAction) case verifySession case skipSessionVerification - case updatedVisibleItemRange(Range) + case updateVisibleItemRange(range: Range, isScrolling: Bool) } enum HomeScreenRoomListMode: CustomStringConvertible { @@ -91,8 +91,6 @@ struct HomeScreenViewState: BindableState { struct HomeScreenViewStateBindings { var searchQuery = "" - var isScrolling = false - var alertInfo: AlertInfo? } diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift index aec00286f..7bf644564 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift @@ -29,7 +29,8 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol private let allRoomsSummaryProvider: RoomSummaryProviderProtocol? private let attributedStringBuilder: AttributedStringBuilderProtocol - private let visibleItemRangePublisher = CurrentValueSubject, Never>(0..<0) + private var visibleItemRangeObservationToken: AnyCancellable? + private let visibleItemRangePublisher = CurrentValueSubject<(range: Range, isScrolling: Bool), Never>((0..<0, false)) var callback: ((HomeScreenViewModelAction) -> Void)? @@ -58,24 +59,6 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol } .store(in: &cancellables) - visibleItemRangePublisher - .debounce(for: 0.1, scheduler: DispatchQueue.main) - .removeDuplicates() - .sink { [weak self] range in - guard let self else { return } - - guard self.state.bindings.searchQuery.isEmpty else { - return - } - - if self.state.bindings.isScrolling { - self.updateVisibleRange(range, timelineLimit: SlidingSyncConstants.lastMessageTimelineLimit) - } else { - self.updateVisibleRange(range, timelineLimit: SlidingSyncConstants.timelinePrecachingTimelineLimit) - } - } - .store(in: &cancellables) - guard let visibleRoomsSummaryProvider, let allRoomsSummaryProvider else { MXLog.error("Room summary provider unavailable") return @@ -136,9 +119,14 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol .store(in: &cancellables) allRoomsSummaryProvider.roomListPublisher + .dropFirst(1) // We don't care about its initial value .receive(on: DispatchQueue.main) .sink { [weak self] _ in self?.updateRooms() + + // Wait for the all rooms view to receive its first update before installing + // dynamic timeline modifiers + self?.installDynamicTimelineLimitModifiers() } .store(in: &cancellables) @@ -166,8 +154,8 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol callback?(.presentSessionVerificationScreen) case .skipSessionVerification: state.showSessionVerificationBanner = false - case .updatedVisibleItemRange(let range): - visibleItemRangePublisher.send(range) + case .updateVisibleItemRange(let range, let isScrolling): + visibleItemRangePublisher.send((range, isScrolling)) } } @@ -181,6 +169,29 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol } // MARK: - Private + + /// We want the timeline limit to be set to 1 while scrolling the list so that last messages load up fast. We also want to set that back to 20 when the scrolling + /// stops to load room history. Also we don't want this to be setup before the initial sync is over so we only call it when the allRoomsSummaryProvider + /// first receives some changes + private func installDynamicTimelineLimitModifiers() { + guard visibleItemRangeObservationToken == nil else { + return + } + + visibleItemRangeObservationToken = visibleItemRangePublisher + .removeDuplicates(by: { $0.isScrolling == $1.isScrolling && $0.range == $1.range }) + .throttle(for: 0.5, scheduler: DispatchQueue.main, latest: true) + .sink { [weak self] value in + guard let self else { return } + + // Ignore scrolling while filtering rooms + guard self.state.bindings.searchQuery.isEmpty else { + return + } + + self.updateVisibleRange(value.range, timelineLimit: value.isScrolling ? SlidingSyncConstants.lastMessageTimelineLimit : SlidingSyncConstants.timelinePrecachingTimelineLimit) + } + } /// This method will update all view state rooms by merging the data from both summary providers /// If a room is empty in the visible room summary provider it will try to get it from the allRooms one diff --git a/ElementX/Sources/Screens/HomeScreen/View/HomeScreen.swift b/ElementX/Sources/Screens/HomeScreen/View/HomeScreen.swift index 4fd51ce09..e694322de 100644 --- a/ElementX/Sources/Screens/HomeScreen/View/HomeScreen.swift +++ b/ElementX/Sources/Screens/HomeScreen/View/HomeScreen.swift @@ -21,8 +21,11 @@ struct HomeScreen: View { @State private var scrollViewAdapter = ScrollViewAdapter() @State private var showingLogoutConfirmation = false - @State private var visibleItemIdentifiers = Set() - @State private var hasTriggeredInitialVisibleItemUpdate = false + @State private var visibleItemIdentifiers = Set() { + didSet { + updateVisibleRange() + } + } var body: some View { ScrollView { @@ -65,6 +68,11 @@ struct HomeScreen: View { guard scrollView != scrollViewAdapter.scrollView else { return } scrollViewAdapter.scrollView = scrollView } + .onReceive(scrollViewAdapter.isScrolling) { isScrolling in + if !isScrolling { + updateVisibleRange() + } + } .scrollDismissesKeyboard(.immediately) .disabled(context.viewState.roomListMode == .skeletons) .animation(.elementDefault, value: context.viewState.showSessionVerificationBanner) @@ -76,17 +84,6 @@ struct HomeScreen: View { userMenuButton } } - .onChange(of: visibleItemIdentifiers) { _ in - if !hasTriggeredInitialVisibleItemUpdate { - updateVisibleRange() - hasTriggeredInitialVisibleItemUpdate = true - } - } - .onReceive(scrollViewAdapter.isScrolling) { isScrolling in - context.isScrolling = isScrolling - - updateVisibleRange() - } .background(Color.element.background) } @@ -196,7 +193,7 @@ struct HomeScreen: View { return } - context.send(viewAction: .updatedVisibleItemRange(firstIndex.., timelineLimit: UInt) { - MXLog.info("Setting sliding sync view range to \(range), timelineLimit: \(timelineLimit)") + MXLog.info("Setting sliding sync view range to \(range), timeline limit: \(timelineLimit)") slidingSyncView.setRange(start: UInt32(range.lowerBound), end: UInt32(range.upperBound)) slidingSyncView.setTimelineLimit(value: UInt32(timelineLimit))