Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix back pagination #432

Merged
merged 2 commits into from
Jan 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ struct RoomScreenViewState: BindableState {
var roomTitle = ""
var roomAvatar: UIImage?
var items: [RoomTimelineViewProvider] = []
var canBackPaginate = true
var isBackPaginating = false
var showLoading = false
var bindings: RoomScreenViewStateBindings
Expand Down
12 changes: 8 additions & 4 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,14 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
}

self.state.items[viewIndex] = timelineViewFactory.buildTimelineViewFor(timelineItem: timelineItem)
case .startedBackPaginating:
self.state.isBackPaginating = true
case .finishedBackPaginating:
self.state.isBackPaginating = false
case .canBackPaginate(let canBackPaginate):
if self.state.canBackPaginate != canBackPaginate {
self.state.canBackPaginate = canBackPaginate
}
case .isBackPaginating(let isBackPaginating):
if self.state.isBackPaginating != isBackPaginating {
self.state.isBackPaginating = isBackPaginating
}
}
}
.store(in: &cancellables)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ class TimelineTableViewController: UIViewController {
}
}

/// 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 {
Expand Down Expand Up @@ -334,7 +337,8 @@ class TimelineTableViewController: UIViewController {
///
/// Prefer not to call this directly, instead using ``paginateBackwardsPublisher`` to throttle requests.
private func paginateBackwardsIfNeeded() {
guard !isBackPaginating,
guard canBackPaginate,
!isBackPaginating,
!hasPendingUpdates,
tableView.contentOffset.y < tableView.visibleSize.height * 2.0
else { return }
Expand Down
3 changes: 3 additions & 0 deletions ElementX/Sources/Screens/RoomScreen/View/TimelineView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ struct TimelineView: UIViewControllerRepresentable {
if tableViewController.timelineItems != context.viewState.items {
tableViewController.timelineItems = context.viewState.items
}
if tableViewController.canBackPaginate != context.viewState.canBackPaginate {
tableViewController.canBackPaginate = context.viewState.canBackPaginate
}
if tableViewController.isBackPaginating != context.viewState.isBackPaginating {
tableViewController.isBackPaginating = context.viewState.isBackPaginating
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import Combine
struct MockRoomTimelineProvider: RoomTimelineProviderProtocol {
var itemsPublisher = CurrentValueSubject<[TimelineItemProxy], Never>([])

var backPaginationPublisher = CurrentValueSubject<Bool, Never>(false)

private var itemProxies = [TimelineItemProxy]()

func paginateBackwards(requestSize: UInt, untilNumberOfItems: UInt) async -> Result<Void, RoomTimelineProviderError> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,10 @@ class RoomTimelineProvider: RoomTimelineProviderProtocol {
private var cancellables = Set<AnyCancellable>()

let itemsPublisher = CurrentValueSubject<[TimelineItemProxy], Never>([])
let backPaginationPublisher = CurrentValueSubject<Bool, Never>(false)

private var itemProxies: [TimelineItemProxy] {
didSet {
itemsPublisher.send(itemProxies)

if backPaginationPublisher.value == true {
backPaginationPublisher.send(false)
}
}
}

Expand Down Expand Up @@ -69,17 +64,13 @@ class RoomTimelineProvider: RoomTimelineProviderProtocol {
}

func paginateBackwards(requestSize: UInt, untilNumberOfItems: UInt) async -> Result<Void, RoomTimelineProviderError> {
// Set this back to false after actually updating the items or if failed
backPaginationPublisher.send(true)

MXLog.info("Started back pagination request")
switch await roomProxy.paginateBackwards(requestSize: requestSize, untilNumberOfItems: untilNumberOfItems) {
case .success:
MXLog.info("Finished back pagination request")
return .success(())
case .failure(let error):
MXLog.error("Failed back pagination request with error: \(error)")
backPaginationPublisher.send(false)

if error == .noMoreMessagesToBackPaginate {
return .failure(.noMoreMessagesToBackPaginate)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ enum RoomTimelineProviderError: Error {
protocol RoomTimelineProviderProtocol {
var itemsPublisher: CurrentValueSubject<[TimelineItemProxy], Never> { get }

var backPaginationPublisher: CurrentValueSubject<Bool, Never> { get }

func paginateBackwards(requestSize: UInt, untilNumberOfItems: UInt) async -> Result<Void, RoomTimelineProviderError>

func sendMessage(_ message: String, inReplyToItemId: String?) async -> Result<Void, RoomTimelineProviderError>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,20 +42,7 @@ class MockRoomTimelineController: RoomTimelineControllerProtocol {
}

func paginateBackwards(requestSize: UInt, untilNumberOfItems: UInt) async -> Result<Void, RoomTimelineControllerError> {
callbacks.send(.startedBackPaginating)

guard !backPaginationResponses.isEmpty else {
callbacks.send(.finishedBackPaginating)
return .failure(.generic)
}

let newItems = backPaginationResponses.removeFirst()

try? await Task.sleep(for: backPaginationDelay)
timelineItems.insert(contentsOf: newItems, at: 0)
callbacks.send(.updatedTimelineItems)
callbacks.send(.finishedBackPaginating)

callbacks.send(.canBackPaginate(false))
return .success(())
}

Expand Down Expand Up @@ -125,11 +112,12 @@ class MockRoomTimelineController: RoomTimelineControllerProtocol {
/// Prepends the next chunk of items to the `timelineItems` array.
private func simulateBackPagination() async throws {
guard !backPaginationResponses.isEmpty else { return }
callbacks.send(.isBackPaginating(true))

let newItems = backPaginationResponses.removeFirst()
timelineItems.insert(contentsOf: newItems, at: 0)
callbacks.send(.updatedTimelineItems)
callbacks.send(.finishedBackPaginating)
callbacks.send(.isBackPaginating(false))

try? await connection?.send(.success)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,6 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
}
.store(in: &cancellables)

self.timelineProvider
.backPaginationPublisher
.receive(on: DispatchQueue.main)
.sink { [weak self] value in
if value {
self?.callbacks.send(.startedBackPaginating)
} else {
self?.callbacks.send(.finishedBackPaginating)
}
}
.store(in: &cancellables)

updateTimelineItems()

NotificationCenter.default.addObserver(self, selector: #selector(contentSizeCategoryDidChange), name: UIContentSizeCategory.didChangeNotification, object: nil)
Expand Down Expand Up @@ -230,6 +218,8 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
// swiftlint:disable:next cyclomatic_complexity
private func asyncUpdateTimelineItems() async {
var newTimelineItems = [RoomTimelineItemProtocol]()
var canBackPaginate = true
var isBackPaginating = false

for (index, itemProxy) in timelineProvider.itemsPublisher.value.enumerated() {
if Task.isCancelled {
Expand Down Expand Up @@ -262,8 +252,10 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
}
case .loadingIndicator:
newTimelineItems.append(PaginationIndicatorRoomTimelineItem())
isBackPaginating = true
case .timelineStart:
newTimelineItems.append(TimelineStartRoomTimelineItem(name: roomProxy.displayName ?? roomProxy.name))
canBackPaginate = false
}
default:
break
Expand All @@ -277,6 +269,8 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
timelineItems = newTimelineItems

callbacks.send(.updatedTimelineItems)
callbacks.send(.canBackPaginate(canBackPaginate))
callbacks.send(.isBackPaginating(isBackPaginating))
}

private func computeGroupState(for itemProxy: TimelineItemProxy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import UIKit
enum RoomTimelineControllerCallback {
case updatedTimelineItems
case updatedTimelineItem(_ itemId: String)
case startedBackPaginating
case finishedBackPaginating
case canBackPaginate(Bool)
case isBackPaginating(Bool)
}

enum RoomTimelineControllerAction {
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion UITests/SupportingFiles/target.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ targets:
- path: ../SupportingFiles
- path: ../../Tools/Scripts/Templates/SimpleScreenExample/Tests/UI
- path: ../../ElementX/Sources/UITests/UITestScreenIdentifier.swift
- path: ../../ElementX/Sources/UITests/UITestSignalling.swift
- path: ../../ElementX/Sources/UITests/UITestsSignalling.swift
- path: ../../ElementX/Sources/Generated/Strings.swift
- path: ../../ElementX/Sources/Generated/Strings+Untranslated.swift
- path: ../../ElementX/Resources
Expand Down
1 change: 1 addition & 0 deletions changelog.d/pr-432.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use pagination indicators and start of room timeline items to update the view's pagination state.