From 219703ecc7c3ff495f3395bcaeccabc46b5745e5 Mon Sep 17 00:00:00 2001 From: Thomas Ricouard Date: Sun, 11 Feb 2024 10:58:51 +0100 Subject: [PATCH] Refactor to NextPageView + handle next page loading failure --- .../Account/AccountDetailViewModel.swift | 85 +++++++++---------- .../AccountsList/AccountsListView.swift | 30 ++----- .../AccountsList/AccountsListViewModel.swift | 60 ++++++------- .../AccountStatusesListViewModel.swift | 16 ++-- .../DesignSystem/Views/NextPageView.swift | 53 ++++++++++++ .../Explore/TrendingLinksListView.swift | 18 +--- .../Notifications/NotificationsListView.swift | 43 ++++------ .../NotificationsViewModel.swift | 31 +++---- .../StatusKit/List/StatusesFetcher.swift | 4 +- .../StatusKit/List/StatusesListView.swift | 30 ++----- .../Timeline/View/TimelineViewModel.swift | 32 ++++--- 11 files changed, 187 insertions(+), 215 deletions(-) create mode 100644 Packages/DesignSystem/Sources/DesignSystem/Views/NextPageView.swift diff --git a/Packages/Account/Sources/Account/AccountDetailViewModel.swift b/Packages/Account/Sources/Account/AccountDetailViewModel.swift index 7b1242c4..b578bbf4 100644 --- a/Packages/Account/Sources/Account/AccountDetailViewModel.swift +++ b/Packages/Account/Sources/Account/AccountDetailViewModel.swift @@ -191,55 +191,46 @@ import SwiftUI } } - func fetchNextPage() async { + func fetchNextPage() async throws { guard let client else { return } - do { - switch selectedTab { - case .statuses, .replies, .boosts, .media: - guard let lastId = statuses.last?.id else { return } - if selectedTab == .boosts { - statusesState = .display(statuses: boosts, nextPageState: .loadingNextPage) - } else { - statusesState = .display(statuses: statuses, nextPageState: .loadingNextPage) - } - let newStatuses: [Status] = - try await client.get(endpoint: Accounts.statuses(id: accountId, - sinceId: lastId, - tag: nil, - onlyMedia: selectedTab == .media, - excludeReplies: selectedTab != .replies, - excludeReblogs: selectedTab != .boosts, - pinned: nil)) - statuses.append(contentsOf: newStatuses) - if selectedTab == .boosts { - let newBoosts = statuses.filter{ $0.reblog != nil } - self.boosts.append(contentsOf: newBoosts) - } - StatusDataControllerProvider.shared.updateDataControllers(for: newStatuses, client: client) - if selectedTab == .boosts { - statusesState = .display(statuses: boosts, - nextPageState: newStatuses.count < 20 ? .none : .hasNextPage) - } else { - statusesState = .display(statuses: statuses, - nextPageState: newStatuses.count < 20 ? .none : .hasNextPage) - } - case .favorites: - guard let nextPageId = favoritesNextPage?.maxId else { return } - let newFavorites: [Status] - (newFavorites, favoritesNextPage) = try await client.getWithLink(endpoint: Accounts.favorites(sinceId: nextPageId)) - favorites.append(contentsOf: newFavorites) - StatusDataControllerProvider.shared.updateDataControllers(for: newFavorites, client: client) - statusesState = .display(statuses: favorites, nextPageState: .hasNextPage) - case .bookmarks: - guard let nextPageId = bookmarksNextPage?.maxId else { return } - let newBookmarks: [Status] - (newBookmarks, bookmarksNextPage) = try await client.getWithLink(endpoint: Accounts.bookmarks(sinceId: nextPageId)) - StatusDataControllerProvider.shared.updateDataControllers(for: newBookmarks, client: client) - bookmarks.append(contentsOf: newBookmarks) - statusesState = .display(statuses: bookmarks, nextPageState: .hasNextPage) + switch selectedTab { + case .statuses, .replies, .boosts, .media: + guard let lastId = statuses.last?.id else { return } + let newStatuses: [Status] = + try await client.get(endpoint: Accounts.statuses(id: accountId, + sinceId: lastId, + tag: nil, + onlyMedia: selectedTab == .media, + excludeReplies: selectedTab != .replies, + excludeReblogs: selectedTab != .boosts, + pinned: nil)) + statuses.append(contentsOf: newStatuses) + if selectedTab == .boosts { + let newBoosts = statuses.filter{ $0.reblog != nil } + self.boosts.append(contentsOf: newBoosts) } - } catch { - statusesState = .error(error: error) + StatusDataControllerProvider.shared.updateDataControllers(for: newStatuses, client: client) + if selectedTab == .boosts { + statusesState = .display(statuses: boosts, + nextPageState: newStatuses.count < 20 ? .none : .hasNextPage) + } else { + statusesState = .display(statuses: statuses, + nextPageState: newStatuses.count < 20 ? .none : .hasNextPage) + } + case .favorites: + guard let nextPageId = favoritesNextPage?.maxId else { return } + let newFavorites: [Status] + (newFavorites, favoritesNextPage) = try await client.getWithLink(endpoint: Accounts.favorites(sinceId: nextPageId)) + favorites.append(contentsOf: newFavorites) + StatusDataControllerProvider.shared.updateDataControllers(for: newFavorites, client: client) + statusesState = .display(statuses: favorites, nextPageState: .hasNextPage) + case .bookmarks: + guard let nextPageId = bookmarksNextPage?.maxId else { return } + let newBookmarks: [Status] + (newBookmarks, bookmarksNextPage) = try await client.getWithLink(endpoint: Accounts.bookmarks(sinceId: nextPageId)) + StatusDataControllerProvider.shared.updateDataControllers(for: newBookmarks, client: client) + bookmarks.append(contentsOf: newBookmarks) + statusesState = .display(statuses: bookmarks, nextPageState: .hasNextPage) } } diff --git a/Packages/Account/Sources/Account/AccountsList/AccountsListView.swift b/Packages/Account/Sources/Account/AccountsList/AccountsListView.swift index b2dfe5e0..04296b66 100644 --- a/Packages/Account/Sources/Account/AccountsList/AccountsListView.swift +++ b/Packages/Account/Sources/Account/AccountsList/AccountsListView.swift @@ -134,21 +134,13 @@ public struct AccountsListView: View { switch nextPageState { case .hasNextPage: - loadingRow - #if !os(visionOS) - .listRowBackground(theme.primaryBackgroundColor) - #endif - .onAppear { - Task { - await viewModel.fetchNextPage() - } - } - - case .loadingNextPage: - loadingRow - #if !os(visionOS) - .listRowBackground(theme.primaryBackgroundColor) - #endif + NextPageView { + try await viewModel.fetchNextPage() + } + #if !os(visionOS) + .listRowBackground(theme.primaryBackgroundColor) + #endif + case .none: EmptyView() } @@ -160,12 +152,4 @@ public struct AccountsListView: View { #endif } } - - private var loadingRow: some View { - HStack { - Spacer() - ProgressView() - Spacer() - } - } } diff --git a/Packages/Account/Sources/Account/AccountsList/AccountsListViewModel.swift b/Packages/Account/Sources/Account/AccountsList/AccountsListViewModel.swift index bc919e5b..1cc51ae5 100644 --- a/Packages/Account/Sources/Account/AccountsList/AccountsListViewModel.swift +++ b/Packages/Account/Sources/Account/AccountsList/AccountsListViewModel.swift @@ -33,7 +33,7 @@ public enum AccountsListMode { public enum State { public enum PagingState { - case hasNextPage, loadingNextPage, none + case hasNextPage, none } case loading @@ -94,42 +94,36 @@ public enum AccountsListMode { } catch {} } - func fetchNextPage() async { + func fetchNextPage() async throws { guard let client, let nextPageId else { return } - do { - state = .display(accounts: accounts, relationships: relationships, nextPageState: .loadingNextPage) - let newAccounts: [Account] - let link: LinkHandler? - switch mode { - case let .followers(accountId): - (newAccounts, link) = try await client.getWithLink(endpoint: Accounts.followers(id: accountId, + let newAccounts: [Account] + let link: LinkHandler? + switch mode { + case let .followers(accountId): + (newAccounts, link) = try await client.getWithLink(endpoint: Accounts.followers(id: accountId, + maxId: nextPageId)) + case let .following(accountId): + (newAccounts, link) = try await client.getWithLink(endpoint: Accounts.following(id: accountId, + maxId: nextPageId)) + case let .rebloggedBy(statusId): + (newAccounts, link) = try await client.getWithLink(endpoint: Statuses.rebloggedBy(id: statusId, maxId: nextPageId)) - case let .following(accountId): - (newAccounts, link) = try await client.getWithLink(endpoint: Accounts.following(id: accountId, + case let .favoritedBy(statusId): + (newAccounts, link) = try await client.getWithLink(endpoint: Statuses.favoritedBy(id: statusId, maxId: nextPageId)) - case let .rebloggedBy(statusId): - (newAccounts, link) = try await client.getWithLink(endpoint: Statuses.rebloggedBy(id: statusId, - maxId: nextPageId)) - case let .favoritedBy(statusId): - (newAccounts, link) = try await client.getWithLink(endpoint: Statuses.favoritedBy(id: statusId, - maxId: nextPageId)) - case .accountsList: - newAccounts = [] - link = nil - } - accounts.append(contentsOf: newAccounts) - let newRelationships: [Relationship] = - try await client.get(endpoint: Accounts.relationships(ids: newAccounts.map(\.id))) - - relationships.append(contentsOf: newRelationships) - self.nextPageId = link?.maxId - state = .display(accounts: accounts, - relationships: relationships, - nextPageState: link?.maxId != nil ? .hasNextPage : .none) - } catch { - let logger = Logger(subsystem: "com.icecubesapp", category: "UI") - logger.log(level: .info, "\(error.localizedDescription)") + case .accountsList: + newAccounts = [] + link = nil } + accounts.append(contentsOf: newAccounts) + let newRelationships: [Relationship] = + try await client.get(endpoint: Accounts.relationships(ids: newAccounts.map(\.id))) + + relationships.append(contentsOf: newRelationships) + self.nextPageId = link?.maxId + state = .display(accounts: accounts, + relationships: relationships, + nextPageState: link?.maxId != nil ? .hasNextPage : .none) } func search() async { diff --git a/Packages/Account/Sources/Account/StatusesLists/AccountStatusesListViewModel.swift b/Packages/Account/Sources/Account/StatusesLists/AccountStatusesListViewModel.swift index 377ddb1b..cf56fdfd 100644 --- a/Packages/Account/Sources/Account/StatusesLists/AccountStatusesListViewModel.swift +++ b/Packages/Account/Sources/Account/StatusesLists/AccountStatusesListViewModel.swift @@ -53,18 +53,14 @@ public class AccountStatusesListViewModel: StatusesFetcher { } } - public func fetchNextPage() async { + public func fetchNextPage() async throws { guard let client, let nextId = nextPage?.maxId else { return } + var newStatuses: [Status] = [] + (newStatuses, nextPage) = try await client.getWithLink(endpoint: mode.endpoint(sinceId: nextId)) + statuses.append(contentsOf: newStatuses) + StatusDataControllerProvider.shared.updateDataControllers(for: statuses, client: client) statusesState = .display(statuses: statuses, - nextPageState: .loadingNextPage) - do { - var newStatuses: [Status] = [] - (newStatuses, nextPage) = try await client.getWithLink(endpoint: mode.endpoint(sinceId: nextId)) - statuses.append(contentsOf: newStatuses) - StatusDataControllerProvider.shared.updateDataControllers(for: statuses, client: client) - statusesState = .display(statuses: statuses, - nextPageState: nextPage?.maxId != nil ? .hasNextPage : .none) - } catch { } + nextPageState: nextPage?.maxId != nil ? .hasNextPage : .none) } public func statusDidAppear(status: Status) { diff --git a/Packages/DesignSystem/Sources/DesignSystem/Views/NextPageView.swift b/Packages/DesignSystem/Sources/DesignSystem/Views/NextPageView.swift new file mode 100644 index 00000000..cf3ecb1c --- /dev/null +++ b/Packages/DesignSystem/Sources/DesignSystem/Views/NextPageView.swift @@ -0,0 +1,53 @@ +import SwiftUI + +public struct NextPageView: View { + @State private var isLoadingNextPage: Bool = false + @State private var showRetry: Bool = false + + let loadNextPage: (() async throws -> Void) + + public init(loadNextPage: @escaping (() async throws -> Void)) { + self.loadNextPage = loadNextPage + } + + public var body: some View { + HStack { + Spacer() + if showRetry { + Button { + Task { + showRetry = false + await executeTask() + } + } label: { + Label("action.retry", systemImage: "arrow.clockwise") + } + .buttonStyle(.bordered) + } else { + Label("placeholder.loading.short", systemImage: "arrow.down") + .font(.footnote) + .foregroundStyle(.secondary) + .symbolEffect(.pulse, value: isLoadingNextPage) + } + Spacer() + } + .task { + await executeTask() + } + .listRowSeparator(.hidden, edges: .all) + } + + private func executeTask() async { + showRetry = false + defer { + isLoadingNextPage = false + } + guard !isLoadingNextPage else { return } + isLoadingNextPage = true + do { + try await loadNextPage() + } catch { + showRetry = true + } + } +} diff --git a/Packages/Explore/Sources/Explore/TrendingLinksListView.swift b/Packages/Explore/Sources/Explore/TrendingLinksListView.swift index 5d9cfac6..0992de1c 100644 --- a/Packages/Explore/Sources/Explore/TrendingLinksListView.swift +++ b/Packages/Explore/Sources/Explore/TrendingLinksListView.swift @@ -25,25 +25,13 @@ public struct TrendingLinksListView: View { #endif .padding(.vertical, 8) } - HStack { - Spacer() - ProgressView() - Spacer() + NextPageView { + let nextPage: [Card] = try await client.get(endpoint: Trends.links(offset: links.count)) + links.append(contentsOf: nextPage) } #if !os(visionOS) .listRowBackground(theme.primaryBackgroundColor) #endif - .task { - defer { - isLoadingNextPage = false - } - guard !isLoadingNextPage else { return } - isLoadingNextPage = true - do { - let nextPage: [Card] = try await client.get(endpoint: Trends.links(offset: links.count)) - links.append(contentsOf: nextPage) - } catch { } - } } #if os(visionOS) .listStyle(.insetGrouped) diff --git a/Packages/Notifications/Sources/Notifications/NotificationsListView.swift b/Packages/Notifications/Sources/Notifications/NotificationsListView.swift index 8c547f61..cbf93cdc 100644 --- a/Packages/Notifications/Sources/Notifications/NotificationsListView.swift +++ b/Packages/Notifications/Sources/Notifications/NotificationsListView.swift @@ -180,20 +180,22 @@ public struct NotificationsListView: View { #endif .id(notification.id) } - } - - switch nextPageState { - case .none: - EmptyView() - case .hasNextPage: - loadingRow - .onAppear { - Task { - await viewModel.fetchNextPage() - } + + switch nextPageState { + case .none: + EmptyView() + case .hasNextPage: + NextPageView { + try await viewModel.fetchNextPage() } - case .loadingNextPage: - loadingRow + .listRowInsets(.init(top: .layoutPadding, + leading: .layoutPadding + 4, + bottom: .layoutPadding, + trailing: .layoutPadding)) + #if !os(visionOS) + .listRowBackground(theme.primaryBackgroundColor) + #endif + } } case .error: @@ -212,21 +214,6 @@ public struct NotificationsListView: View { } } - private var loadingRow: some View { - HStack { - Spacer() - ProgressView() - Spacer() - } - .listRowInsets(.init(top: .layoutPadding, - leading: .layoutPadding + 4, - bottom: .layoutPadding, - trailing: .layoutPadding)) - #if !os(visionOS) - .listRowBackground(theme.primaryBackgroundColor) - #endif - } - private var topPaddingView: some View { HStack {} .listRowBackground(Color.clear) diff --git a/Packages/Notifications/Sources/Notifications/NotificationsViewModel.swift b/Packages/Notifications/Sources/Notifications/NotificationsViewModel.swift index 49ec6c9e..5f44108b 100644 --- a/Packages/Notifications/Sources/Notifications/NotificationsViewModel.swift +++ b/Packages/Notifications/Sources/Notifications/NotificationsViewModel.swift @@ -9,7 +9,7 @@ import SwiftUI @Observable class NotificationsViewModel { public enum State { public enum PagingState { - case none, hasNextPage, loadingNextPage + case none, hasNextPage } case loading @@ -143,25 +143,20 @@ import SwiftUI return allNotifications } - func fetchNextPage() async { + func fetchNextPage() async throws { guard let client else { return } - do { - guard let lastId = consolidatedNotifications.last?.notificationIds.last else { return } - state = .display(notifications: consolidatedNotifications, nextPageState: .loadingNextPage) - let newNotifications: [Models.Notification] = - try await client.get(endpoint: Notifications.notifications(minId: nil, - maxId: lastId, - types: queryTypes, - limit: Constants.notificationLimit)) - await consolidatedNotifications.append(contentsOf: newNotifications.consolidated(selectedType: selectedType)) - if consolidatedNotifications.contains(where: { $0.type == .follow_request }) { - await currentAccount?.fetchFollowerRequests() - } - state = .display(notifications: consolidatedNotifications, - nextPageState: newNotifications.count < Constants.notificationLimit ? .none : .hasNextPage) - } catch { - state = .error(error: error) + guard let lastId = consolidatedNotifications.last?.notificationIds.last else { return } + let newNotifications: [Models.Notification] = + try await client.get(endpoint: Notifications.notifications(minId: nil, + maxId: lastId, + types: queryTypes, + limit: Constants.notificationLimit)) + await consolidatedNotifications.append(contentsOf: newNotifications.consolidated(selectedType: selectedType)) + if consolidatedNotifications.contains(where: { $0.type == .follow_request }) { + await currentAccount?.fetchFollowerRequests() } + state = .display(notifications: consolidatedNotifications, + nextPageState: newNotifications.count < Constants.notificationLimit ? .none : .hasNextPage) } func markAsRead() { diff --git a/Packages/StatusKit/Sources/StatusKit/List/StatusesFetcher.swift b/Packages/StatusKit/Sources/StatusKit/List/StatusesFetcher.swift index a90427d8..ab91435a 100644 --- a/Packages/StatusKit/Sources/StatusKit/List/StatusesFetcher.swift +++ b/Packages/StatusKit/Sources/StatusKit/List/StatusesFetcher.swift @@ -5,7 +5,7 @@ import SwiftUI public enum StatusesState { public enum PagingState { - case hasNextPage, loadingNextPage, none + case hasNextPage, none } case loading @@ -17,7 +17,7 @@ public enum StatusesState { public protocol StatusesFetcher { var statusesState: StatusesState { get } func fetchNewestStatuses(pullToRefresh: Bool) async - func fetchNextPage() async + func fetchNextPage() async throws func statusDidAppear(status: Status) func statusDidDisappear(status: Status) } diff --git a/Packages/StatusKit/Sources/StatusKit/List/StatusesListView.swift b/Packages/StatusKit/Sources/StatusKit/List/StatusesListView.swift index 913f586d..68f301ce 100644 --- a/Packages/StatusKit/Sources/StatusKit/List/StatusesListView.swift +++ b/Packages/StatusKit/Sources/StatusKit/List/StatusesListView.swift @@ -60,31 +60,17 @@ public struct StatusesListView: View where Fetcher: StatusesFetcher { } switch nextPageState { case .hasNextPage: - loadingRow - .id(UUID().uuidString) - .onAppear { - Task { - await fetcher.fetchNextPage() - } - } - case .loadingNextPage: - loadingRow - .id(UUID().uuidString) + NextPageView { + try await fetcher.fetchNextPage() + } + .padding(.horizontal, .layoutPadding) + #if !os(visionOS) + .listRowBackground(theme.primaryBackgroundColor) + #endif + case .none: EmptyView() } } } - - private var loadingRow: some View { - HStack { - Spacer() - ProgressView() - Spacer() - } - .padding(.horizontal, .layoutPadding) - #if !os(visionOS) - .listRowBackground(theme.primaryBackgroundColor) - #endif - } } diff --git a/Packages/Timeline/Sources/Timeline/View/TimelineViewModel.swift b/Packages/Timeline/Sources/Timeline/View/TimelineViewModel.swift index 919bcf0f..7434b620 100644 --- a/Packages/Timeline/Sources/Timeline/View/TimelineViewModel.swift +++ b/Packages/Timeline/Sources/Timeline/View/TimelineViewModel.swift @@ -417,26 +417,24 @@ extension TimelineViewModel: StatusesFetcher { return allStatuses } - func fetchNextPage() async { - guard let client else { return } - do { - let statuses = await datasource.get() - guard let lastId = statuses.last?.id else { return } - statusesState = await .display(statuses: datasource.getFiltered(), nextPageState: .loadingNextPage) - let newStatuses: [Status] = try await client.get(endpoint: timeline.endpoint(sinceId: nil, - maxId: lastId, - minId: nil, - offset: statuses.count)) + enum NextPageError: Error { + case internalError + } + + func fetchNextPage() async throws { + let statuses = await datasource.get() + guard let client, let lastId = statuses.last?.id else { throw NextPageError.internalError } + let newStatuses: [Status] = try await client.get(endpoint: timeline.endpoint(sinceId: nil, + maxId: lastId, + minId: nil, + offset: statuses.count)) - await datasource.append(contentOf: newStatuses) - StatusDataControllerProvider.shared.updateDataControllers(for: newStatuses, client: client) + await datasource.append(contentOf: newStatuses) + StatusDataControllerProvider.shared.updateDataControllers(for: newStatuses, client: client) - statusesState = await .display(statuses: datasource.getFiltered(), - nextPageState: newStatuses.count < 20 ? .none : .hasNextPage) - } catch { - statusesState = .error(error: error) - } + statusesState = await .display(statuses: datasource.getFiltered(), + nextPageState: newStatuses.count < 20 ? .none : .hasNextPage) } func statusDidAppear(status: Status) {