From d3f4f475fcb9d378bb50e0cbe689ba2e330e3bd7 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Thu, 20 Apr 2023 18:15:06 +0300 Subject: [PATCH] Fixes #641 - Room previews failing to load because of incorrect sliding sync view ranges --- .../Sources/Other/ScrollViewAdapter.swift | 29 ++++++--- .../Screens/HomeScreen/View/HomeScreen.swift | 63 ++++++++----------- changelog.d/641.bugfix | 1 + 3 files changed, 47 insertions(+), 46 deletions(-) create mode 100644 changelog.d/641.bugfix diff --git a/ElementX/Sources/Other/ScrollViewAdapter.swift b/ElementX/Sources/Other/ScrollViewAdapter.swift index 996023d3a..d06d6dbfb 100644 --- a/ElementX/Sources/Other/ScrollViewAdapter.swift +++ b/ElementX/Sources/Other/ScrollViewAdapter.swift @@ -24,30 +24,39 @@ class ScrollViewAdapter: NSObject, UIScrollViewDelegate { scrollView?.delegate = self } } - - var isScrolling = CurrentValueSubject(false) - - private func update(_ scrollView: UIScrollView) { - isScrolling.send(scrollView.isDragging || scrollView.isDecelerating) + + let didScroll = PassthroughSubject() + let isScrolling = CurrentValueSubject(false) + + // MARK: - UIScrollViewDelegate + + func scrollViewDidScroll(_ scrollView: UIScrollView) { + didScroll.send(()) } func scrollViewWillBeginDragging(_ scrollView: UIScrollView) { - update(scrollView) + updateDidScroll(scrollView) } func scrollViewDidEndDragging(_ scrollView: UIScrollView, willDecelerate decelerate: Bool) { - update(scrollView) + updateDidScroll(scrollView) } func scrollViewDidEndScrollingAnimation(_ scrollView: UIScrollView) { - update(scrollView) + updateDidScroll(scrollView) } func scrollViewDidEndDecelerating(_ scrollView: UIScrollView) { - update(scrollView) + updateDidScroll(scrollView) } func scrollViewDidScrollToTop(_ scrollView: UIScrollView) { - update(scrollView) + updateDidScroll(scrollView) + } + + // MARK: - Private + + private func updateDidScroll(_ scrollView: UIScrollView) { + isScrolling.send(scrollView.isDragging || scrollView.isDecelerating) } } diff --git a/ElementX/Sources/Screens/HomeScreen/View/HomeScreen.swift b/ElementX/Sources/Screens/HomeScreen/View/HomeScreen.swift index 371102a0a..b51959649 100644 --- a/ElementX/Sources/Screens/HomeScreen/View/HomeScreen.swift +++ b/ElementX/Sources/Screens/HomeScreen/View/HomeScreen.swift @@ -23,17 +23,8 @@ struct HomeScreen: View { @ObservedObject var context: HomeScreenViewModel.Context - @State private var isViewVisible = false - @State private var scrollViewAdapter = ScrollViewAdapter() @State private var showingLogoutConfirmation = false - @State private var visibleItemIdentifiers = Set() { - didSet { - if isViewVisible { - updateVisibleRange() - } - } - } var body: some View { ScrollView { @@ -69,16 +60,6 @@ struct HomeScreen: View { HomeScreenRoomCell(room: room, context: context) } } - .onAppear { - // Ignore while filtering rooms - guard context.searchQuery.isEmpty else { return } - visibleItemIdentifiers.insert(room.id) - } - .onDisappear { - // Ignore while filtering rooms - guard context.searchQuery.isEmpty else { return } - visibleItemIdentifiers.remove(room.id) - } } } .searchable(text: $context.searchQuery) @@ -86,18 +67,27 @@ struct HomeScreen: View { .disableAutocorrection(true) } } - .onAppear { - isViewVisible = true - } - .onDisappear { - isViewVisible = false - } .introspectScrollView { scrollView in guard scrollView != scrollViewAdapter.scrollView else { return } scrollViewAdapter.scrollView = scrollView } - .onReceive(scrollViewAdapter.isScrolling) { isScrolling in - if !isScrolling { + .onReceive(scrollViewAdapter.didScroll) { _ in + updateVisibleRange() + } + .onReceive(scrollViewAdapter.isScrolling) { _ in + updateVisibleRange() + } + .onChange(of: context.searchQuery) { searchQuery in + if searchQuery.isEmpty { + // Allow the view to update after changing the query + DispatchQueue.main.async { + updateVisibleRange() + } + } + } + .onChange(of: context.viewState.visibleRooms) { _ in + // Give the view a chance to update + DispatchQueue.main.async { updateVisibleRange() } } @@ -225,21 +215,22 @@ struct HomeScreen: View { } private func updateVisibleRange() { - let result = visibleItemIdentifiers.compactMap { itemIdentifier in - context.viewState.rooms.firstIndex { $0.id == itemIdentifier } - }.sorted() - - guard !result.isEmpty else { + guard let scrollView = scrollViewAdapter.scrollView, + context.viewState.rooms.count > 0 else { return } - guard let firstIndex = result.first, let lastIndex = result.last else { - return - } + let adjustedContentSize = max(scrollView.contentSize.height - scrollView.contentInset.top - scrollView.contentInset.bottom, scrollView.bounds.height) + let cellHeight = adjustedContentSize / Double(context.viewState.rooms.count) + let firstIndex = Int(max(0.0, scrollView.contentOffset.y + scrollView.contentInset.top) / cellHeight) + let lastIndex = Int(max(0.0, scrollView.contentOffset.y + scrollView.bounds.height) / cellHeight) + + // Add some extra padding just to be on the safe side let lowerBound = max(0, firstIndex - Constants.slidingWindowBoundsPadding) let upperBound = min(Int(context.viewState.rooms.count), lastIndex + Constants.slidingWindowBoundsPadding) - + + // This will be deduped and throttled on the view model layer context.send(viewAction: .updateVisibleItemRange(range: lowerBound..