From ed28126cf717addfef0e472a5c569810138b2090 Mon Sep 17 00:00:00 2001 From: Davide Date: Tue, 15 Oct 2024 21:34:02 +0200 Subject: [PATCH] Rework OpenVPN view with provider modifiers (#733) Improve rendering and work around some SwiftUI bugs, e.g. with .menu Picker on iOS (use .navigationLink instead). Here goes the hierarchy bottom-up: - ProviderPicker: a Picker wrapper built around ProviderManager - ProviderContentModifier: adds a ProviderPicker on top and replaces the content with a set of provider selectors when a provider is selected - VPNProviderContentModifier: wrapper for ProviderContentModifier that adds a VPN server selector - OpenVPNView: provides a view of specific OpenVPN settings, and adds a credentials selector to the provider/server selectors provided by VPNProviderContentModifier --- .../xcshareddata/swiftpm/Package.resolved | 2 +- Passepartout/Library/Package.swift | 2 +- .../Sources/AppUI/L10n/SwiftGen+Strings.swift | 18 +- .../Resources/en.lproj/Localizable.strings | 7 +- .../AppUI/Views/Modules/OpenVPNView.swift | 132 ++++------- .../Provider/ProviderContentModifier.swift | 215 ++++++++++++++++++ .../Provider/ProviderPanelModifier.swift | 194 ---------------- .../AppUI/Views/Provider/ProviderPicker.swift | 59 +++++ .../AppUI/Views/Provider/VPNFiltersView.swift | 4 +- .../Provider/VPNProviderContentModifier.swift | 122 ++++++++++ .../Provider/VPNProviderServerView.swift | 2 +- 11 files changed, 461 insertions(+), 296 deletions(-) create mode 100644 Passepartout/Library/Sources/AppUI/Views/Provider/ProviderContentModifier.swift delete mode 100644 Passepartout/Library/Sources/AppUI/Views/Provider/ProviderPanelModifier.swift create mode 100644 Passepartout/Library/Sources/AppUI/Views/Provider/ProviderPicker.swift create mode 100644 Passepartout/Library/Sources/AppUI/Views/Provider/VPNProviderContentModifier.swift diff --git a/Passepartout.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Passepartout.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 6f43a034..ea59c084 100644 --- a/Passepartout.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Passepartout.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -41,7 +41,7 @@ "kind" : "remoteSourceControl", "location" : "git@github.com:passepartoutvpn/passepartoutkit-source", "state" : { - "revision" : "aeb982951e2798863e28f55081dd25e2221083e3" + "revision" : "c4182832032fab8fef24386d209572a2c288e28e" } }, { diff --git a/Passepartout/Library/Package.swift b/Passepartout/Library/Package.swift index a4ddb0e4..0eb3f00b 100644 --- a/Passepartout/Library/Package.swift +++ b/Passepartout/Library/Package.swift @@ -28,7 +28,7 @@ let package = Package( ], dependencies: [ // .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", from: "0.9.0"), - .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", revision: "aeb982951e2798863e28f55081dd25e2221083e3"), + .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", revision: "c4182832032fab8fef24386d209572a2c288e28e"), // .package(path: "../../../passepartoutkit-source"), .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-openvpn-openssl", from: "0.9.1"), // .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-openvpn-openssl", revision: "031863a1cd683962a7dfe68e20b91fa820a1ecce"), diff --git a/Passepartout/Library/Sources/AppUI/L10n/SwiftGen+Strings.swift b/Passepartout/Library/Sources/AppUI/L10n/SwiftGen+Strings.swift index 51eeea44..d5872dee 100644 --- a/Passepartout/Library/Sources/AppUI/L10n/SwiftGen+Strings.swift +++ b/Passepartout/Library/Sources/AppUI/L10n/SwiftGen+Strings.swift @@ -209,6 +209,8 @@ public enum Strings { public static let keepAlive = Strings.tr("Localizable", "global.keep_alive", fallback: "Keep-alive") /// Key public static let key = Strings.tr("Localizable", "global.key", fallback: "Key") + /// Loading + public static let loading = Strings.tr("Localizable", "global.loading", fallback: "Loading") /// Method public static let method = Strings.tr("Localizable", "global.method", fallback: "Method") /// Modules @@ -588,9 +590,19 @@ public enum Strings { } } public enum Provider { - public enum Vpn { - /// Refresh infrastructure - public static let refreshInfrastructure = Strings.tr("Localizable", "views.provider.vpn.refresh_infrastructure", fallback: "Refresh infrastructure") + /// Last updated on %@ + public static func lastUpdated(_ p1: Any) -> String { + return Strings.tr("Localizable", "views.provider.last_updated", String(describing: p1), fallback: "Last updated on %@") + } + /// None + public static let noProvider = Strings.tr("Localizable", "views.provider.no_provider", fallback: "None") + /// Refresh infrastructure + public static let refreshInfrastructure = Strings.tr("Localizable", "views.provider.refresh_infrastructure", fallback: "Refresh infrastructure") + /// Select a provider + public static let selectProvider = Strings.tr("Localizable", "views.provider.select_provider", fallback: "Select a provider") + public enum LastUpdated { + /// Loading... + public static let loading = Strings.tr("Localizable", "views.provider.last_updated.loading", fallback: "Loading...") } } public enum Settings { diff --git a/Passepartout/Library/Sources/AppUI/Resources/en.lproj/Localizable.strings b/Passepartout/Library/Sources/AppUI/Resources/en.lproj/Localizable.strings index f8770e2a..2c017c99 100644 --- a/Passepartout/Library/Sources/AppUI/Resources/en.lproj/Localizable.strings +++ b/Passepartout/Library/Sources/AppUI/Resources/en.lproj/Localizable.strings @@ -31,6 +31,7 @@ "global.interface" = "Interface"; "global.keep_alive" = "Keep-alive"; "global.key" = "Key"; +"global.loading" = "Loading"; "global.method" = "Method"; "global.modules" = "Modules"; "global.n_seconds" = "%d seconds"; @@ -128,7 +129,11 @@ "views.profile.rows.add_module" = "Add module"; "views.profile.module_list.section.footer" = "Drag modules to rearrange them, as their order determines priority."; -"views.provider.vpn.refresh_infrastructure" = "Refresh infrastructure"; +"views.provider.no_provider" = "None"; +"views.provider.select_provider" = "Select a provider"; +"views.provider.refresh_infrastructure" = "Refresh infrastructure"; +"views.provider.last_updated" = "Last updated on %@"; +"views.provider.last_updated.loading" = "Loading..."; "views.settings.sections.icloud.footer" = "To erase the iCloud store securely, do so on all your synced devices. This will not affect local profiles."; "views.settings.rows.confirm_quit" = "Ask before quit"; diff --git a/Passepartout/Library/Sources/AppUI/Views/Modules/OpenVPNView.swift b/Passepartout/Library/Sources/AppUI/Views/Modules/OpenVPNView.swift index 47314499..566aa654 100644 --- a/Passepartout/Library/Sources/AppUI/Views/Modules/OpenVPNView.swift +++ b/Passepartout/Library/Sources/AppUI/Views/Modules/OpenVPNView.swift @@ -54,41 +54,27 @@ struct OpenVPNView: View { @Binding private var draft: OpenVPNModule.Builder - @Binding - private var providerId: ProviderID? - - @Binding - private var providerEntity: VPNEntity? - - @StateObject - private var vpnProviderManager = VPNProviderManager() - init(serverConfiguration: OpenVPN.Configuration) { let module = OpenVPNModule.Builder(configurationBuilder: serverConfiguration.builder()) let editor = ProfileEditor(modules: [module]) self.editor = editor _draft = .constant(module) - _providerId = .constant(nil) - _providerEntity = .constant(nil) isServerPushed = true } init(editor: ProfileEditor, module: OpenVPNModule.Builder) { self.editor = editor _draft = editor.binding(forModule: module) - _providerId = editor.binding(forProviderOf: module.id) - _providerEntity = editor.binding(forProviderEntityOf: module.id) isServerPushed = false } var body: some View { - debugChanges() - return contentView + manualView .modifier(providerModifier) + .themeAnimation(on: editor.profile.modulesMetadata, category: .modules) .moduleView(editor: editor, draft: draft, withName: !isServerPushed) .navigationDestination(for: Subroute.self, destination: destination) - .themeAnimation(on: providerId, category: .modules) } } @@ -100,36 +86,53 @@ private extension OpenVPNView { } var providerModifier: some ViewModifier { - ProviderPanelModifier( + VPNProviderContentModifier( + providerId: editor.binding(forProviderOf: draft.id), + selectedEntity: editor.binding(forProviderEntityOf: draft.id), + configurationType: OpenVPN.Configuration.self, isRequired: draft.configurationBuilder == nil, - entityType: VPNEntity.self, - providerId: $providerId, - providerContent: providerContentView, - onSelectProvider: onSelectProvider + providerRows: { + moduleGroup(for: providerAccountRows) + } ) } - @ViewBuilder - func providerContentView(providerId: ProviderID) -> some View { - providerServerRow - moduleGroup(for: accountRows) + var providerAccountRows: [ModuleRow]? { + [.push(caption: Strings.Modules.Openvpn.credentials, route: HashableRoute(Subroute.credentials))] + } +} + +private extension OpenVPNView { + func importConfiguration(from url: URL) { + // TODO: #657, import draft from external URL + } +} + +// MARK: - Destinations + +private extension OpenVPNView { + enum Subroute: Hashable { + case credentials } - var providerServerRow: some View { - NavigationLink(value: Subroute.providerServer) { - HStack { - Text(Strings.Global.server) - if let providerEntity { - Spacer() - Text(providerEntity.server.hostname ?? providerEntity.server.serverId) - .foregroundStyle(.secondary) - } - } + @ViewBuilder + func destination(for route: Subroute) -> some View { + switch route { + case .credentials: + CredentialsView( + isInteractive: $draft.isInteractive, + credentials: $draft.credentials + ) } } +} + +// MARK: - Manual configuration + +private extension OpenVPNView { @ViewBuilder - var contentView: some View { + var manualView: some View { moduleSection(for: accountRows, header: Strings.Global.account) moduleSection(for: remotesRows, header: Strings.Modules.Openvpn.remotes) if !isServerPushed { @@ -153,64 +156,9 @@ private extension OpenVPNView { } moduleSection(for: otherRows, header: Strings.Global.other) } -} -private extension OpenVPNView { - func onSelectProvider(manager: ProviderManager) { - guard let providerId else { - return - } - vpnProviderManager.view = manager.vpnView( - withId: providerId, - initialParameters: .init(sorting: [ - .localizedCountry, - .area, - .hostname - ]) - ) - } - - func onSelect(server: VPNServer, preset: VPNPreset) { - providerEntity = VPNEntity(server: server, preset: preset) - } - - func importConfiguration(from url: URL) { - // TODO: #657, import draft from external URL - } -} - -// MARK: - Destinations - -private extension OpenVPNView { - enum Subroute: Hashable { - case providerServer - - case credentials - } - - @ViewBuilder - func destination(for route: Subroute) -> some View { - switch route { - case .providerServer: - VPNProviderServerView( - manager: vpnProviderManager, - onSelect: onSelect - ) - - case .credentials: - CredentialsView( - isInteractive: $draft.isInteractive, - credentials: $draft.credentials - ) - } - } -} - -// MARK: - Subviews - -private extension OpenVPNView { var accountRows: [ModuleRow]? { - guard configuration.authUserPass == true || providerId != nil else { + guard configuration.authUserPass == true else { return nil } return [.push(caption: Strings.Modules.Openvpn.credentials, route: HashableRoute(Subroute.credentials))] diff --git a/Passepartout/Library/Sources/AppUI/Views/Provider/ProviderContentModifier.swift b/Passepartout/Library/Sources/AppUI/Views/Provider/ProviderContentModifier.swift new file mode 100644 index 00000000..971cad50 --- /dev/null +++ b/Passepartout/Library/Sources/AppUI/Views/Provider/ProviderContentModifier.swift @@ -0,0 +1,215 @@ +// +// ProviderContentModifier.swift +// Passepartout +// +// Created by Davide De Rosa on 10/14/24. +// Copyright (c) 2024 Davide De Rosa. All rights reserved. +// +// https://github.com/passepartoutvpn +// +// This file is part of Passepartout. +// +// Passepartout is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Passepartout is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Passepartout. If not, see . +// + +import PassepartoutKit +import SwiftUI + +struct ProviderContentModifier: ViewModifier where Entity: ProviderEntity, Entity.Configuration: ProviderConfigurationIdentifiable & Codable, ProviderRows: View { + + @EnvironmentObject + private var providerManager: ProviderManager + + var apis: [APIMapper] = API.shared + + @Binding + var providerId: ProviderID? + + let entityType: Entity.Type + + let isRequired: Bool + + @ViewBuilder + let providerRows: ProviderRows + + let onSelectProvider: (ProviderManager, ProviderID?, _ isInitial: Bool) -> Void + + func body(content: Content) -> some View { + providerView + .onLoad(perform: loadCurrentProvider) + .onChange(of: providerId) { newId in + Task { + if let newId { + await refreshInfrastructure(for: newId) + } + onSelectProvider(providerManager, newId, false) + } + } + .disabled(providerManager.isLoading) + + if providerId == nil && !isRequired { + content + } + } + + static func == (lhs: Self, rhs: Self) -> Bool { + lhs.providerId == rhs.providerId + } +} + +private extension ProviderContentModifier { + +#if os(iOS) + var providerView: some View { + Group { + providerPicker + if providerId != nil { + providerRows + refreshButton { + HStack { + Text(Strings.Views.Provider.refreshInfrastructure) + if providerManager.isLoading { + Spacer() + ProgressView() + } + } + } + } + } + .themeSection(footer: lastUpdatedString) + } +#else + var providerView: some View { + Group { + providerPicker + if providerId != nil { + providerRows + HStack { + lastUpdatedString.map { + Text($0) + .foregroundStyle(.secondary) + } + Spacer() + refreshButton { + Text(Strings.Views.Provider.refreshInfrastructure) + } + } + } + } + } +#endif + + var providerPicker: some View { + ProviderPicker( + providers: supportedProviders, + providerId: $providerId, + isRequired: isRequired, + isLoading: providerManager.isLoading + ) + } + + func refreshButton