From 2432f0d97ae9244d61f0507a4826177b5338451d Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Fri, 22 Apr 2022 15:58:03 +0200 Subject: [PATCH] Rewrite ProfileView as a view of currentProfile Do not load profile in View, instead: - Load active profile on app launch - Load selected profile on organizer selection --- Passepartout/App/Constants/AppContext.swift | 8 ++- Passepartout/App/Views/MainView.swift | 2 +- .../App/Views/OrganizerView+Profiles.swift | 66 ++++++++----------- Passepartout/App/Views/ProfileView+VPN.swift | 10 +-- Passepartout/App/Views/ProfileView.swift | 53 +++------------ .../Managers/ProfileManager.swift | 46 +++++++++++-- .../Models/ObservableProfile.swift | 2 +- 7 files changed, 92 insertions(+), 95 deletions(-) diff --git a/Passepartout/App/Constants/AppContext.swift b/Passepartout/App/Constants/AppContext.swift index 47d9850b..6c93a574 100644 --- a/Passepartout/App/Constants/AppContext.swift +++ b/Passepartout/App/Constants/AppContext.swift @@ -132,7 +132,13 @@ class AppContext { profileManager.availabilityFilter = { self.isEligibleProfile(withHeader: $0) } - profileManager.activeProfileId = appManager.activeProfileId + if let activeProfileId = appManager.activeProfileId { + do { + try profileManager.loadActiveProfile(withId: activeProfileId) + } catch { + pp_log.warning("Unable to load active profile: \(error)") + } + } providerManager.rateLimitMilliseconds = Constants.RateLimit.providerManager vpnManager.rateLimitMilliseconds = Constants.RateLimit.vpnManager vpnManager.isOnDemandRulesSupported = { diff --git a/Passepartout/App/Views/MainView.swift b/Passepartout/App/Views/MainView.swift index 7a8f2405..803ef8c8 100644 --- a/Passepartout/App/Views/MainView.swift +++ b/Passepartout/App/Views/MainView.swift @@ -31,7 +31,7 @@ struct MainView: View { OrganizerView() .themePrimaryView() - ProfileView(header: nil) + ProfileView() }.themeGlobal() } } diff --git a/Passepartout/App/Views/OrganizerView+Profiles.swift b/Passepartout/App/Views/OrganizerView+Profiles.swift index 91dce9ea..361f2edd 100644 --- a/Passepartout/App/Views/OrganizerView+Profiles.swift +++ b/Passepartout/App/Views/OrganizerView+Profiles.swift @@ -41,7 +41,7 @@ extension OrganizerView { @State private var isFirstLaunch = true - @State private var selectedProfileId: UUID? + @State private var isPresentingProfile = false init(alertType: Binding) { appManager = .shared @@ -53,7 +53,11 @@ extension OrganizerView { var body: some View { debugChanges() - return Group { + return ZStack { + NavigationLink("", isActive: $isPresentingProfile) { + ProfileView() + }.onAppear(perform: presentActiveProfile) + mainView if profileManager.headers.isEmpty { emptyView @@ -66,14 +70,14 @@ extension OrganizerView { // from AddProfileView .onReceive(profileManager.didCreateProfile) { - selectedProfileId = $0.id + presentProfile(withId: $0.id) } } private var mainView: some View { List { Section { - ForEach(sortedHeaders, content: navigationLink(forHeader:)) + ForEach(sortedHeaders, content: profileButton(forHeader:)) .onDelete(perform: removeProfiles) } }.animation(.default, value: profileManager.headers) @@ -86,27 +90,14 @@ extension OrganizerView { } } - private func navigationLink(forHeader header: Profile.Header) -> some View { - NavigationLink(tag: header.id, selection: $selectedProfileId) { - ProfileView(header: header) + private func profileButton(forHeader header: Profile.Header) -> some View { + Button { + presentProfile(withId: header.id) } label: { ProfileHeaderRow( header: header, isActive: profileManager.isActiveProfile(header.id) ) - }.onAppear { - preselectIfActiveProfile(header.id) - - // XXX: iOS 14 bug, if selectedProfileId is set before its NavigationLink - // has appeared, the NavigationLink will not auto-activate once appeared - // enforce activation by clearing and resetting selectedProfileId to its - // current value - withAnimation { - if let tmp = selectedProfileId, tmp == header.id { - selectedProfileId = nil - selectedProfileId = tmp - } - } } } } @@ -117,22 +108,21 @@ extension OrganizerView.ProfilesList { profileManager.headers.sorted() } - private func preselectIfActiveProfile(_ id: UUID) { - - // do not push profile if: - // - // - an alert is active, as it would break navigation - // - on iPad, as it's already shown - // - guard alertType == nil, themeIdiom != .pad, id == profileManager.activeHeader?.id else { - return - } - guard isFirstLaunch else { + private func presentActiveProfile() { + guard isFirstLaunch, profileManager.hasActiveProfile else { return } isFirstLaunch = false - - selectedProfileId = id + isPresentingProfile = true + } + + private func presentProfile(withId id: UUID) { + do { + try profileManager.loadCurrentProfile(withId: id, makeReady: true) + isPresentingProfile = true + } catch { + pp_log.error("Unable to load profile: \(error)") + } } private func performMigrationsIfNeeded() { @@ -149,18 +139,16 @@ extension OrganizerView.ProfilesList { } // clear selection before removal to avoid triggering a bogus navigation push - if let selectedProfileId = selectedProfileId, toDelete.contains(selectedProfileId) { - self.selectedProfileId = nil + if toDelete.contains(profileManager.currentProfile.value.id) { + isPresentingProfile = false } profileManager.removeProfiles(withIds: toDelete) } private func dismissSelectionIfDeleted(headers: [Profile.Header]) { - if let selectedProfileId = selectedProfileId, - !profileManager.isExistingProfile(withId: selectedProfileId) { - - self.selectedProfileId = nil + if isPresentingProfile, !profileManager.isCurrentProfileExisting() { + isPresentingProfile = false } } } diff --git a/Passepartout/App/Views/ProfileView+VPN.swift b/Passepartout/App/Views/ProfileView+VPN.swift index 561c0e32..745480ac 100644 --- a/Passepartout/App/Views/ProfileView+VPN.swift +++ b/Passepartout/App/Views/ProfileView+VPN.swift @@ -42,7 +42,7 @@ extension ProfileView { @ObservedObject private var currentProfile: ObservableProfile - private let isLoaded: Bool + private let isLoading: Bool private var isActiveProfile: Bool { profileManager.isCurrentProfileActive() @@ -52,7 +52,7 @@ extension ProfileView { productManager.isEligible(forFeature: .siriShortcuts) } - init(currentProfile: ObservableProfile, isLoaded: Bool) { + init(currentProfile: ObservableProfile, isLoading: Bool) { appManager = .shared profileManager = .shared providerManager = .shared @@ -60,11 +60,11 @@ extension ProfileView { currentVPNState = .shared productManager = .shared self.currentProfile = currentProfile - self.isLoaded = isLoaded + self.isLoading = isLoading } var body: some View { - if isLoaded { + if !isLoading { if isActiveProfile { activeView } else { @@ -114,7 +114,7 @@ extension ProfileView { profileManager.activateCurrentProfile() // IMPORTANT: save immediately to keep in sync with VPN status - appManager.activeProfileId = profileManager.activeProfileId + appManager.activeProfileId = profileManager.activeHeader?.id } Task { await vpnManager.disable() diff --git a/Passepartout/App/Views/ProfileView.swift b/Passepartout/App/Views/ProfileView.swift index d1e44653..6ee9023d 100644 --- a/Passepartout/App/Views/ProfileView.swift +++ b/Passepartout/App/Views/ProfileView.swift @@ -47,21 +47,18 @@ struct ProfileView: View { @ObservedObject private var profileManager: ProfileManager - private let header: Profile.Header + private var isLoading: Bool { + profileManager.isLoadingCurrentProfile + } private var isExisting: Bool { - profileManager.isExistingProfile(withId: header.id) + profileManager.isCurrentProfileExisting() } @State private var modalType: ModalType? - @State private var isLoaded = false - - init(header: Profile.Header?) { - let profileManager: ProfileManager = .shared - - self.profileManager = profileManager - self.header = header ?? profileManager.activeHeader ?? Profile.placeholder.header + init() { + profileManager = .shared } var body: some View { @@ -84,22 +81,21 @@ struct ProfileView: View { ).disabled(!isExisting) } }.sheet(item: $modalType, content: presentedModal) - .onAppear(perform: loadProfileIfNeeded) .navigationTitle(title) .themeSecondaryView() } private var title: String { - isExisting ? header.name : "" + profileManager.currentProfile.name } private var mainView: some View { List { VPNSection( currentProfile: profileManager.currentProfile, - isLoaded: isLoaded + isLoading: isLoading ) - if isLoaded { + if !isLoading { ProviderSection(currentProfile: profileManager.currentProfile) ConfigurationSection( currentProfile: profileManager.currentProfile, @@ -156,37 +152,6 @@ struct ProfileView: View { }.themeGlobal() } } - - private func loadProfileIfNeeded() { - guard !isLoaded else { - return - } - guard !header.isPlaceholder else { - pp_log.debug("ProfileView is a placeholder for WelcomeView, no active profile") - return - } - do { - let result = try profileManager.loadCurrentProfile(withId: header.id) - if result.isReady { - isLoaded = true - return - } - Task { - do { - try await profileManager.makeProfileReady(result.profile) - withAnimation { - isLoaded = true - } - } catch { - pp_log.error("Profile \(header.id) could not be made ready: \(error)") - presentationMode.wrappedValue.dismiss() - } - } - } catch { - pp_log.error("Profile \(header.id) could not be loaded: \(error)") - presentationMode.wrappedValue.dismiss() - } - } private func presentPaywallTrustedNetworks() { modalType = .paywallTrustedNetworks diff --git a/PassepartoutCore/Sources/PassepartoutProfiles/Managers/ProfileManager.swift b/PassepartoutCore/Sources/PassepartoutProfiles/Managers/ProfileManager.swift index 40eabeda..cc472986 100644 --- a/PassepartoutCore/Sources/PassepartoutProfiles/Managers/ProfileManager.swift +++ b/PassepartoutCore/Sources/PassepartoutProfiles/Managers/ProfileManager.swift @@ -47,7 +47,7 @@ public class ProfileManager: ObservableObject { public var availabilityFilter: ((Profile.Header) -> Bool)? - public var activeProfileId: UUID? { + private var activeProfileId: UUID? { willSet { willUpdateActiveId.send(newValue) } @@ -58,6 +58,8 @@ public class ProfileManager: ObservableObject { // MARK: Observables + @Published public private(set) var isLoadingCurrentProfile = false + public let currentProfile: ObservableProfile public let willUpdateActiveId = PassthroughSubject() @@ -85,6 +87,15 @@ public class ProfileManager: ObservableObject { currentProfile = ObservableProfile() } + + public func loadActiveProfile(withId id: UUID) throws { + guard isExistingProfile(withId: id) else { + pp_log.warning("Active profile \(id) does not exist, ignoring") + return + } + activeProfileId = id + try loadCurrentProfile(withId: id, makeReady: true) + } } // MARK: Index @@ -104,6 +115,10 @@ extension ProfileManager { public var headers: [Profile.Header] { availableHeaders } + + public var hasActiveProfile: Bool { + activeHeader != nil + } public var activeHeader: Profile.Header? { availableHeaders.first { @@ -232,7 +247,16 @@ extension ProfileManager { // MARK: Observation extension ProfileManager { - public func loadCurrentProfile(withId id: UUID) throws -> LoadResult { + public func loadCurrentProfile(withId id: UUID, makeReady: Bool) throws { + guard !isLoadingCurrentProfile else { + pp_log.warning("Already loading another profile") + return + } + guard id != currentProfile.value.id else { + pp_log.debug("Profile \(id) is already current profile") + return + } + isLoadingCurrentProfile = true if isExistingProfile(withId: currentProfile.value.id) { pp_log.info("Committing changes of former current profile \(currentProfile.value.logDescription)") saveCurrentProfile() @@ -240,14 +264,28 @@ extension ProfileManager { do { let result = try loadProfile(withId: id) pp_log.info("Current profile: \(result.profile.logDescription)") - currentProfile.value = result.profile - return result + + if !makeReady || result.isReady { + currentProfile.value = result.profile + isLoadingCurrentProfile = false + } else { + Task { + try await makeProfileReady(result.profile) + currentProfile.value = result.profile + isLoadingCurrentProfile = false + } + } } catch { currentProfile.value = .placeholder + isLoadingCurrentProfile = false throw error } } + public func isCurrentProfileExisting() -> Bool { + isExistingProfile(withId: currentProfile.value.id) + } + public func isCurrentProfileActive() -> Bool { currentProfile.value.id == activeProfileId } diff --git a/PassepartoutCore/Sources/PassepartoutProfiles/Models/ObservableProfile.swift b/PassepartoutCore/Sources/PassepartoutProfiles/Models/ObservableProfile.swift index da446bda..42f0e8e7 100644 --- a/PassepartoutCore/Sources/PassepartoutProfiles/Models/ObservableProfile.swift +++ b/PassepartoutCore/Sources/PassepartoutProfiles/Models/ObservableProfile.swift @@ -30,7 +30,7 @@ public class ObservableProfile: ValueHolder, ObservableObject { @Published public var value: Profile public var name: String { - value.header.name + !value.header.isPlaceholder ? value.header.name : "" } public init() {