From 1ffb4ab901c115731c7f8ccf17063f9e1c2e54fb Mon Sep 17 00:00:00 2001 From: Thomas Ricouard Date: Tue, 9 Jul 2024 16:04:24 +0200 Subject: [PATCH] Cleanup scrollToTop --- IceCubesApp/App/AppRegistry.swift | 11 +- IceCubesApp/App/Main/AppView.swift | 22 ++-- IceCubesApp/App/Tabs/ExploreTab.swift | 3 +- IceCubesApp/App/Tabs/MessagesTab.swift | 3 +- IceCubesApp/App/Tabs/NotificationTab.swift | 3 +- IceCubesApp/App/Tabs/ProfileTab.swift | 5 +- .../App/Tabs/Timeline/TimelineTab.swift | 2 - .../Sources/Account/AccountDetailView.swift | 15 +-- .../List/ConversationsListView.swift | 11 +- .../Explore/Sources/Explore/ExploreView.swift | 15 +-- .../Notifications/NotificationsListView.swift | 10 +- .../Sources/Timeline/View/TimelineView.swift | 118 ++++++++---------- 12 files changed, 79 insertions(+), 139 deletions(-) diff --git a/IceCubesApp/App/AppRegistry.swift b/IceCubesApp/App/AppRegistry.swift index 398d35a7..62029115 100644 --- a/IceCubesApp/App/AppRegistry.swift +++ b/IceCubesApp/App/AppRegistry.swift @@ -19,9 +19,9 @@ extension View { navigationDestination(for: RouterDestination.self) { destination in switch destination { case let .accountDetail(id): - AccountDetailView(accountId: id, scrollToTopSignal: .constant(0)) + AccountDetailView(accountId: id) case let .accountDetailWithAccount(account): - AccountDetailView(account: account, scrollToTopSignal: .constant(0)) + AccountDetailView(account: account) case let .accountSettingsWithAccount(account, appAccount): AccountSettingsView(account: account, appAccount: appAccount) case let .statusDetail(id): @@ -36,19 +36,16 @@ extension View { TimelineView(timeline: .constant(.hashtag(tag: tag, accountId: accountId)), pinnedFilters: .constant([]), selectedTagGroup: .constant(nil), - scrollToTopSignal: .constant(0), canFilterTimeline: false) case let .list(list): TimelineView(timeline: .constant(.list(list: list)), pinnedFilters: .constant([]), selectedTagGroup: .constant(nil), - scrollToTopSignal: .constant(0), canFilterTimeline: false) case let .linkTimeline(url, title): TimelineView(timeline: .constant(.link(url: url, title: title)), pinnedFilters: .constant([]), selectedTagGroup: .constant(nil), - scrollToTopSignal: .constant(0), canFilterTimeline: false) case let .following(id): AccountsListView(mode: .following(accountId: id)) @@ -64,7 +61,6 @@ extension View { TimelineView(timeline: .constant(.trending), pinnedFilters: .constant([]), selectedTagGroup: .constant(nil), - scrollToTopSignal: .constant(0), canFilterTimeline: false) case let .trendingLinks(cards): TrendingLinksListView(cards: cards) @@ -74,8 +70,7 @@ extension View { NotificationsRequestsListView() case let .notificationForAccount(accountId): NotificationsListView(lockedType: nil, - lockedAccountId: accountId, - scrollToTopSignal: .constant(0)) + lockedAccountId: accountId) case .blockedAccounts: AccountsListView(mode: .blocked) case .mutedAccounts: diff --git a/IceCubesApp/App/Main/AppView.swift b/IceCubesApp/App/Main/AppView.swift index ad12a41c..8f061649 100644 --- a/IceCubesApp/App/Main/AppView.swift +++ b/IceCubesApp/App/Main/AppView.swift @@ -70,18 +70,18 @@ struct AppView: View { selectedTab = newTab })) { ForEach(availableTabs) { tab in - tab.makeContentView(selectedTab: $selectedTab) - .tabItem { - if userPreferences.showiPhoneTabLabel { - tab.label - .environment(\.symbolVariants, tab == selectedTab ? .fill : .none) - } else { - Image(systemName: tab.iconName) - } + Tab(value: tab) { + tab.makeContentView(selectedTab: $selectedTab) + .toolbarBackground(theme.primaryBackgroundColor.opacity(0.30), for: .tabBar) + } label: { + if userPreferences.showiPhoneTabLabel { + tab.label + .environment(\.symbolVariants, tab == selectedTab ? .fill : .none) + } else { + Image(systemName: tab.iconName) } - .tag(tab) - .badge(badgeFor(tab: tab)) - .toolbarBackground(theme.primaryBackgroundColor.opacity(0.30), for: .tabBar) + } + .badge(badgeFor(tab: tab)) } } .id(appAccountsManager.currentClient.id) diff --git a/IceCubesApp/App/Tabs/ExploreTab.swift b/IceCubesApp/App/Tabs/ExploreTab.swift index e36234d5..cac3cb2f 100644 --- a/IceCubesApp/App/Tabs/ExploreTab.swift +++ b/IceCubesApp/App/Tabs/ExploreTab.swift @@ -13,11 +13,10 @@ struct ExploreTab: View { @Environment(CurrentAccount.self) private var currentAccount @Environment(Client.self) private var client @State private var routerPath = RouterPath() - @State private var scrollToTopSignal: Int = 0 var body: some View { NavigationStack(path: $routerPath.path) { - ExploreView(scrollToTopSignal: $scrollToTopSignal) + ExploreView() .withAppRouter() .withSheetDestinations(sheetDestinations: $routerPath.presentedSheet) .toolbarBackground(theme.primaryBackgroundColor.opacity(0.30), for: .navigationBar) diff --git a/IceCubesApp/App/Tabs/MessagesTab.swift b/IceCubesApp/App/Tabs/MessagesTab.swift index 76e73e61..bf8f01f4 100644 --- a/IceCubesApp/App/Tabs/MessagesTab.swift +++ b/IceCubesApp/App/Tabs/MessagesTab.swift @@ -15,11 +15,10 @@ struct MessagesTab: View { @Environment(CurrentAccount.self) private var currentAccount @Environment(AppAccountsManager.self) private var appAccount @State private var routerPath = RouterPath() - @State private var scrollToTopSignal: Int = 0 var body: some View { NavigationStack(path: $routerPath.path) { - ConversationsListView(scrollToTopSignal: $scrollToTopSignal) + ConversationsListView() .withAppRouter() .withSheetDestinations(sheetDestinations: $routerPath.presentedSheet) .toolbar { diff --git a/IceCubesApp/App/Tabs/NotificationTab.swift b/IceCubesApp/App/Tabs/NotificationTab.swift index 2ab6f805..301e60ae 100644 --- a/IceCubesApp/App/Tabs/NotificationTab.swift +++ b/IceCubesApp/App/Tabs/NotificationTab.swift @@ -20,7 +20,6 @@ struct NotificationsTab: View { @Environment(UserPreferences.self) private var userPreferences @Environment(PushNotificationsService.self) private var pushNotificationsService @State private var routerPath = RouterPath() - @State private var scrollToTopSignal: Int = 0 @Binding var selectedTab: AppTab @@ -28,7 +27,7 @@ struct NotificationsTab: View { var body: some View { NavigationStack(path: $routerPath.path) { - NotificationsListView(lockedType: lockedType, scrollToTopSignal: $scrollToTopSignal) + NotificationsListView(lockedType: lockedType) .withAppRouter() .withSheetDestinations(sheetDestinations: $routerPath.presentedSheet) .toolbar { diff --git a/IceCubesApp/App/Tabs/ProfileTab.swift b/IceCubesApp/App/Tabs/ProfileTab.swift index 12670823..c63e7836 100644 --- a/IceCubesApp/App/Tabs/ProfileTab.swift +++ b/IceCubesApp/App/Tabs/ProfileTab.swift @@ -14,18 +14,17 @@ struct ProfileTab: View { @Environment(Client.self) private var client @Environment(CurrentAccount.self) private var currentAccount @State private var routerPath = RouterPath() - @State private var scrollToTopSignal: Int = 0 var body: some View { NavigationStack(path: $routerPath.path) { if let account = currentAccount.account { - AccountDetailView(account: account, scrollToTopSignal: $scrollToTopSignal) + AccountDetailView(account: account) .withAppRouter() .withSheetDestinations(sheetDestinations: $routerPath.presentedSheet) .toolbarBackground(theme.primaryBackgroundColor.opacity(0.30), for: .navigationBar) .id(account.id) } else { - AccountDetailView(account: .placeholder(), scrollToTopSignal: $scrollToTopSignal) + AccountDetailView(account: .placeholder()) .redacted(reason: .placeholder) .allowsHitTesting(false) } diff --git a/IceCubesApp/App/Tabs/Timeline/TimelineTab.swift b/IceCubesApp/App/Tabs/Timeline/TimelineTab.swift index 172ed8ba..0ce19f0f 100644 --- a/IceCubesApp/App/Tabs/Timeline/TimelineTab.swift +++ b/IceCubesApp/App/Tabs/Timeline/TimelineTab.swift @@ -22,7 +22,6 @@ struct TimelineTab: View { @State private var didAppear: Bool = false @State private var timeline: TimelineFilter = .home @State private var selectedTagGroup: TagGroup? - @State private var scrollToTopSignal: Int = 0 @Query(sort: \LocalTimeline.creationDate, order: .reverse) var localTimelines: [LocalTimeline] @Query(sort: \TagGroup.creationDate, order: .reverse) var tagGroups: [TagGroup] @@ -42,7 +41,6 @@ struct TimelineTab: View { TimelineView(timeline: $timeline, pinnedFilters: $pinnedFilters, selectedTagGroup: $selectedTagGroup, - scrollToTopSignal: $scrollToTopSignal, canFilterTimeline: canFilterTimeline) .withAppRouter() .withSheetDestinations(sheetDestinations: $routerPath.presentedSheet) diff --git a/Packages/Account/Sources/Account/AccountDetailView.swift b/Packages/Account/Sources/Account/AccountDetailView.swift index 9bf02fbd..a006531c 100644 --- a/Packages/Account/Sources/Account/AccountDetailView.swift +++ b/Packages/Account/Sources/Account/AccountDetailView.swift @@ -28,18 +28,14 @@ public struct AccountDetailView: View { @State private var displayTitle: Bool = false - @Binding var scrollToTopSignal: Int - /// When coming from a URL like a mention tap in a status. - public init(accountId: String, scrollToTopSignal: Binding) { + public init(accountId: String) { _viewModel = .init(initialValue: .init(accountId: accountId)) - _scrollToTopSignal = scrollToTopSignal } /// When the account is already fetched by the parent caller. - public init(account: Account, scrollToTopSignal: Binding) { + public init(account: Account) { _viewModel = .init(initialValue: .init(account: account)) - _scrollToTopSignal = scrollToTopSignal } public var body: some View { @@ -89,11 +85,6 @@ public struct AccountDetailView: View { .scrollContentBackground(.hidden) .background(theme.primaryBackgroundColor) #endif - .onChange(of: scrollToTopSignal) { - withAnimation { - proxy.scrollTo(ScrollToView.Constants.scrollToTop, anchor: .top) - } - } } .onAppear { guard reasons != .placeholder else { return } @@ -403,6 +394,6 @@ extension View { struct AccountDetailView_Previews: PreviewProvider { static var previews: some View { - AccountDetailView(account: .placeholder(), scrollToTopSignal: .constant(0)) + AccountDetailView(account: .placeholder()) } } diff --git a/Packages/Conversations/Sources/Conversations/List/ConversationsListView.swift b/Packages/Conversations/Sources/Conversations/List/ConversationsListView.swift index bffc0460..4bd49c97 100644 --- a/Packages/Conversations/Sources/Conversations/List/ConversationsListView.swift +++ b/Packages/Conversations/Sources/Conversations/List/ConversationsListView.swift @@ -14,11 +14,7 @@ public struct ConversationsListView: View { @State private var viewModel = ConversationsListViewModel() - @Binding var scrollToTopSignal: Int - - public init(scrollToTopSignal: Binding) { - _scrollToTopSignal = scrollToTopSignal - } + public init() { } private var conversations: Binding<[Conversation]> { if viewModel.isLoadingFirstPage { @@ -89,11 +85,6 @@ public struct ConversationsListView: View { viewModel.handleEvent(event: latestEvent) } } - .onChange(of: scrollToTopSignal) { - withAnimation { - proxy.scrollTo(ScrollToView.Constants.scrollToTop, anchor: .top) - } - } .refreshable { // note: this Task wrapper should not be necessary, but it reportedly crashes without it // when refreshing on an empty list diff --git a/Packages/Explore/Sources/Explore/ExploreView.swift b/Packages/Explore/Sources/Explore/ExploreView.swift index be1d3ae5..48b515b3 100644 --- a/Packages/Explore/Sources/Explore/ExploreView.swift +++ b/Packages/Explore/Sources/Explore/ExploreView.swift @@ -14,11 +14,7 @@ public struct ExploreView: View { @State private var viewModel = ExploreViewModel() - @Binding var scrollToTopSignal: Int - - public init(scrollToTopSignal: Binding) { - _scrollToTopSignal = scrollToTopSignal - } + public init() { } public var body: some View { ScrollViewReader { proxy in @@ -111,15 +107,6 @@ public struct ExploreView: View { .task(id: viewModel.searchQuery) { await viewModel.search() } - .onChange(of: scrollToTopSignal) { - if viewModel.scrollToTopVisible { - viewModel.isSearchPresented.toggle() - } else { - withAnimation { - proxy.scrollTo(ScrollToView.Constants.scrollToTop, anchor: .top) - } - } - } } } diff --git a/Packages/Notifications/Sources/Notifications/NotificationsListView.swift b/Packages/Notifications/Sources/Notifications/NotificationsListView.swift index a90f82f2..e0ab4f0e 100644 --- a/Packages/Notifications/Sources/Notifications/NotificationsListView.swift +++ b/Packages/Notifications/Sources/Notifications/NotificationsListView.swift @@ -17,18 +17,15 @@ public struct NotificationsListView: View { @State private var viewModel = NotificationsViewModel() @State private var isNotificationsPolicyPresented: Bool = false - @Binding var scrollToTopSignal: Int let lockedType: Models.Notification.NotificationType? let lockedAccountId: String? public init(lockedType: Models.Notification.NotificationType? = nil, - lockedAccountId: String? = nil, - scrollToTopSignal: Binding) + lockedAccountId: String? = nil) { self.lockedType = lockedType self.lockedAccountId = lockedAccountId - _scrollToTopSignal = scrollToTopSignal } public var body: some View { @@ -44,11 +41,6 @@ public struct NotificationsListView: View { .id(account.account?.id) .environment(\.defaultMinListRowHeight, 1) .listStyle(.plain) - .onChange(of: scrollToTopSignal) { - withAnimation { - proxy.scrollTo(ScrollToView.Constants.scrollToTop, anchor: .top) - } - } } .toolbar { ToolbarItem(placement: .principal) { diff --git a/Packages/Timeline/Sources/Timeline/View/TimelineView.swift b/Packages/Timeline/Sources/Timeline/View/TimelineView.swift index 47db3c4d..e759d660 100644 --- a/Packages/Timeline/Sources/Timeline/View/TimelineView.swift +++ b/Packages/Timeline/Sources/Timeline/View/TimelineView.swift @@ -27,7 +27,6 @@ public struct TimelineView: View { @Binding var timeline: TimelineFilter @Binding var pinnedFilters: [TimelineFilter] @Binding var selectedTagGroup: TagGroup? - @Binding var scrollToTopSignal: Int @Query(sort: \TagGroup.creationDate, order: .reverse) var tagGroups: [TagGroup] @@ -36,82 +35,73 @@ public struct TimelineView: View { public init(timeline: Binding, pinnedFilters: Binding<[TimelineFilter]>, selectedTagGroup: Binding, - scrollToTopSignal: Binding, canFilterTimeline: Bool) { _timeline = timeline _pinnedFilters = pinnedFilters _selectedTagGroup = selectedTagGroup - _scrollToTopSignal = scrollToTopSignal self.canFilterTimeline = canFilterTimeline } public var body: some View { - ScrollViewReader { proxy in - ZStack(alignment: .top) { - List { - scrollToTopView - TimelineTagGroupheaderView(group: $selectedTagGroup, timeline: $timeline) - TimelineTagHeaderView(tag: $viewModel.tag) - switch viewModel.timeline { - case .remoteLocal: - StatusesListView(fetcher: viewModel, client: client, routerPath: routerPath, isRemote: true) - default: - StatusesListView(fetcher: viewModel, client: client, routerPath: routerPath) - .environment(\.isHomeTimeline, timeline == .home) + ZStack(alignment: .top) { + List { + scrollToTopView + TimelineTagGroupheaderView(group: $selectedTagGroup, timeline: $timeline) + TimelineTagHeaderView(tag: $viewModel.tag) + switch viewModel.timeline { + case .remoteLocal: + StatusesListView(fetcher: viewModel, client: client, routerPath: routerPath, isRemote: true) + default: + StatusesListView(fetcher: viewModel, client: client, routerPath: routerPath) + .environment(\.isHomeTimeline, timeline == .home) + } + } + .id(client.id) + .environment(\.defaultMinListRowHeight, 1) + .listStyle(.plain) + #if !os(visionOS) + .scrollContentBackground(.hidden) + .background(theme.primaryBackgroundColor) + #endif + .introspect(.list, on: .iOS(.v17, .v18)) { (collectionView: UICollectionView) in + DispatchQueue.main.async { + self.collectionView = collectionView } + prefetcher.viewModel = viewModel + collectionView.isPrefetchingEnabled = true + collectionView.prefetchDataSource = prefetcher } - .id(client.id) - .environment(\.defaultMinListRowHeight, 1) - .listStyle(.plain) - #if !os(visionOS) - .scrollContentBackground(.hidden) - .background(theme.primaryBackgroundColor) - #endif - .introspect(.list, on: .iOS(.v17, .v18)) { (collectionView: UICollectionView) in - DispatchQueue.main.async { - self.collectionView = collectionView - } - prefetcher.viewModel = viewModel - collectionView.isPrefetchingEnabled = true - collectionView.prefetchDataSource = prefetcher - } - if viewModel.timeline.supportNewestPagination { - TimelineUnreadStatusesView(observer: viewModel.pendingStatusesObserver) + if viewModel.timeline.supportNewestPagination { + TimelineUnreadStatusesView(observer: viewModel.pendingStatusesObserver) + } + } + .safeAreaInset(edge: .top, spacing: 0) { + if canFilterTimeline, !pinnedFilters.isEmpty { + VStack(spacing: 0) { + TimelineQuickAccessPills(pinnedFilters: $pinnedFilters, timeline: $timeline) + .padding(.vertical, 8) + .padding(.horizontal, .layoutPadding) + .background(theme.primaryBackgroundColor.opacity(0.30)) + .background(Material.ultraThin) + Divider() } } - .safeAreaInset(edge: .top, spacing: 0) { - if canFilterTimeline, !pinnedFilters.isEmpty { - VStack(spacing: 0) { - TimelineQuickAccessPills(pinnedFilters: $pinnedFilters, timeline: $timeline) - .padding(.vertical, 8) - .padding(.horizontal, .layoutPadding) - .background(theme.primaryBackgroundColor.opacity(0.30)) - .background(Material.ultraThin) - Divider() - } - } - } - .if(canFilterTimeline && !pinnedFilters.isEmpty) { view in - view.toolbarBackground(.hidden, for: .navigationBar) - } - .onChange(of: viewModel.scrollToIndex) { _, newValue in - if let collectionView, - let newValue, - let rows = collectionView.dataSource?.collectionView(collectionView, numberOfItemsInSection: 0), - rows > newValue - { - collectionView.scrollToItem(at: .init(row: newValue, section: 0), - at: .top, - animated: viewModel.scrollToIndexAnimated) - viewModel.scrollToIndexAnimated = false - viewModel.scrollToIndex = nil - } - } - .onChange(of: scrollToTopSignal) { - withAnimation { - proxy.scrollTo(ScrollToView.Constants.scrollToTop, anchor: .top) - } + } + .if(canFilterTimeline && !pinnedFilters.isEmpty) { view in + view.toolbarBackground(.hidden, for: .navigationBar) + } + .onChange(of: viewModel.scrollToIndex) { _, newValue in + if let collectionView, + let newValue, + let rows = collectionView.dataSource?.collectionView(collectionView, numberOfItemsInSection: 0), + rows > newValue + { + collectionView.scrollToItem(at: .init(row: newValue, section: 0), + at: .top, + animated: viewModel.scrollToIndexAnimated) + viewModel.scrollToIndexAnimated = false + viewModel.scrollToIndex = nil } } .toolbar {