From 52eff96ab4ba558b1f9c6900eb4500127b227f7e Mon Sep 17 00:00:00 2001 From: Thomas Ricouard Date: Tue, 31 Jan 2023 12:17:35 +0100 Subject: [PATCH] Iron out timeline issues with the new behaviour --- .../Account/AccountDetailViewModel.swift | 6 ++-- .../Sources/Status/List/StatusesFetcher.swift | 1 + .../Status/List/StatusesListView.swift | 3 ++ .../Status/Row/StatusMediaPreviewView.swift | 14 +++++--- .../Timeline/PendingStatusesObserver.swift | 7 +++- .../Sources/Timeline/TimelineView.swift | 5 ++- .../Sources/Timeline/TimelineViewModel.swift | 36 ++++++++++++++++--- 7 files changed, 57 insertions(+), 15 deletions(-) diff --git a/Packages/Account/Sources/Account/AccountDetailViewModel.swift b/Packages/Account/Sources/Account/AccountDetailViewModel.swift index 7e157eda..1ea0d714 100644 --- a/Packages/Account/Sources/Account/AccountDetailViewModel.swift +++ b/Packages/Account/Sources/Account/AccountDetailViewModel.swift @@ -247,7 +247,7 @@ class AccountDetailViewModel: ObservableObject, StatusesFetcher { } } - func statusDidAppear(status: Models.Status) { - - } + func statusDidAppear(status: Models.Status) { } + + func statusDidDisappear(status: Status) { } } diff --git a/Packages/Status/Sources/Status/List/StatusesFetcher.swift b/Packages/Status/Sources/Status/List/StatusesFetcher.swift index 45facbcc..07f3d36d 100644 --- a/Packages/Status/Sources/Status/List/StatusesFetcher.swift +++ b/Packages/Status/Sources/Status/List/StatusesFetcher.swift @@ -17,4 +17,5 @@ public protocol StatusesFetcher: ObservableObject { func fetchStatuses() async func fetchNextPage() async func statusDidAppear(status: Status) + func statusDidDisappear(status: Status) } diff --git a/Packages/Status/Sources/Status/List/StatusesListView.swift b/Packages/Status/Sources/Status/List/StatusesListView.swift index 1a228b98..91d22692 100644 --- a/Packages/Status/Sources/Status/List/StatusesListView.swift +++ b/Packages/Status/Sources/Status/List/StatusesListView.swift @@ -59,6 +59,9 @@ public struct StatusesListView: View where Fetcher: StatusesFetcher { .onAppear { fetcher.statusDidAppear(status: status) } + .onDisappear { + fetcher.statusDidDisappear(status: status) + } if !isEmbdedInList { Divider() .padding(.vertical, .dividerPadding) diff --git a/Packages/Status/Sources/Status/Row/StatusMediaPreviewView.swift b/Packages/Status/Sources/Status/Row/StatusMediaPreviewView.swift index d58b96c3..ead97791 100644 --- a/Packages/Status/Sources/Status/Row/StatusMediaPreviewView.swift +++ b/Packages/Status/Sources/Status/Row/StatusMediaPreviewView.swift @@ -199,6 +199,7 @@ public struct StatusMediaPreviewView: View { Text("status.image.alt-text.abbreviation") .font(theme.statusDisplayStyle == .compact ? .footnote : .body) } + .buttonStyle(.borderless) .padding(4) .background(.thinMaterial) .cornerRadius(4) @@ -242,6 +243,7 @@ public struct StatusMediaPreviewView: View { Text("status.image.alt-text.abbreviation") .font(.scaledFootnote) } + .buttonStyle(.borderless) .padding(4) .background(.thinMaterial) .cornerRadius(4) @@ -287,6 +289,7 @@ public struct StatusMediaPreviewView: View { private var sensitiveMediaOverlay: some View { ZStack { Rectangle() + .foregroundColor(.clear) .background(.ultraThinMaterial) if !isNotifications { Button { @@ -294,11 +297,14 @@ public struct StatusMediaPreviewView: View { isHidingMedia = false } } label: { - if sensitive { - Label("status.media.sensitive.show", systemImage: "eye") - } else { - Label("status.media.content.show", systemImage: "eye") + Group { + if sensitive { + Label("status.media.sensitive.show", systemImage: "eye") + } else { + Label("status.media.content.show", systemImage: "eye") + } } + .foregroundColor(theme.labelColor) } .buttonStyle(.borderedProminent) } diff --git a/Packages/Timeline/Sources/Timeline/PendingStatusesObserver.swift b/Packages/Timeline/Sources/Timeline/PendingStatusesObserver.swift index 1aff61db..8407c5fc 100644 --- a/Packages/Timeline/Sources/Timeline/PendingStatusesObserver.swift +++ b/Packages/Timeline/Sources/Timeline/PendingStatusesObserver.swift @@ -4,8 +4,12 @@ import Models @MainActor class PendingStatusesObserver: ObservableObject { + let feedbackGenerator = UIImpactFeedbackGenerator(style: .light) + @Published var pendingStatusesCount: Int = 0 + var disableUpdate: Bool = false + var pendingStatuses: [String] = [] { didSet { pendingStatusesCount = pendingStatuses.count @@ -13,8 +17,9 @@ class PendingStatusesObserver: ObservableObject { } func removeStatus(status: Status) { - if let index = pendingStatuses.firstIndex(of: status.id) { + if !disableUpdate, let index = pendingStatuses.firstIndex(of: status.id) { pendingStatuses.removeSubrange(index...(pendingStatuses.count - 1)) + feedbackGenerator.impactOccurred() } } diff --git a/Packages/Timeline/Sources/Timeline/TimelineView.swift b/Packages/Timeline/Sources/Timeline/TimelineView.swift index dbc33caa..4954b8ec 100644 --- a/Packages/Timeline/Sources/Timeline/TimelineView.swift +++ b/Packages/Timeline/Sources/Timeline/TimelineView.swift @@ -20,7 +20,6 @@ public struct TimelineView: View { @StateObject private var viewModel = TimelineViewModel() - @State private var scrollProxy: ScrollViewProxy? @State private var wasBackgrounded: Bool = false @Binding var timeline: TimelineFilter @@ -58,7 +57,7 @@ public struct TimelineView: View { } } .onAppear { - scrollProxy = proxy + viewModel.scrollProxy = proxy } } .navigationTitle(timeline.localizedTitle()) @@ -85,7 +84,7 @@ public struct TimelineView: View { } .onChange(of: scrollToTopSignal, perform: { _ in withAnimation { - scrollProxy?.scrollTo(Constants.scrollToTop, anchor: .top) + viewModel.scrollProxy?.scrollTo(Constants.scrollToTop, anchor: .top) } }) .onChange(of: timeline) { newTimeline in diff --git a/Packages/Timeline/Sources/Timeline/TimelineViewModel.swift b/Packages/Timeline/Sources/Timeline/TimelineViewModel.swift index ab0d7b00..c5eb28ce 100644 --- a/Packages/Timeline/Sources/Timeline/TimelineViewModel.swift +++ b/Packages/Timeline/Sources/Timeline/TimelineViewModel.swift @@ -16,6 +16,10 @@ class TimelineViewModel: ObservableObject, StatusesFetcher { // Internal source of truth for a timeline. private var statuses: [Status] = [] + private var visibileStatusesIds = Set() + private var isFetchingNewPages: Bool = false + + var scrollProxy: ScrollViewProxy? var pendingStatusesObserver: PendingStatusesObserver = .init() @@ -55,7 +59,7 @@ class TimelineViewModel: ObservableObject, StatusesFetcher { } func fetchStatuses(userIntent: Bool) async { - guard let client else { return } + guard let client, !isFetchingNewPages else { return } do { if statuses.isEmpty { pendingStatusesObserver.pendingStatuses = [] @@ -68,6 +72,7 @@ class TimelineViewModel: ObservableObject, StatusesFetcher { statusesState = .display(statuses: statuses, nextPageState: statuses.count < 20 ? .none : .hasNextPage) } } else if let first = statuses.first { + isFetchingNewPages = true var newStatuses: [Status] = await fetchNewPages(minId: first.id, maxPages: 20) if userIntent || !pendingStatusesEnabled { statuses.insert(contentsOf: newStatuses, at: 0) @@ -80,14 +85,31 @@ class TimelineViewModel: ObservableObject, StatusesFetcher { !statuses.contains(where: { $0.id == status.id }) } pendingStatusesObserver.pendingStatuses.insert(contentsOf: newStatuses.map{ $0.id }, at: 0) - statuses.insert(contentsOf: newStatuses, at: 0) - withAnimation { - statusesState = .display(statuses: statuses, nextPageState: statuses.count < 20 ? .none : .hasNextPage) + pendingStatusesObserver.feedbackGenerator.impactOccurred() + + // High chance the user is scrolled to the top, this is a workaround to keep scroll position when prepending statuses. + if let firstStatusId = statuses.first?.id, visibileStatusesIds.contains(firstStatusId) { + statuses.insert(contentsOf: newStatuses, at: 0) + pendingStatusesObserver.disableUpdate = true + withAnimation { + statusesState = .display(statuses: statuses, nextPageState: statuses.count < 20 ? .none : .hasNextPage) + DispatchQueue.main.async { + self.scrollProxy?.scrollTo(firstStatusId) + self.pendingStatusesObserver.disableUpdate = false + } + } + } else { + statuses.insert(contentsOf: newStatuses, at: 0) + withAnimation { + statusesState = .display(statuses: statuses, nextPageState: statuses.count < 20 ? .none : .hasNextPage) + } } } + isFetchingNewPages = false } } catch { statusesState = .error(error: error) + isFetchingNewPages = false print("timeline parse error: \(error)") } } @@ -141,6 +163,7 @@ class TimelineViewModel: ObservableObject, StatusesFetcher { func handleEvent(event: any StreamEvent, currentAccount: CurrentAccount) { if let event = event as? StreamEventUpdate, pendingStatusesEnabled, + !isFetchingNewPages, !statuses.contains(where: { $0.id == event.status.id }) { pendingStatusesObserver.pendingStatuses.insert(event.status.id, at: 0) @@ -163,5 +186,10 @@ class TimelineViewModel: ObservableObject, StatusesFetcher { func statusDidAppear(status: Status) { pendingStatusesObserver.removeStatus(status: status) + visibileStatusesIds.insert(status.id) + } + + func statusDidDisappear(status: Status) { + visibileStatusesIds.remove(status.id) } }