Rewrite provider views (#740)

Resolve some flickering and state inconsistency due to overextended
observation of VPNProviderManager. Narrow down its scope to
VPNProviderServerView.

The downside of that, for now, is that servers are loaded "lazily late",
but this flow will make region selection from home easier.

Finally, show filters in popover on iPad.
This commit is contained in:
Davide 2024-10-18 18:12:28 +02:00 committed by GitHub
parent 0221aea6b6
commit 5d2e24792c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 317 additions and 283 deletions

View File

@ -41,7 +41,7 @@
"kind" : "remoteSourceControl", "kind" : "remoteSourceControl",
"location" : "git@github.com:passepartoutvpn/passepartoutkit-source", "location" : "git@github.com:passepartoutvpn/passepartoutkit-source",
"state" : { "state" : {
"revision" : "046a7594fa9823407dd405ba700b1b81506ce9af" "revision" : "9a43e23e9134c3e93926271b2d630be607433fa0"
} }
}, },
{ {

View File

@ -28,7 +28,7 @@ let package = Package(
], ],
dependencies: [ dependencies: [
// .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", from: "0.9.0"), // .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", from: "0.9.0"),
.package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", revision: "046a7594fa9823407dd405ba700b1b81506ce9af"), .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", revision: "9a43e23e9134c3e93926271b2d630be607433fa0"),
// .package(path: "../../../passepartoutkit-source"), // .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", from: "0.9.1"),
// .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-openvpn-openssl", revision: "031863a1cd683962a7dfe68e20b91fa820a1ecce"), // .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-openvpn-openssl", revision: "031863a1cd683962a7dfe68e20b91fa820a1ecce"),

View File

@ -146,7 +146,7 @@ extension OnDemandModule.Policy: LocalizableEntity {
extension VPNServer { extension VPNServer {
public var region: String { public var region: String {
[provider.countryCodes.first?.localizedAsRegionCode, provider.area] [provider.countryCode.localizedAsRegionCode, provider.area]
.compactMap { $0 } .compactMap { $0 }
.joined(separator: " - ") .joined(separator: " - ")
} }

View File

@ -63,8 +63,7 @@ extension AppContext {
tunnelEnvironment: env, tunnelEnvironment: env,
registry: registry, registry: registry,
providerManager: ProviderManager( providerManager: ProviderManager(
repository: InMemoryProviderRepository(), repository: InMemoryProviderRepository()
vpnRepository: InMemoryVPNProviderRepository()
), ),
constants: .shared constants: .shared
) )

View File

@ -31,7 +31,7 @@ struct ProviderContentModifier<Entity, ProviderRows>: ViewModifier where Entity:
@EnvironmentObject @EnvironmentObject
private var providerManager: ProviderManager private var providerManager: ProviderManager
var apis: [APIMapper] = API.shared let apis: [APIMapper]
@Binding @Binding
var providerId: ProviderID? var providerId: ProviderID?

View File

@ -1,45 +0,0 @@
//
// VPNFiltersModifier.swift
// Passepartout
//
// Created by Davide De Rosa on 10/9/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 <http://www.gnu.org/licenses/>.
//
import PassepartoutKit
import SwiftUI
struct VPNFiltersModifier<Configuration>: ViewModifier where Configuration: Decodable {
@ObservedObject
var manager: VPNProviderManager
@State
var isFiltersPresented = false
func body(content: Content) -> some View {
contentView(with: content)
.onChange(of: manager.parameters.filters) { _ in
Task {
manager.applyFilters()
}
}
}
}

View File

@ -27,95 +27,36 @@ import AppLibrary
import PassepartoutKit import PassepartoutKit
import SwiftUI import SwiftUI
struct VPNFiltersView<Configuration>: View where Configuration: Decodable { struct VPNFiltersView<Configuration>: View where Configuration: ProviderConfigurationIdentifiable & Decodable {
@ObservedObject @ObservedObject
var manager: VPNProviderManager var manager: VPNProviderManager<Configuration>
@Binding
var filters: VPNFilters
var body: some View { var body: some View {
Form { debugChanges()
Section { return Subview(
categoryPicker filters: $filters,
countryPicker categories: categories,
presetPicker countries: countries,
#if os(iOS) presets: presets
clearFiltersButton )
.frame(maxWidth: .infinity, alignment: .center) .onChange(of: filters, perform: manager.applyFilters)
#else
HStack {
Spacer()
clearFiltersButton
}
#endif
}
}
}
}
private extension VPNFiltersView {
var categoryPicker: some View {
Picker(Strings.Global.category, selection: $manager.parameters.filters.categoryName) {
Text(Strings.Global.any)
.tag(nil as String?)
ForEach(categories, id: \.self) {
Text($0.capitalized)
.tag($0 as String?)
}
}
}
var countryPicker: some View {
Picker(Strings.Global.country, selection: $manager.parameters.filters.countryCode) {
Text(Strings.Global.any)
.tag(nil as String?)
ForEach(countries, id: \.code) {
Text($0.description)
.tag($0.code as String?)
}
}
}
@ViewBuilder
var presetPicker: some View {
if manager.allPresets.count > 1 {
Picker(Strings.Views.Provider.Vpn.preset, selection: $manager.parameters.filters.presetId) {
Text(Strings.Global.any)
.tag(nil as String?)
ForEach(presets, id: \.presetId) {
Text($0.description)
.tag($0.presetId as String?)
}
}
}
}
var clearFiltersButton: some View {
Button(Strings.Views.Provider.clearFilters, role: .destructive) {
Task {
manager.resetFilters()
}
}
} }
} }
private extension VPNFiltersView { private extension VPNFiltersView {
var categories: [String] { var categories: [String] {
let allCategories = manager manager
.allServers .allCategoryNames
.values
.map(\.provider.categoryName)
return Set(allCategories)
.sorted() .sorted()
} }
var countries: [(code: String, description: String)] { var countries: [(code: String, description: String)] {
let allCodes = manager manager
.allServers .allCountryCodes
.values
.flatMap(\.provider.countryCodes)
return Set(allCodes)
.map { .map {
(code: $0, description: $0.localizedAsRegionCode ?? $0) (code: $0, description: $0.localizedAsRegionCode ?? $0)
} }
@ -126,15 +67,95 @@ private extension VPNFiltersView {
var presets: [VPNPreset<Configuration>] { var presets: [VPNPreset<Configuration>] {
manager manager
.presets(ofType: Configuration.self) .presets
.sorted { .sorted {
$0.description < $1.description $0.description < $1.description
} }
} }
} }
#Preview { // MARK: -
NavigationStack {
VPNFiltersView<String>(manager: VPNProviderManager()) private extension VPNFiltersView {
struct Subview: View {
@Binding
var filters: VPNFilters
let categories: [String]
let countries: [(code: String, description: String)]
let presets: [VPNPreset<Configuration>]
var body: some View {
debugChanges()
return Form {
Section {
categoryPicker
countryPicker
presetPicker
#if os(iOS)
clearFiltersButton
.frame(maxWidth: .infinity, alignment: .center)
#else
HStack {
Spacer()
clearFiltersButton
}
#endif
}
}
}
}
}
private extension VPNFiltersView.Subview {
var categoryPicker: some View {
Picker(Strings.Global.category, selection: $filters.categoryName) {
Text(Strings.Global.any)
.tag(nil as String?)
ForEach(categories, id: \.self) {
Text($0.capitalized)
.tag($0 as String?)
}
}
}
var countryPicker: some View {
Picker(Strings.Global.country, selection: $filters.countryCode) {
Text(Strings.Global.any)
.tag(nil as String?)
ForEach(countries, id: \.code) {
Text($0.description)
.tag($0.code as String?)
}
}
}
var presetPicker: some View {
Picker(Strings.Views.Provider.Vpn.preset, selection: $filters.presetId) {
Text(Strings.Global.any)
.tag(nil as String?)
ForEach(presets, id: \.presetId) {
Text($0.description)
.tag($0.presetId as String?)
}
}
}
var clearFiltersButton: some View {
Button(Strings.Views.Provider.clearFilters, role: .destructive) {
filters = VPNFilters()
}
}
}
#Preview {
NavigationStack {
VPNFiltersView<OpenVPN.Configuration>(
manager: VPNProviderManager(),
filters: .constant(VPNFilters())
)
} }
} }

View File

@ -28,8 +28,11 @@ import PassepartoutKit
import SwiftUI import SwiftUI
import UtilsLibrary import UtilsLibrary
@MainActor
struct VPNProviderContentModifier<Configuration, ProviderRows>: ViewModifier where Configuration: ProviderConfigurationIdentifiable & Codable, ProviderRows: View { struct VPNProviderContentModifier<Configuration, ProviderRows>: ViewModifier where Configuration: ProviderConfigurationIdentifiable & Codable, ProviderRows: View {
var apis: [APIMapper] = API.shared
@Binding @Binding
var providerId: ProviderID? var providerId: ProviderID?
@ -43,12 +46,11 @@ struct VPNProviderContentModifier<Configuration, ProviderRows>: ViewModifier whe
@ViewBuilder @ViewBuilder
let providerRows: ProviderRows let providerRows: ProviderRows
@StateObject
private var vpnProviderManager = VPNProviderManager()
func body(content: Content) -> some View { func body(content: Content) -> some View {
content debugChanges()
return content
.modifier(ProviderContentModifier( .modifier(ProviderContentModifier(
apis: apis,
providerId: $providerId, providerId: $providerId,
entityType: VPNEntity<Configuration>.self, entityType: VPNEntity<Configuration>.self,
isRequired: isRequired, isRequired: isRequired,
@ -64,10 +66,15 @@ struct VPNProviderContentModifier<Configuration, ProviderRows>: ViewModifier whe
private extension VPNProviderContentModifier { private extension VPNProviderContentModifier {
var providerServerRow: some View { var providerServerRow: some View {
NavigationLink { NavigationLink {
VPNProviderServerView<Configuration>( providerId.map {
manager: vpnProviderManager, VPNProviderServerView<Configuration>(
onSelect: onSelectServer apis: apis,
) providerId: $0,
configurationType: Configuration.self,
selectedEntity: selectedEntity,
onSelect: onSelectServer
)
}
} label: { } label: {
HStack { HStack {
Text(Strings.Global.server) Text(Strings.Global.server)
@ -79,27 +86,13 @@ private extension VPNProviderContentModifier {
} }
} }
} }
}
private extension VPNProviderContentModifier {
func onSelectProvider(manager: ProviderManager, providerId: ProviderID?, isInitial: Bool) { func onSelectProvider(manager: ProviderManager, providerId: ProviderID?, isInitial: Bool) {
guard let providerId else {
return
}
let initialEntity = isInitial ? selectedEntity : nil
if !isInitial { if !isInitial {
selectedEntity = nil selectedEntity = nil
} }
let view = manager.vpnView(
for: providerId,
configurationType: OpenVPN.Configuration.self,
initialParameters: .init(
sorting: [
.localizedCountry,
.area,
.hostname
]
)
)
vpnProviderManager.setView(view, filteringWith: initialEntity?.server.provider)
} }
func onSelectServer(server: VPNServer, preset: VPNPreset<Configuration>) { func onSelectServer(server: VPNServer, preset: VPNPreset<Configuration>) {

View File

@ -26,27 +26,85 @@
import AppLibrary import AppLibrary
import PassepartoutKit import PassepartoutKit
import SwiftUI import SwiftUI
import UtilsLibrary
struct VPNProviderServerView<Configuration>: View where Configuration: ProviderConfigurationIdentifiable & Codable { struct VPNProviderServerView<Configuration>: View where Configuration: ProviderConfigurationIdentifiable & Codable {
@EnvironmentObject
private var providerManager: ProviderManager
@Environment(\.dismiss) @Environment(\.dismiss)
private var dismiss private var dismiss
@ObservedObject let apis: [APIMapper]
var manager: VPNProviderManager
let providerId: ProviderID
let configurationType: Configuration.Type
var selectedEntity: VPNEntity<Configuration>?
let onSelect: (_ server: VPNServer, _ preset: VPNPreset<Configuration>) -> Void let onSelect: (_ server: VPNServer, _ preset: VPNPreset<Configuration>) -> Void
@StateObject
private var manager = VPNProviderManager<Configuration>(sorting: [
.localizedCountry,
.area,
.hostname
])
@State
private var filters = VPNFilters()
@StateObject
private var errorHandler: ErrorHandler = .default()
var body: some View { var body: some View {
serversView debugChanges()
.modifier(VPNFiltersModifier<Configuration>(manager: manager)) return Subview(
.navigationTitle(Strings.Global.servers) manager: manager,
filters: $filters,
onSelect: selectServer
)
.withErrorHandler(errorHandler)
.navigationTitle(Strings.Global.servers)
.onLoad {
Task {
do {
manager.repository = try await providerManager.vpnRepository(
from: apis,
for: providerId,
configurationType: Configuration.self
)
if let selectedEntity {
filters = VPNFilters(with: selectedEntity.server.provider)
} else {
filters = VPNFilters()
}
manager.applyFilters(filters)
} catch {
pp_log(.app, .error, "Unable to load VPN repository: \(error)")
errorHandler.handle(error, title: Strings.Global.servers)
}
}
}
} }
} }
// MARK: - Actions // MARK: - Actions
extension VPNProviderServerView { extension VPNProviderServerView {
func compatiblePreset(with server: VPNServer) -> VPNPreset<Configuration>? {
manager
.presets
.first {
if let supportedIds = server.provider.supportedPresetIds {
return supportedIds.contains($0.presetId)
}
return true
}
}
func selectServer(_ server: VPNServer) { func selectServer(_ server: VPNServer) {
guard let preset = compatiblePreset(with: server) else { guard let preset = compatiblePreset(with: server) else {
pp_log(.app, .error, "Unable to find a compatible preset. Supported IDs: \(server.provider.supportedPresetIds ?? [])") pp_log(.app, .error, "Unable to find a compatible preset. Supported IDs: \(server.provider.supportedPresetIds ?? [])")
@ -58,24 +116,16 @@ extension VPNProviderServerView {
} }
} }
private extension VPNProviderServerView {
func compatiblePreset(with server: VPNServer) -> VPNPreset<Configuration>? {
manager
.presets(ofType: Configuration.self)
.first {
if let supportedIds = server.provider.supportedPresetIds {
return supportedIds.contains($0.presetId)
}
return true
}
}
}
// MARK: - Preview // MARK: - Preview
#Preview { #Preview {
NavigationStack { NavigationStack {
VPNProviderServerView<OpenVPN.Configuration>(manager: VPNProviderManager()) { _, _ in VPNProviderServerView<OpenVPN.Configuration>(
apis: [API.bundled],
providerId: .protonvpn,
configurationType: OpenVPN.Configuration.self
) { _, _ in
} }
} }
.withMockEnvironment() .withMockEnvironment()

View File

@ -1,55 +0,0 @@
//
// VPNFiltersModifier+iOS.swift
// Passepartout
//
// Created by Davide De Rosa on 10/9/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 <http://www.gnu.org/licenses/>.
//
#if os(iOS)
import SwiftUI
// FIXME: ###, providers UI, iPadOS show filters in popover
extension VPNFiltersModifier {
func contentView(with content: Content) -> some View {
content
.toolbar {
ToolbarItem(placement: .bottomBar) {
Button {
isFiltersPresented = true
} label: {
ThemeImage(.filters)
}
.themeModal(isPresented: $isFiltersPresented) {
NavigationStack {
VPNFiltersView<Configuration>(manager: manager)
.navigationTitle(Strings.Global.filters)
.navigationBarTitleDisplayMode(.inline)
}
.presentationDetents([.medium])
}
}
}
}
}
#endif

View File

@ -25,22 +25,72 @@
#if os(iOS) #if os(iOS)
import PassepartoutKit
import SwiftUI import SwiftUI
// FIXME: ###, providers UI, iOS server rows + country flags
extension VPNProviderServerView { extension VPNProviderServerView {
struct Subview: View {
@ViewBuilder @ObservedObject
var serversView: some View { var manager: VPNProviderManager<Configuration>
List {
ForEach(manager.filteredServers, id: \.id) { server in @Binding
Button("\(server.hostname ?? server.id) \(server.provider.countryCodes)") { var filters: VPNFilters
selectServer(server)
let onSelect: (VPNServer) -> Void
@State
private var isFiltersPresented = false
var body: some View {
listView
.disabled(manager.isFiltering)
.toolbar {
filtersItem
} }
}
} }
} }
} }
private extension VPNProviderServerView.Subview {
var listView: some View {
List {
// FIXME: ###, providers UI, iOS server rows + country flags
if manager.isFiltering {
ProgressView()
} else {
ForEach(manager.filteredServers, id: \.id) { server in
Button("\(server.hostname ?? server.id) \(server.provider.countryCode)") {
onSelect(server)
}
}
}
}
.themeAnimation(on: manager.isFiltering, category: .providers)
}
var filtersItem: some ToolbarContent {
ToolbarItem {
Button {
isFiltersPresented = true
} label: {
ThemeImage(.filters)
}
.themePopover(isPresented: $isFiltersPresented, content: filtersView)
}
}
func filtersView() -> some View {
NavigationStack {
VPNFiltersView(
manager: manager,
filters: $filters
)
.navigationTitle(Strings.Global.filters)
.navigationBarTitleDisplayMode(.inline)
}
.presentationDetents([.medium])
}
}
#endif #endif

View File

@ -1,40 +0,0 @@
//
// VPNFiltersModifier+macOS.swift
// Passepartout
//
// Created by Davide De Rosa on 10/9/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 <http://www.gnu.org/licenses/>.
//
#if os(macOS)
import SwiftUI
extension VPNFiltersModifier {
func contentView(with content: Content) -> some View {
VStack {
VPNFiltersView<Configuration>(manager: manager)
.padding()
content
}
}
}
#endif

View File

@ -25,14 +25,33 @@
#if os(macOS) #if os(macOS)
import PassepartoutKit
import SwiftUI import SwiftUI
// FIXME: ###, providers UI, macOS country flags // FIXME: ###, providers UI, macOS country flags
extension VPNProviderServerView { extension VPNProviderServerView {
struct Subview: View {
@ViewBuilder @ObservedObject
var serversView: some View { var manager: VPNProviderManager<Configuration>
@Binding
var filters: VPNFilters
let onSelect: (VPNServer) -> Void
var body: some View {
VStack {
filtersView
tableView
}
}
}
}
private extension VPNProviderServerView.Subview {
var tableView: some View {
Table(manager.filteredServers) { Table(manager.filteredServers) {
TableColumn(Strings.Global.region) { server in TableColumn(Strings.Global.region) { server in
Text(server.region) Text(server.region)
@ -43,13 +62,22 @@ extension VPNProviderServerView {
TableColumn("") { server in TableColumn("") { server in
Button { Button {
selectServer(server) onSelect(server)
} label: { } label: {
Text(Strings.Views.Provider.selectServer) Text(Strings.Views.Provider.selectServer)
} }
} }
.width(min: 100.0, max: 100.0) .width(min: 100.0, max: 100.0)
} }
.disabled(manager.isFiltering)
}
var filtersView: some View {
VPNFiltersView(
manager: manager,
filters: $filters
)
.padding()
} }
} }

View File

@ -102,6 +102,27 @@ struct ThemeItemModalModifier<Modal, T>: ViewModifier where Modal: View, T: Iden
} }
} }
struct ThemeBooleanPopoverModifier<Popover>: ViewModifier where Popover: View {
@EnvironmentObject
private var theme: Theme
@Binding
var isPresented: Bool
@ViewBuilder
let popover: Popover
func body(content: Content) -> some View {
content
.popover(isPresented: $isPresented) {
popover
.frame(minWidth: theme.popoverSize?.width, minHeight: theme.popoverSize?.height)
.themeLockScreen()
}
}
}
struct ThemeConfirmationModifier: ViewModifier { struct ThemeConfirmationModifier: ViewModifier {
@Binding @Binding

View File

@ -31,6 +31,7 @@ extension Theme {
public convenience init() { public convenience init() {
self.init(dummy: ()) self.init(dummy: ())
animationCategories = [.diagnostics, .modules, .profiles, .providers] animationCategories = [.diagnostics, .modules, .profiles, .providers]
popoverSize = .init(width: 400.0, height: 400.0)
} }
} }

View File

@ -40,6 +40,8 @@ public final class Theme: ObservableObject {
var secondaryModalSize: CGSize? var secondaryModalSize: CGSize?
var popoverSize: CGSize?
var relevantWeight: Font.Weight = .semibold var relevantWeight: Font.Weight = .semibold
var titleColor: Color = .primary var titleColor: Color = .primary
@ -163,6 +165,16 @@ extension View {
)) ))
} }
public func themePopover<Content>(
isPresented: Binding<Bool>,
content: @escaping () -> Content
) -> some View where Content: View {
modifier(ThemeBooleanPopoverModifier(
isPresented: isPresented,
popover: content
))
}
public func themeConfirmation(isPresented: Binding<Bool>, title: String, action: @escaping () -> Void) -> some View { public func themeConfirmation(isPresented: Binding<Bool>, title: String, action: @escaping () -> Void) -> some View {
modifier(ThemeConfirmationModifier(isPresented: isPresented, title: title, action: action)) modifier(ThemeConfirmationModifier(isPresented: isPresented, title: title, action: action))
} }

View File

@ -209,8 +209,7 @@ private extension ProfileManager {
// FIXME: #705, store providers to Core Data // FIXME: #705, store providers to Core Data
extension ProviderManager { extension ProviderManager {
static let shared = ProviderManager( static let shared = ProviderManager(
repository: InMemoryProviderRepository(), repository: InMemoryProviderRepository()
vpnRepository: InMemoryVPNProviderRepository()
) )
} }