From e71b22c7c8797da4517b479f3ac62896a7c57d6f Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Thu, 21 Apr 2022 15:08:40 +0200 Subject: [PATCH] Simplify AddProfileView with implicit animations - Animate on ViewModel in profile name views - Animate on providers in provider selection view --- Passepartout/App/Views/AddHostView.swift | 26 ++++++---------- Passepartout/App/Views/AddHostViewModel.swift | 2 +- .../App/Views/AddProviderView+Name.swift | 18 ++++------- Passepartout/App/Views/AddProviderView.swift | 31 ++++++++----------- .../App/Views/AddProviderViewModel.swift | 9 +----- 5 files changed, 30 insertions(+), 56 deletions(-) diff --git a/Passepartout/App/Views/AddHostView.swift b/Passepartout/App/Views/AddHostView.swift index f122f554..9262bbed 100644 --- a/Passepartout/App/Views/AddHostView.swift +++ b/Passepartout/App/Views/AddHostView.swift @@ -60,7 +60,7 @@ struct AddHostView: View { } else { completeView } - } + }.animation(.default, value: viewModel) // hidden NavigationLink("", isActive: $isEnteringCredentials) { @@ -159,31 +159,23 @@ struct AddHostView: View { title: Text(L10n.AddProfile.Shared.title), message: Text(L10n.AddProfile.Shared.Alerts.Overwrite.message), primaryButton: .destructive(Text(L10n.Global.Strings.ok)) { - - // XXX: delay withAnimation() to not overlap with alert dismiss animation - Task { - processProfile(replacingExisting: true) - } + processProfile(replacingExisting: true) }, secondaryButton: .cancel(Text(L10n.Global.Strings.cancel)) ) } private func processProfile(replacingExisting: Bool) { - withAnimation { - viewModel.processURL( - url, - with: profileManager, - replacingExisting: replacingExisting, - deletingURLOnSuccess: deletingURLOnSuccess - ) - } + viewModel.processURL( + url, + with: profileManager, + replacingExisting: replacingExisting, + deletingURLOnSuccess: deletingURLOnSuccess + ) } private func saveProfile() { - let result = withAnimation { - viewModel.addProcessedProfile(to: profileManager) - } + let result = viewModel.addProcessedProfile(to: profileManager) guard result else { return } diff --git a/Passepartout/App/Views/AddHostViewModel.swift b/Passepartout/App/Views/AddHostViewModel.swift index cb82b660..0b6ca65e 100644 --- a/Passepartout/App/Views/AddHostViewModel.swift +++ b/Passepartout/App/Views/AddHostViewModel.swift @@ -29,7 +29,7 @@ import TunnelKitOpenVPN import TunnelKitWireGuard extension AddHostView { - struct ViewModel { + struct ViewModel: Equatable { private var isNamePreset = false var profileName = "" diff --git a/Passepartout/App/Views/AddProviderView+Name.swift b/Passepartout/App/Views/AddProviderView+Name.swift index dcfd706f..dbce1863 100644 --- a/Passepartout/App/Views/AddProviderView+Name.swift +++ b/Passepartout/App/Views/AddProviderView+Name.swift @@ -96,24 +96,18 @@ extension AddProviderView { title: Text(L10n.AddProfile.Shared.title), message: Text(L10n.AddProfile.Shared.Alerts.Overwrite.message), primaryButton: .destructive(Text(L10n.Global.Strings.ok)) { - - // XXX: delay withAnimation() to not overlap with alert dismiss animation - Task { - saveProfile(replacingExisting: true) - } + saveProfile(replacingExisting: true) }, secondaryButton: .cancel(Text(L10n.Global.Strings.cancel)) ) } private func saveProfile(replacingExisting: Bool) { - let addedProfile = withAnimation { - viewModel.addProfile( - profile, - to: profileManager, - replacingExisting: replacingExisting - ) - } + let addedProfile = viewModel.addProfile( + profile, + to: profileManager, + replacingExisting: replacingExisting + ) guard let addedProfile = addedProfile else { return } diff --git a/Passepartout/App/Views/AddProviderView.swift b/Passepartout/App/Views/AddProviderView.swift index aec2c079..58e1204e 100644 --- a/Passepartout/App/Views/AddProviderView.swift +++ b/Passepartout/App/Views/AddProviderView.swift @@ -41,9 +41,16 @@ struct AddProviderView: View { self.bindings = bindings } + private var providers: [ProviderMetadata] { + providerManager.allProviders() + .filter { + $0.supportedVPNProtocols.contains(viewModel.selectedVPNProtocol) + }.sorted() + } + private var availableVPNProtocols: [VPNProtocolType] { var protos: Set = [] - viewModel.providers.forEach { + providers.forEach { $0.supportedVPNProtocols.forEach { protos.insert($0) } @@ -70,16 +77,17 @@ struct AddProviderView: View { ScrollViewReader { scrollProxy in List { mainSection - if !viewModel.providers.isEmpty { + if !providers.isEmpty { providersSection } }.onChange(of: viewModel.errorMessage) { onErrorMessage($0, scrollProxy) }.disabled(viewModel.pendingOperation != nil) + .animation(.default, value: providers) } // hidden - ForEach(viewModel.providers, id: \.navigationId, content: providerNavigationLink) + ForEach(providers, id: \.navigationId, content: providerNavigationLink) }.themeSecondaryView() .navigationTitle(L10n.AddProfile.Shared.title) .toolbar(content: toolbar) @@ -87,12 +95,6 @@ struct AddProviderView: View { NavigationView { PaywallView(isPresented: $viewModel.isPaywallPresented) }.themeGlobal() - }.onAppear { - refreshProviders() - }.onChange(of: viewModel.newProviders) { newValue in - withAnimation { - refreshProviders(newValue) - } } } @@ -122,7 +124,7 @@ struct AddProviderView: View { Section( footer: themeErrorMessage(viewModel.errorMessage) ) { - ForEach(viewModel.providers, content: providerRow) + ForEach(providers, content: providerRow) } } @@ -150,13 +152,6 @@ struct AddProviderView: View { }.withTrailingProgress(when: isUpdatingIndex) } - private func refreshProviders(_ newProviders: [ProviderMetadata]? = nil) { - viewModel.providers = (newProviders ?? providerManager.allProviders()) - .filter { - $0.supportedVPNProtocols.contains(viewModel.selectedVPNProtocol) - }.sorted() - } - // eligibility: select or purchase provider private func presentOrPurchaseProvider(_ metadata: ProviderMetadata) { if productManager.isEligible(forProvider: metadata.name) { @@ -176,7 +171,7 @@ struct AddProviderView: View { extension AddProviderView { private func scrollToErrorMessage(_ proxy: ScrollViewProxy) { - proxy.maybeScrollTo(viewModel.providers.last?.id, animated: true) + proxy.maybeScrollTo(providers.last?.id, animated: true) } } diff --git a/Passepartout/App/Views/AddProviderViewModel.swift b/Passepartout/App/Views/AddProviderViewModel.swift index 6c603193..e222f00c 100644 --- a/Passepartout/App/Views/AddProviderViewModel.swift +++ b/Passepartout/App/Views/AddProviderViewModel.swift @@ -34,11 +34,6 @@ extension AddProviderView { case provider(ProviderName) } - // local copy for animations - @Published var providers: [ProviderMetadata] = [] - - @Published private(set) var newProviders: [ProviderMetadata] = [] - @Published var selectedVPNProtocol: VPNProtocolType = .openVPN @Published var selectedProvider: ProviderMetadata? @@ -107,8 +102,6 @@ extension AddProviderView { try await providerManager.fetchProvidersIndexPublisher( priority: .remoteThenBundle ).async() - - newProviders = providerManager.allProviders() } catch { errorMessage = error.localizedDescription } @@ -123,7 +116,7 @@ extension AddProviderView { } extension AddProviderView.NameView { - struct ViewModel { + struct ViewModel: Equatable { private var isNamePreset = false var profileName = ""