From aba50814501da2ddb370d895db69772944a35365 Mon Sep 17 00:00:00 2001 From: Davide Date: Sat, 2 Nov 2024 15:24:41 +0100 Subject: [PATCH] Refactor and improve interactive login (#801) Define two styles for interactive login: - Modal (iOS/macOS) - Form inside NavigationStack - Inline (tvOS) - VStack Requires OpenVPN credentials view to be container-agnostic. Play with focus to improve the overall TV experience. --- .../Views/App/ProfileContainerView.swift | 3 +- .../AppUIMain/Views/Modules/OpenVPNView.swift | 12 +- .../Profile/iOS/ProfileEditView+iOS.swift | 2 +- .../AppUITV/Views/App/AppCoordinator.swift | 19 +- .../Views/Profile/ProfileListView.swift | 2 +- .../AppUITV/Views/Profile/ProfileView.swift | 47 +++-- .../UILibrary/L10n/SwiftGen+Strings.swift | 4 + .../Resources/en.lproj/Localizable.strings | 1 + .../Theme/Platforms/Theme+tvOS.swift | 4 +- .../Modules/OpenVPNView+Credentials.swift | 15 +- .../Views/UI/InteractiveCoordinator.swift | 194 ++++++++++++++++++ .../UILibrary/Views/UI/InteractiveView.swift | 89 -------- swiftgen.yml | 4 +- 13 files changed, 262 insertions(+), 134 deletions(-) create mode 100644 Passepartout/Library/Sources/UILibrary/Views/UI/InteractiveCoordinator.swift delete mode 100644 Passepartout/Library/Sources/UILibrary/Views/UI/InteractiveView.swift diff --git a/Passepartout/Library/Sources/AppUIMain/Views/App/ProfileContainerView.swift b/Passepartout/Library/Sources/AppUIMain/Views/App/ProfileContainerView.swift index 530b45d4..e6955c9b 100644 --- a/Passepartout/Library/Sources/AppUIMain/Views/App/ProfileContainerView.swift +++ b/Passepartout/Library/Sources/AppUIMain/Views/App/ProfileContainerView.swift @@ -92,13 +92,14 @@ private extension ProfileContainerView { } func interactiveDestination() -> some View { - InteractiveView(manager: interactiveManager) { + InteractiveCoordinator(style: .modal, manager: interactiveManager) { errorHandler.handle( $0, title: Strings.Global.connection, message: Strings.Views.Profiles.Errors.tunnel ) } + .presentationDetents([.medium]) } } diff --git a/Passepartout/Library/Sources/AppUIMain/Views/Modules/OpenVPNView.swift b/Passepartout/Library/Sources/AppUIMain/Views/Modules/OpenVPNView.swift index e4ab5ebe..00165fe0 100644 --- a/Passepartout/Library/Sources/AppUIMain/Views/Modules/OpenVPNView.swift +++ b/Passepartout/Library/Sources/AppUIMain/Views/Modules/OpenVPNView.swift @@ -226,10 +226,14 @@ private extension OpenVPNView { } case .credentials: - OpenVPNCredentialsView( - isInteractive: draft.isInteractive, - credentials: draft.credentials - ) + Form { + OpenVPNCredentialsView( + isInteractive: draft.isInteractive, + credentials: draft.credentials + ) + } + .navigationTitle(Strings.Modules.Openvpn.credentials) + .themeForm() .themeAnimation(on: draft.wrappedValue.isInteractive, category: .modules) .modifier(PaywallModifier(reason: $paywallReason)) } diff --git a/Passepartout/Library/Sources/AppUIMain/Views/Profile/iOS/ProfileEditView+iOS.swift b/Passepartout/Library/Sources/AppUIMain/Views/Profile/iOS/ProfileEditView+iOS.swift index e255dacb..a9849937 100644 --- a/Passepartout/Library/Sources/AppUIMain/Views/Profile/iOS/ProfileEditView+iOS.swift +++ b/Passepartout/Library/Sources/AppUIMain/Views/Profile/iOS/ProfileEditView+iOS.swift @@ -80,7 +80,7 @@ private extension ProfileEditView { @ToolbarContentBuilder func toolbarContent() -> some ToolbarContent { - ToolbarItem { + ToolbarItem(placement: .confirmationAction) { ProfileSaveButton( title: Strings.Global.save, errorModuleIds: $malformedModuleIds diff --git a/Passepartout/Library/Sources/AppUITV/Views/App/AppCoordinator.swift b/Passepartout/Library/Sources/AppUITV/Views/App/AppCoordinator.swift index 538ca68c..7e34bd44 100644 --- a/Passepartout/Library/Sources/AppUITV/Views/App/AppCoordinator.swift +++ b/Passepartout/Library/Sources/AppUITV/Views/App/AppCoordinator.swift @@ -48,10 +48,10 @@ public struct AppCoordinator: View, AppCoordinatorConforming { Text(Strings.Global.profile) } - searchView - .tabItem { - ThemeImage(.search) - } +// searchView +// .tabItem { +// ThemeImage(.search) +// } settingsView .tabItem { @@ -66,12 +66,11 @@ private extension AppCoordinator { ProfileView(profileManager: profileManager, tunnel: tunnel) } - // FIXME: #788, UI for TV - var searchView: some View { - VStack { - Text("Search") - } - } +// var searchView: some View { +// VStack { +// Text("Search") +// } +// } // FIXME: #788, UI for TV var settingsView: some View { diff --git a/Passepartout/Library/Sources/AppUITV/Views/Profile/ProfileListView.swift b/Passepartout/Library/Sources/AppUITV/Views/Profile/ProfileListView.swift index 9222473e..a8c9429c 100644 --- a/Passepartout/Library/Sources/AppUITV/Views/Profile/ProfileListView.swift +++ b/Passepartout/Library/Sources/AppUITV/Views/Profile/ProfileListView.swift @@ -70,6 +70,7 @@ private extension ProfileListView { toggleView(for: header) } ) + .focused($focusedField, equals: .profile(header.id)) } func toggleView(for header: ProfileHeader) -> some View { @@ -81,6 +82,5 @@ private extension ProfileListView { } } .font(.headline) - .focused($focusedField, equals: .profile(header.id)) } } diff --git a/Passepartout/Library/Sources/AppUITV/Views/Profile/ProfileView.swift b/Passepartout/Library/Sources/AppUITV/Views/Profile/ProfileView.swift index 98a5f5e2..3f19f7ef 100644 --- a/Passepartout/Library/Sources/AppUITV/Views/Profile/ProfileView.swift +++ b/Passepartout/Library/Sources/AppUITV/Views/Profile/ProfileView.swift @@ -71,12 +71,21 @@ struct ProfileView: View, TunnelInstallationProviding { .focusSection() } .frame(maxWidth: .infinity) + .disabled(interactiveManager.isPresented) if isSwitching { - listView - .padding(.horizontal) - .frame(width: geo.size.width * 0.5) - .focusSection() + ZStack { + listView + .padding(.horizontal) + .opacity(interactiveManager.isPresented ? 0.0 : 1.0) + + if interactiveManager.isPresented { + interactiveView + .padding(.horizontal, 100) + } + } +// .frame(width: geo.size.width * 0.5) // seems redundant + .focusSection() } } } @@ -84,21 +93,11 @@ struct ProfileView: View, TunnelInstallationProviding { .background(theme.primaryColor.gradient) .animation(.default, value: isSwitching) .withErrorHandler(errorHandler) - .themeModal(isPresented: $interactiveManager.isPresented) { - InteractiveView(manager: interactiveManager) { - errorHandler.handle( - $0, - title: Strings.Global.connection, - message: Strings.Views.Profiles.Errors.tunnel - ) - } - } - .onLoad { - focusedField = .switchProfile - } + .defaultFocus($focusedField, .switchProfile) .onChange(of: tunnel.status) { _, new in if new == .activating { isSwitching = false + focusedField = .connect } } .onChange(of: tunnel.currentProfile) { _, new in @@ -141,6 +140,22 @@ private extension ProfileView { ) } + var interactiveView: some View { + InteractiveCoordinator(style: .inline(withCancel: false), manager: interactiveManager) { + errorHandler.handle( + $0, + title: Strings.Global.connection, + message: Strings.Views.Profiles.Errors.tunnel + ) + } + .font(.body) + .onExitCommand { + let formerProfileId = interactiveManager.editor.profile.id + focusedField = .profile(formerProfileId) + interactiveManager.isPresented = false + } + } + var listView: some View { ProfileListView( profileManager: profileManager, diff --git a/Passepartout/Library/Sources/UILibrary/L10n/SwiftGen+Strings.swift b/Passepartout/Library/Sources/UILibrary/L10n/SwiftGen+Strings.swift index 0e2ec97b..03e8870a 100644 --- a/Passepartout/Library/Sources/UILibrary/L10n/SwiftGen+Strings.swift +++ b/Passepartout/Library/Sources/UILibrary/L10n/SwiftGen+Strings.swift @@ -531,6 +531,10 @@ public enum Strings { /// (on-demand) public static let onDemandSuffix = Strings.tr("Localizable", "ui.connection_status.on_demand_suffix", fallback: " (on-demand)") } + public enum InteractiveCoordinator { + /// Interactive + public static let title = Strings.tr("Localizable", "ui.interactive_coordinator.title", fallback: "Interactive") + } public enum ProfileContext { /// Connect to... public static let connectTo = Strings.tr("Localizable", "ui.profile_context.connect_to", fallback: "Connect to...") diff --git a/Passepartout/Library/Sources/UILibrary/Resources/en.lproj/Localizable.strings b/Passepartout/Library/Sources/UILibrary/Resources/en.lproj/Localizable.strings index e93d9abd..c8a4234a 100644 --- a/Passepartout/Library/Sources/UILibrary/Resources/en.lproj/Localizable.strings +++ b/Passepartout/Library/Sources/UILibrary/Resources/en.lproj/Localizable.strings @@ -246,6 +246,7 @@ // MARK: - Components "ui.connection_status.on_demand_suffix" = " (on-demand)"; +"ui.interactive_coordinator.title" = "Interactive"; "ui.profile_context.connect_to" = "Connect to..."; // MARK: - Paywalls diff --git a/Passepartout/Library/Sources/UILibrary/Theme/Platforms/Theme+tvOS.swift b/Passepartout/Library/Sources/UILibrary/Theme/Platforms/Theme+tvOS.swift index 1ee9a437..eab31011 100644 --- a/Passepartout/Library/Sources/UILibrary/Theme/Platforms/Theme+tvOS.swift +++ b/Passepartout/Library/Sources/UILibrary/Theme/Platforms/Theme+tvOS.swift @@ -67,13 +67,13 @@ extension ThemeSectionWithHeaderFooterModifier { extension ThemeTextField { public var body: some View { - TextField(placeholder, text: $text) + TextField(title ?? "", text: $text) } } extension ThemeSecureField { public var body: some View { - SecureField(placeholder, text: $text) + SecureField(title ?? "", text: $text) } } diff --git a/Passepartout/Library/Sources/UILibrary/Views/Modules/OpenVPNView+Credentials.swift b/Passepartout/Library/Sources/UILibrary/Views/Modules/OpenVPNView+Credentials.swift index 151fd6a5..359f79bd 100644 --- a/Passepartout/Library/Sources/UILibrary/Views/Modules/OpenVPNView+Credentials.swift +++ b/Passepartout/Library/Sources/UILibrary/Views/Modules/OpenVPNView+Credentials.swift @@ -58,13 +58,11 @@ public struct OpenVPNCredentialsView: View { } public var body: some View { - Form { + Group { restrictedArea inputSection } .themeManualInput() - .themeForm() - .navigationTitle(Strings.Modules.Openvpn.credentials) .onLoad { builder = credentials?.builder() ?? OpenVPN.Credentials.Builder() builder.otp = nil @@ -136,11 +134,12 @@ private extension OpenVPNCredentialsView { var inputSection: some View { Group { - ThemeTextField(Strings.Global.username, text: $builder.username, placeholder: Strings.Placeholders.username) - .textContentType(.username) - ThemeSecureField(title: Strings.Global.password, text: $builder.password, placeholder: Strings.Placeholders.secret) - .textContentType(.password) - + if !isAuthenticating || builder.otpMethod == .none { + ThemeTextField(Strings.Global.username, text: $builder.username, placeholder: Strings.Placeholders.username) + .textContentType(.username) + ThemeSecureField(title: Strings.Global.password, text: $builder.password, placeholder: Strings.Placeholders.secret) + .textContentType(.password) + } if isEligibleForInteractiveLogin, isAuthenticating, builder.otpMethod != .none { ThemeSecureField( title: Strings.Unlocalized.otp, diff --git a/Passepartout/Library/Sources/UILibrary/Views/UI/InteractiveCoordinator.swift b/Passepartout/Library/Sources/UILibrary/Views/UI/InteractiveCoordinator.swift new file mode 100644 index 00000000..f4b0d631 --- /dev/null +++ b/Passepartout/Library/Sources/UILibrary/Views/UI/InteractiveCoordinator.swift @@ -0,0 +1,194 @@ +// +// InteractiveCoordinator.swift +// Passepartout +// +// Created by Davide De Rosa on 9/8/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 CommonUtils +import PassepartoutKit +import SwiftUI + +public struct InteractiveCoordinator: View { + public enum Style { + case modal + + case inline(withCancel: Bool) + } + + private let style: Style + + @ObservedObject + private var manager: InteractiveManager + + private let onError: (Error) -> Void + + public init(style: Style, manager: InteractiveManager, onError: @escaping (Error) -> Void) { + self.style = style + self.manager = manager + self.onError = onError + } + + public var body: some View { + switch style { + case .modal: + interactiveView + .modifier(ModalInteractiveModifier( + confirm: confirm, + cancel: cancel + )) + + case .inline(let withCancel): + interactiveView + .modifier(InlineInteractiveModifier( + title: manager.editor.profile.name, + withCancel: withCancel, + confirm: confirm, + cancel: cancel + )) + } + } +} + +// MARK: - Modal + +private extension InteractiveCoordinator { + struct ModalInteractiveModifier: ViewModifier { + let confirm: () -> Void + + let cancel: () -> Void + + func body(content: Content) -> some View { + NavigationStack { + Form { + content + } + .themeForm() + .themeNavigationDetail() + .navigationTitle(Strings.Ui.InteractiveCoordinator.title) + .toolbar(content: modalToolbar) + } + } + + @ToolbarContentBuilder + func modalToolbar() -> some ToolbarContent { + ToolbarItem(placement: .confirmationAction) { + Button(action: confirm) { + Text(Strings.Global.connect) + } + } + ToolbarItem(placement: .cancellationAction) { + Button(action: cancel) { +#if os(iOS) + ThemeImage(.close) +#else + Text(Strings.Global.cancel) +#endif + } + } + } + } +} + +// MARK: - Inline + +private extension InteractiveCoordinator { + struct InlineInteractiveModifier: ViewModifier { + let title: String + + let withCancel: Bool + + let confirm: () -> Void + + let cancel: () -> Void + + func body(content: Content) -> some View { + VStack { + Text(title) + .font(.title2) + content + toolbar + .padding(.top) + Spacer() + } +#if os(tvOS) + .scrollClipDisabled() +#endif + } + + var toolbar: some View { + VStack { + Button(action: confirm) { + Text(Strings.Global.connect) + .frame(maxWidth: .infinity) + } + if withCancel { + Button(role: .cancel, action: cancel) { + Text(Strings.Global.cancel) + .frame(maxWidth: .infinity) + } + } + } + .frame(maxWidth: .infinity) + } + } +} + +// MARK: - Common + +private extension InteractiveCoordinator { + var interactiveView: some View { + manager + .editor + .interactiveProvider + .map(innerView) + } + + func innerView(with provider: any InteractiveViewProviding) -> some View { + AnyView(provider.interactiveView(with: manager.editor)) + } + + func confirm() { + Task { + do { + try await manager.complete() + } catch { + onError(error) + } + } + } + + func cancel() { + manager.isPresented = false + } +} + +private extension ProfileEditor { + + // in the future, multiple modules may be interactive + // here we only intercept the first interactive module + var interactiveProvider: (any InteractiveViewProviding)? { + modules + .first { + $0 is any InteractiveViewProviding + } as? any InteractiveViewProviding + } +} diff --git a/Passepartout/Library/Sources/UILibrary/Views/UI/InteractiveView.swift b/Passepartout/Library/Sources/UILibrary/Views/UI/InteractiveView.swift deleted file mode 100644 index 947f109f..00000000 --- a/Passepartout/Library/Sources/UILibrary/Views/UI/InteractiveView.swift +++ /dev/null @@ -1,89 +0,0 @@ -// -// InteractiveView.swift -// Passepartout -// -// Created by Davide De Rosa on 9/8/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 - -public struct InteractiveView: View { - - @ObservedObject - private var manager: InteractiveManager - - private let onError: (Error) -> Void - - public init(manager: InteractiveManager, onError: @escaping (Error) -> Void) { - self.manager = manager - self.onError = onError - } - - public var body: some View { - manager - .editor - .interactiveProvider - .map(stackView) - } -} - -@MainActor -private extension InteractiveView { - func stackView(with provider: any InteractiveViewProviding) -> some View { - NavigationStack { - AnyView(provider.interactiveView(with: manager.editor)) - .toolbar(content: toolbarContent) - } - } - - @ToolbarContentBuilder - func toolbarContent() -> some ToolbarContent { - ToolbarItem(placement: .cancellationAction) { - Button(Strings.Global.cancel, role: .cancel) { - manager.isPresented = false - } - } - ToolbarItem(placement: .confirmationAction) { - Button(Strings.Global.ok) { - Task { - do { - try await manager.complete() - } catch { - onError(error) - } - } - } - } - } -} - -private extension ProfileEditor { - - // in the future, multiple modules may be interactive - // here we only intercept the first interactive module - var interactiveProvider: (any InteractiveViewProviding)? { - modules - .first { - $0 is any InteractiveViewProviding - } as? any InteractiveViewProviding - } -} diff --git a/swiftgen.yml b/swiftgen.yml index 8400d22b..593f79c8 100644 --- a/swiftgen.yml +++ b/swiftgen.yml @@ -1,9 +1,9 @@ strings: inputs: - - Passepartout/Library/Sources/AppUI/Resources/en.lproj/Localizable.strings + - Passepartout/Library/Sources/UILibrary/Resources/en.lproj/Localizable.strings outputs: - templateName: structured-swift5 - output: Passepartout/Library/Sources/AppUI/L10n/SwiftGen+Strings.swift + output: Passepartout/Library/Sources/UILibrary/L10n/SwiftGen+Strings.swift params: bundle: Bundle.main enumName: Strings