From 07454bc0680b0e5464ed6126e73a55d0d6e29d9e Mon Sep 17 00:00:00 2001 From: Kaede Akino Date: Fri, 25 Sep 2026 19:16:31 +0800 Subject: [PATCH] feat(metrics): unify multi-host selection forms --- .../TermindPresentation/Strings/Common.swift | 4 + .../TermindPresentation/Strings/Metrics.swift | 1 - .../Strings/Snippets.swift | 4 - .../FeatureModels/MobileHostPickerModel.swift | 9 +- .../MobileMultiHostPickerModel.swift | 52 +++++++ .../Metrics/MobileMetricsModel.swift | 9 +- .../HostChooser/MobileConnectionPicker.swift | 25 +--- .../HostChooser/MobileMultiHostPicker.swift | 99 +++++++++++++ TFI/Features/Metrics/MobileMetricsView.swift | 43 +++--- .../Snippets/MobileSnippetsView.swift | 136 ++---------------- TFITests/MobileConnectionPickerTests.swift | 19 ++- TFITests/MobileIconRenderingTests.swift | 58 ++++++-- TFITests/MobileMarketingScreenshotTests.swift | 2 +- TFITests/MobileMultiHostPickerTests.swift | 106 ++++++++++++++ .../HostChooser/HostChooserView.swift | 116 +++++---------- TFM/Features/Snippets/SnippetsPageView.swift | 8 +- TFM/UI/Components/ChromeComponents.swift | 25 +--- TFM/UI/Components/WindowToolbar.swift | 2 +- TFMTests/ChooserInteractionTests.swift | 29 ++++ TFMTests/ManagedChooserTests.swift | 13 +- 20 files changed, 442 insertions(+), 318 deletions(-) create mode 100644 TFI/Application/FeatureModels/MobileMultiHostPickerModel.swift create mode 100644 TFI/Features/HostChooser/MobileMultiHostPicker.swift create mode 100644 TFITests/MobileMultiHostPickerTests.swift diff --git a/Frameworks/TermindPresentation/Strings/Common.swift b/Frameworks/TermindPresentation/Strings/Common.swift index 9ae6354b..1b639c07 100644 --- a/Frameworks/TermindPresentation/Strings/Common.swift +++ b/Frameworks/TermindPresentation/Strings/Common.swift @@ -10,6 +10,10 @@ extension AppStrings { "Add" } + public static var addHosts: LocalizedStringResource { + "Add Hosts" + } + public static var back: LocalizedStringResource { "Back" } diff --git a/Frameworks/TermindPresentation/Strings/Metrics.swift b/Frameworks/TermindPresentation/Strings/Metrics.swift index 0de86a04..0833aa54 100644 --- a/Frameworks/TermindPresentation/Strings/Metrics.swift +++ b/Frameworks/TermindPresentation/Strings/Metrics.swift @@ -90,6 +90,5 @@ extension AppStrings { "Select Devices" } - public static var addServer: LocalizedStringResource { "Add Server" } } } diff --git a/Frameworks/TermindPresentation/Strings/Snippets.swift b/Frameworks/TermindPresentation/Strings/Snippets.swift index 7b67e4ae..118b0621 100644 --- a/Frameworks/TermindPresentation/Strings/Snippets.swift +++ b/Frameworks/TermindPresentation/Strings/Snippets.swift @@ -6,10 +6,6 @@ extension AppStrings { "Add a host before choosing default targets." } - public static var addHosts: LocalizedStringResource { - "Add Hosts" - } - public static var deletePackage: LocalizedStringResource { "Delete Package" } diff --git a/TFI/Application/FeatureModels/MobileHostPickerModel.swift b/TFI/Application/FeatureModels/MobileHostPickerModel.swift index 0675fce7..a39c759b 100644 --- a/TFI/Application/FeatureModels/MobileHostPickerModel.swift +++ b/TFI/Application/FeatureModels/MobileHostPickerModel.swift @@ -10,18 +10,13 @@ final class MobileHostPickerModel { var query = "" private let library: HostLibraryModel - private let useCase: HostUseCase? - init(library: HostLibraryModel, useCase: HostUseCase? = nil) { + init(library: HostLibraryModel) { self.library = library - self.useCase = useCase } var isLibraryEmpty: Bool { library.hosts.isEmpty } - var hosts: [Host] { - if let useCase { return library.hosts(matching: query, for: useCase) } - return library.searchHosts(query) - } + var hosts: [Host] { library.searchHosts(query) } func loginAddress(for host: Host) -> String { library.preferredTerminalLoginAddress(for: host) } } diff --git a/TFI/Application/FeatureModels/MobileMultiHostPickerModel.swift b/TFI/Application/FeatureModels/MobileMultiHostPickerModel.swift new file mode 100644 index 00000000..50f89a81 --- /dev/null +++ b/TFI/Application/FeatureModels/MobileMultiHostPickerModel.swift @@ -0,0 +1,52 @@ +import Observation +import TermindApplication +import TermindDomain + +import struct TermindDomain.Host + +@MainActor +@Observable +final class MobileMultiHostPickerModel { + var query = "" + private(set) var selected: Set + + private let library: HostLibraryModel + private let useCase: HostUseCase + private let isEligible: (Host) -> Bool + + init( + library: HostLibraryModel, useCase: HostUseCase, selected: Set, + isEligible: @escaping (Host) -> Bool = { _ in true } + ) { + self.library = library + self.useCase = useCase + self.selected = selected + self.isEligible = isEligible + } + + var isLibraryEmpty: Bool { library.hosts.isEmpty } + var hosts: [Host] { library.hosts(matching: query, for: useCase).filter(isEligible) } + var availableHostIDs: [Host.ID] { library.hosts(matching: "", for: useCase).filter(isEligible).map(\.id) } + var orderedSelection: [Host.ID] { availableHostIDs.filter { selected.contains($0) } } + var allResultsSelected: Bool { !hosts.isEmpty && hosts.allSatisfy { selected.contains($0.id) } } + + func loginAddress(for host: Host) -> String { library.loginAddress(for: host) } + + func reconcileSelection() { selected.formIntersection(availableHostIDs) } + + func toggle(_ id: Host.ID) { + if selected.contains(id) { + selected.remove(id) + } else { + selected.insert(id) + } + } + + func toggleAllResults() { + if allResultsSelected { + selected.subtract(hosts.map(\.id)) + } else { + selected.formUnion(hosts.map(\.id)) + } + } +} diff --git a/TFI/Application/Metrics/MobileMetricsModel.swift b/TFI/Application/Metrics/MobileMetricsModel.swift index ee473111..2262535f 100644 --- a/TFI/Application/Metrics/MobileMetricsModel.swift +++ b/TFI/Application/Metrics/MobileMetricsModel.swift @@ -27,9 +27,14 @@ final class MobileMetricsModel { } func add(_ id: UUID, pro: Bool) -> Bool { + add([id], pro: pro) + } + + func add(_ ids: [UUID], pro: Bool) -> Bool { limit.maximum = pro ? nil : ServerMetricsModel.freeCardLimit - guard let host = hosts.host(id) else { return false } - return core.add(host) + let batch = ids.compactMap(hosts.host) + guard !batch.isEmpty else { return false } + return core.add(batch) } func remove(_ id: UUID) { core.remove(id) } diff --git a/TFI/Features/HostChooser/MobileConnectionPicker.swift b/TFI/Features/HostChooser/MobileConnectionPicker.swift index a2b6b011..0344221a 100644 --- a/TFI/Features/HostChooser/MobileConnectionPicker.swift +++ b/TFI/Features/HostChooser/MobileConnectionPicker.swift @@ -123,7 +123,6 @@ struct MobileHostPicker: View { init( library: HostLibraryModel, - useCase: HostUseCase? = nil, title: LocalizedStringResource, emptyDescription: LocalizedStringResource, cancel: @escaping () -> Void, @@ -131,7 +130,7 @@ struct MobileHostPicker: View { isEligible: @escaping (Host) -> Bool = { _ in true }, @ViewBuilder header: () -> Header ) { - _model = State(initialValue: MobileHostPickerModel(library: library, useCase: useCase)) + _model = State(initialValue: MobileHostPickerModel(library: library)) self.title = title self.emptyDescription = emptyDescription self.cancel = cancel @@ -183,25 +182,3 @@ struct MobileHostPicker: View { private var eligibleHosts: [Host] { model.hosts.filter(isEligible) } } - -extension MobileHostPicker where Header == EmptyView { - init( - library: HostLibraryModel, - useCase: HostUseCase? = nil, - title: LocalizedStringResource, - emptyDescription: LocalizedStringResource, - cancel: @escaping () -> Void, - select: @escaping (Host) -> Void, - isEligible: @escaping (Host) -> Bool = { _ in true } - ) { - self.init( - library: library, - useCase: useCase, - title: title, - emptyDescription: emptyDescription, - cancel: cancel, - select: select, - isEligible: isEligible, - header: { EmptyView() }) - } -} diff --git a/TFI/Features/HostChooser/MobileMultiHostPicker.swift b/TFI/Features/HostChooser/MobileMultiHostPicker.swift new file mode 100644 index 00000000..c4535e06 --- /dev/null +++ b/TFI/Features/HostChooser/MobileMultiHostPicker.swift @@ -0,0 +1,99 @@ +import SwiftUI +import TermindApplication +import TermindDomain +import TermindPresentation + +import struct TermindDomain.Host + +struct MobileMultiHostPicker: View { + let title: LocalizedStringResource + let actionTitle: LocalizedStringResource + let emptyDescription: LocalizedStringResource + let requiresSelection: Bool + let save: ([Host.ID]) -> Void + @Environment(\.dismiss) private var dismiss + @State private var model: MobileMultiHostPickerModel + + init( + library: HostLibraryModel, useCase: HostUseCase, selected: Set, + title: LocalizedStringResource, actionTitle: LocalizedStringResource, + emptyDescription: LocalizedStringResource, requiresSelection: Bool, + isEligible: @escaping (Host) -> Bool = { _ in true }, + save: @escaping ([Host.ID]) -> Void + ) { + self.title = title + self.actionTitle = actionTitle + self.emptyDescription = emptyDescription + self.requiresSelection = requiresSelection + self.save = save + _model = State( + initialValue: MobileMultiHostPickerModel( + library: library, useCase: useCase, selected: selected, isEligible: isEligible)) + } + + var body: some View { + NavigationStack { + List { + if model.isLibraryEmpty { + ContentUnavailableView( + AppStrings.Vault.noHosts.swiftUIKey, systemImage: "server.rack", + description: Text(emptyDescription) + ) + .listRowBackground(Color.clear) + .listRowSeparator(.hidden) + } else if model.hosts.isEmpty { + ContentUnavailableView.search(text: model.query) + .listRowBackground(Color.clear) + .listRowSeparator(.hidden) + } else { + Section { + Button( + model.allResultsSelected ? AppStrings.Common.deselectAll : AppStrings.Common.selectAll, + action: model.toggleAllResults + ) + .accessibilityIdentifier("host-picker.select-all") + } + Section { + ForEach(model.hosts) { host in + Button { + model.toggle(host.id) + } label: { + MobileHostPickerRow(host: host, loginAddress: model.loginAddress(for: host)) { + if model.selected.contains(host.id) { + Image(systemName: "checkmark") + .font(.body.weight(.semibold)) + .foregroundStyle(.tint) + .accessibilityHidden(true) + } + } + } + .buttonStyle(.plain) + .accessibilityIdentifier("host-picker.host.\(host.id)") + .accessibilityAddTraits(model.selected.contains(host.id) ? [.isSelected] : []) + } + } + } + } + .searchable(text: $model.query.singleLineText, prompt: AppStrings.Vault.searchHosts.swiftUIKey) + .navigationTitle(Text(title)) + .navigationBarTitleDisplayMode(.inline) + .toolbar { + ToolbarItem(placement: .cancellationAction) { + Button(AppStrings.Common.cancel.swiftUIKey, systemImage: "xmark") { dismiss() } + .labelStyle(.iconOnly) + } + ToolbarItem(placement: .confirmationAction) { + Button(actionTitle, systemImage: "checkmark") { + save(model.orderedSelection) + dismiss() + } + .labelStyle(.iconOnly) + .foregroundStyle(.tint) + .disabled(requiresSelection && model.orderedSelection.isEmpty) + } + } + .onChange(of: model.availableHostIDs, initial: true) { model.reconcileSelection() } + } + .mobileSheetPresentation(compact: true) + } +} diff --git a/TFI/Features/Metrics/MobileMetricsView.swift b/TFI/Features/Metrics/MobileMetricsView.swift index 84c04be4..04660b1a 100644 --- a/TFI/Features/Metrics/MobileMetricsView.swift +++ b/TFI/Features/Metrics/MobileMetricsView.swift @@ -7,6 +7,7 @@ struct MobileMetricsView: View { let app: MobileAppModel @Environment(\.dynamicTypeSize) private var typeSize @State private var selecting = false + @State private var pendingHostIDs: [UUID] = [] @State private var selectedID: UUID? var body: some View { @@ -48,7 +49,7 @@ struct MobileMetricsView: View { } .navigationTitle(AppStrings.Navigation.metrics.swiftUIKey) .toolbar { - Button(AppStrings.Metrics.addServer.swiftUIKey, systemImage: "plus") { selecting = true }.labelStyle( + Button(AppStrings.Common.addHosts.swiftUIKey, systemImage: "plus") { selecting = true }.labelStyle( .iconOnly) } .navigationDestination(item: $selectedID) { id in @@ -56,28 +57,30 @@ struct MobileMetricsView: View { MobileMetricsDetailView(app: app, server: server) } } - .sheet(isPresented: $selecting) { - NavigationStack { - MobileHostPicker( - library: app.library, - useCase: .metrics, - title: AppStrings.Metrics.addServer, - emptyDescription: AppStrings.Metrics.addASavedLinuxServerToStartMonitoring, - cancel: { selecting = false }, - select: { host in - selecting = false - if !app.metrics.add(host.id, pro: app.pro.hasProAccess) { - if let error = app.metrics.core.storageError { - app.error = error - } else { - app.pro.presentPaywall(for: .unlimitedMetrics) - } - } - }) - }.mobileSheetPresentation(compact: true) + .sheet(isPresented: $selecting, onDismiss: addSelectedHosts) { + MobileMultiHostPicker( + library: app.library, useCase: .metrics, selected: [], + title: AppStrings.Common.addHosts, actionTitle: AppStrings.Common.addHosts, + emptyDescription: AppStrings.Metrics.addASavedLinuxServerToStartMonitoring, + requiresSelection: true, + isEligible: { host in !app.metrics.selections.contains(where: { $0.id == host.id }) } + ) { pendingHostIDs = $0 } } .task { app.metrics.start() } } + + private func addSelectedHosts() { + let ids = pendingHostIDs + pendingHostIDs = [] + guard !ids.isEmpty else { return } + if !app.metrics.add(ids, pro: app.pro.hasProAccess) { + if let error = app.metrics.core.storageError { + app.error = error + } else { + app.pro.presentPaywall(for: .unlimitedMetrics) + } + } + } } struct MobileMetricsDetailView: View { diff --git a/TFI/Features/Snippets/MobileSnippetsView.swift b/TFI/Features/Snippets/MobileSnippetsView.swift index b77bb7e8..792c78e4 100644 --- a/TFI/Features/Snippets/MobileSnippetsView.swift +++ b/TFI/Features/Snippets/MobileSnippetsView.swift @@ -119,8 +119,10 @@ struct MobileSnippetsView: View { parentID: request.parentID) } .sheet(item: $targetRequest, onDismiss: completeTargetSelection) { request in - MobileSnippetHostPicker( - app: app, selected: [], title: AppStrings.Common.run, actionTitle: AppStrings.Common.run, + MobileMultiHostPicker( + library: app.library, useCase: .snippetExecution, selected: [], title: AppStrings.Common.run, + actionTitle: AppStrings.Common.run, + emptyDescription: AppStrings.Snippets.addAHostBeforeChoosingDefaultTargets, requiresSelection: true ) { ids in pendingTargetExecution = ExecutionRequest(snippetID: request.snippetID, hostIDs: ids) @@ -648,7 +650,7 @@ struct MobileSnippetEditor: View { Button { choosingTargets = true } label: { - Label(AppStrings.Snippets.addHosts.swiftUIKey, systemImage: "server.rack") + Label(AppStrings.Common.addHosts.swiftUIKey, systemImage: "server.rack") } .disabled(app.snippets.hosts.isEmpty) if selectedHosts.isEmpty { @@ -680,9 +682,11 @@ struct MobileSnippetEditor: View { } } .sheet(isPresented: $choosingTargets) { - MobileSnippetHostPicker( - app: app, selected: Set(draft.targetHostIDs), title: AppStrings.Snippets.addHosts, - actionTitle: AppStrings.Common.done, requiresSelection: false + MobileMultiHostPicker( + library: app.library, useCase: .snippetExecution, selected: Set(draft.targetHostIDs), + title: AppStrings.Common.addHosts, + actionTitle: AppStrings.Common.done, + emptyDescription: AppStrings.Snippets.addAHostBeforeChoosingDefaultTargets, requiresSelection: false ) { ids in draft.targetHostIDs = ids choosingTargets = false @@ -798,123 +802,3 @@ struct MobileSnippetPackageEditor: View { didSave(draft) } } - -struct MobileSnippetHostPicker: View { - let app: MobileAppModel - let title: LocalizedStringResource - let actionTitle: LocalizedStringResource - let requiresSelection: Bool - let save: ([Host.ID]) -> Void - @Environment(\.dismiss) private var dismiss - @State private var query = "" - @State private var selected: Set - - init( - app: MobileAppModel, selected: Set, title: LocalizedStringResource, - actionTitle: LocalizedStringResource, requiresSelection: Bool, - save: @escaping ([Host.ID]) -> Void - ) { - self.app = app - self.title = title - self.actionTitle = actionTitle - self.requiresSelection = requiresSelection - self.save = save - _selected = State(initialValue: selected) - } - - private var results: [Host] { app.snippets.searchHosts(query) } - private var allResultsSelected: Bool { - !results.isEmpty && results.allSatisfy { selected.contains($0.id) } - } - - var body: some View { - NavigationStack { - List { - if app.snippets.hosts.isEmpty { - ContentUnavailableView( - AppStrings.Vault.noHosts.swiftUIKey, systemImage: "server.rack", - description: Text( - AppStrings.Snippets.addAHostBeforeChoosingDefaultTargets - ) - ) - .listRowBackground(Color.clear) - .listRowSeparator(.hidden) - } else if results.isEmpty { - ContentUnavailableView.search(text: query) - .listRowBackground(Color.clear) - .listRowSeparator(.hidden) - } else { - Section { - Button( - allResultsSelected - ? String(localized: AppStrings.Common.deselectAll) - : String(localized: AppStrings.Common.selectAll) - ) { - toggleAllResults() - } - } - Section { - ForEach(results) { host in - Button { - toggle(host.id) - } label: { - MobileHostPickerRow( - host: host, - loginAddress: app.snippets.loginAddress(for: host) - ) { - if selected.contains(host.id) { - Image(systemName: "checkmark") - .font(.body.weight(.semibold)) - .foregroundStyle(.tint) - .accessibilityHidden(true) - } - } - } - .buttonStyle(.plain) - .accessibilityAddTraits(selected.contains(host.id) ? [.isSelected] : []) - } - } - } - } - .searchable(text: $query.singleLineText, prompt: AppStrings.Vault.searchHosts.swiftUIKey) - .navigationTitle(Text(title)) - .navigationBarTitleDisplayMode(.inline) - .toolbar { - ToolbarItem(placement: .cancellationAction) { - Button(AppStrings.Common.cancel.swiftUIKey, systemImage: "xmark") { dismiss() }.labelStyle( - .iconOnly) - } - ToolbarItem(placement: .confirmationAction) { - Button(actionTitle, systemImage: "checkmark") { - let ordered = app.snippets.searchHosts("").filter { selected.contains($0.id) }.map(\.id) - save(ordered) - dismiss() - } - .labelStyle(.iconOnly) - .foregroundStyle(.tint) - .disabled(requiresSelection && selected.isEmpty) - } - } - .onAppear { - selected.formIntersection(Set(app.snippets.hosts.map(\.id))) - } - } - .mobileSheetPresentation(compact: true) - } - - private func toggle(_ id: Host.ID) { - if selected.contains(id) { - selected.remove(id) - } else { - selected.insert(id) - } - } - - private func toggleAllResults() { - if allResultsSelected { - selected.subtract(results.map(\.id)) - } else { - selected.formUnion(results.map(\.id)) - } - } -} diff --git a/TFITests/MobileConnectionPickerTests.swift b/TFITests/MobileConnectionPickerTests.swift index d0a340ba..6407cdc7 100644 --- a/TFITests/MobileConnectionPickerTests.swift +++ b/TFITests/MobileConnectionPickerTests.swift @@ -2,6 +2,7 @@ import Foundation import SwiftUI import TermindApplication import TermindDomain +import TermindPresentation import Testing import UIKit @@ -190,14 +191,20 @@ struct MobileConnectionPickerTests { #expect(app.connectionHistory.entries(for: host.id).map(\.kind) == [.telnet]) } - @Test func snippetHostPickerGatesConfirmationOnRequiredSelection() async throws { + @Test( + .enabled( + if: ProcessInfo.processInfo.environment["TERMIND_MOBILE_UI_TESTS"] == "1", + "Set TERMIND_MOBILE_UI_TESTS=1 and disable parallel testing for key-window integration tests"), + arguments: [HostUseCase.snippetExecution, .metrics]) + func multiHostPickerGatesConfirmationOnRequiredSelection(useCase: HostUseCase) async throws { let app = try MobileAppModel(inMemory: true) let host = Host(name: "Production", host: "production.example.invalid") app.library.upsert(host) try await withStandaloneNavigation( - MobileSnippetHostPicker( - app: app, selected: [], title: "Run", actionTitle: "Run", + MobileMultiHostPicker( + library: app.library, useCase: useCase, selected: [], title: "Run", actionTitle: "Run", + emptyDescription: AppStrings.Snippets.addAHostBeforeChoosingDefaultTargets, requiresSelection: true, save: { _ in }), language: "en" ) { navigation in @@ -206,8 +213,10 @@ struct MobileConnectionPickerTests { } try await withStandaloneNavigation( - MobileSnippetHostPicker( - app: app, selected: [host.id], title: "Add Hosts", actionTitle: "Done", + MobileMultiHostPicker( + library: app.library, useCase: useCase, selected: [host.id], title: "Add Hosts", + actionTitle: "Done", + emptyDescription: AppStrings.Snippets.addAHostBeforeChoosingDefaultTargets, requiresSelection: false, save: { _ in }), language: "en" ) { navigation in diff --git a/TFITests/MobileIconRenderingTests.swift b/TFITests/MobileIconRenderingTests.swift index 35b9d6af..40f38259 100644 --- a/TFITests/MobileIconRenderingTests.swift +++ b/TFITests/MobileIconRenderingTests.swift @@ -1,7 +1,9 @@ import Foundation import GhosttyTerminal import SwiftUI +import TermindApplication import TermindDomain +import TermindPresentation import Testing import UIKit @@ -13,6 +15,33 @@ import struct TermindDomain.Host /// Set TERMIND_ICON_SCREENSHOTS to en, zh-Hans, or zh-Hant in the test runner environment. @Suite(.serialized) @MainActor struct MobileIconRenderingTests { + @Test func renderMultiHostPickers() async throws { + guard let language = ProcessInfo.processInfo.environment["TERMIND_ICON_SCREENSHOTS"] else { return } + let hosts = ["Production", "Staging", "Development"].enumerated().map { index, name in + Host(name: name, host: "\(name.lowercased()).invalid", sortIndex: Double(index)) + } + let library = HostLibraryModel(hosts: hosts) + for useCase in [HostUseCase.metrics, .snippetExecution] { + let metrics = useCase == .metrics + let name = metrics ? "metrics-multi-picker" : "snippet-multi-picker" + for scheme in [ColorScheme.light, .dark] { + let suffix = scheme == .dark ? "dark" : "light" + try await capture("\(name)-\(suffix)", language: language, scheme: scheme) { + MobileMultiHostPicker( + library: library, useCase: useCase, selected: [hosts[0].id, hosts[2].id], + title: AppStrings.Common.addHosts, + actionTitle: metrics ? AppStrings.Common.addHosts : AppStrings.Common.done, + emptyDescription: metrics + ? AppStrings.Metrics.addASavedLinuxServerToStartMonitoring + : AppStrings.Snippets.addAHostBeforeChoosingDefaultTargets, + requiresSelection: metrics, save: { _ in } + ) + .environment(\.locale, Locale(identifier: language)) + } + } + } + } + @Test func terminalKeyboardKeepsItsCompactHeight() { #expect(UIImage(systemName: "play.rectangle") != nil) #expect(UIImage(systemName: "doc.on.clipboard") != nil) @@ -78,16 +107,19 @@ struct MobileIconRenderingTests { .environment(\.locale, locale) } try await capture("snippet-targets-\(suffix)", language: language, scheme: scheme) { - MobileSnippetHostPicker( - app: snippetApp, selected: [snippetHost.id], title: "Add Hosts", - actionTitle: "Done", requiresSelection: false, save: { _ in } + MobileMultiHostPicker( + library: snippetApp.library, useCase: .snippetExecution, selected: [snippetHost.id], + title: "Add Hosts", + actionTitle: "Done", emptyDescription: AppStrings.Snippets.addAHostBeforeChoosingDefaultTargets, + requiresSelection: false, save: { _ in } ) .environment(\.locale, locale) } try await capture("snippet-execution-targets-\(suffix)", language: language, scheme: scheme) { - MobileSnippetHostPicker( - app: snippetApp, selected: [], title: "Run", - actionTitle: "Run", requiresSelection: true, save: { _ in } + MobileMultiHostPicker( + library: snippetApp.library, useCase: .snippetExecution, selected: [], title: "Run", + actionTitle: "Run", emptyDescription: AppStrings.Snippets.addAHostBeforeChoosingDefaultTargets, + requiresSelection: true, save: { _ in } ) .environment(\.locale, locale) } @@ -123,14 +155,12 @@ struct MobileIconRenderingTests { .environment(\.locale, locale) } try await capture("metrics-host-picker-\(suffix)", language: language, scheme: scheme) { - NavigationStack { - MobileHostPicker( - library: connectionApp.library, - title: "Add Server", - emptyDescription: "Add a saved Linux server to start monitoring.", - cancel: {}, - select: { _ in }) - } + MobileMultiHostPicker( + library: connectionApp.library, useCase: .metrics, selected: [], + title: AppStrings.Common.addHosts, actionTitle: AppStrings.Common.addHosts, + emptyDescription: AppStrings.Metrics.addASavedLinuxServerToStartMonitoring, + requiresSelection: true, save: { _ in } + ) .environment(\.locale, locale) } try await capture("connection-settings-\(suffix)", language: language, scheme: scheme) { diff --git a/TFITests/MobileMarketingScreenshotTests.swift b/TFITests/MobileMarketingScreenshotTests.swift index 7a51150a..69564f06 100644 --- a/TFITests/MobileMarketingScreenshotTests.swift +++ b/TFITests/MobileMarketingScreenshotTests.swift @@ -423,7 +423,7 @@ private struct MarketingMetricsScene: View { .navigationTitle("Metrics") .toolbar { ToolbarItem(placement: .primaryAction) { - Button("Add Server", systemImage: "plus") {}.labelStyle(.iconOnly) + Button(AppStrings.Common.addHosts, systemImage: "plus") {}.labelStyle(.iconOnly) } } } diff --git a/TFITests/MobileMultiHostPickerTests.swift b/TFITests/MobileMultiHostPickerTests.swift new file mode 100644 index 00000000..13b00222 --- /dev/null +++ b/TFITests/MobileMultiHostPickerTests.swift @@ -0,0 +1,106 @@ +import Foundation +import TermindApplication +import TermindDomain +import Testing + +import struct TermindDomain.Host + +@testable import TermindIOS + +@Suite @MainActor +struct MobileMultiHostPickerTests { + @Test(arguments: [HostUseCase.metrics, .snippetExecution]) + func filteredSelectAllPreservesOtherSelectionsAndLibraryOrder(useCase: HostUseCase) { + let hosts = ["Alpha", "Bravo", "Charlie"].enumerated().map { index, name in + Host(name: name, host: "\(name.lowercased()).invalid", sortIndex: Double(index)) + } + let picker = MobileMultiHostPickerModel( + library: HostLibraryModel(hosts: hosts), useCase: useCase, selected: []) + + picker.toggle(hosts[2].id) + picker.query = "Alpha" + picker.toggleAllResults() + #expect(picker.allResultsSelected) + #expect(picker.orderedSelection == [hosts[0].id, hosts[2].id]) + picker.toggleAllResults() + #expect(picker.orderedSelection == [hosts[2].id]) + #expect(!picker.allResultsSelected) + picker.query = "missing" + picker.toggleAllResults() + #expect(!picker.allResultsSelected) + #expect(picker.orderedSelection == [hosts[2].id]) + picker.query = "" + picker.toggleAllResults() + #expect(picker.orderedSelection == hosts.map(\.id)) + picker.toggleAllResults() + #expect(picker.orderedSelection.isEmpty) + } + + @Test + func metricsExcludesMonitoredAndNonSSHHostsAndPrunesUnavailableSelections() { + let monitored = Host(name: "Monitored", host: "monitored.invalid") + var eligible = Host(name: "Available", host: "available.invalid") + let telnet = Host(name: "Telnet", host: "telnet.invalid", sshEnabled: false, telnetEnabled: true) + let library = HostLibraryModel(hosts: [monitored, eligible, telnet]) + let picker = MobileMultiHostPickerModel( + library: library, useCase: .metrics, selected: [monitored.id, eligible.id, telnet.id, UUID()], + isEligible: { $0.id != monitored.id }) + + picker.reconcileSelection() + #expect(picker.hosts.map(\.id) == [eligible.id]) + #expect(picker.selected == [eligible.id]) + eligible.sshEnabled = false + eligible.telnetEnabled = true + library.upsert(eligible) + #expect(library.storageError == nil) + #expect(picker.orderedSelection.isEmpty) + picker.reconcileSelection() + #expect(picker.selected.isEmpty) + } + + @Test + func batchMetricsAdditionIsAtomicAndUsesTheCurrentEntitlement() { + let hosts = (0..<5).map { Host(name: "Host \($0)", host: "host\($0).invalid") } + let preferences = PickerMetricsPreferences() + let metrics = MobileMetricsModel( + preferences: preferences, hosts: HostLibraryModel(hosts: hosts), services: .testing) + defer { metrics.core.shutdown() } + metrics.pause() + + #expect(metrics.add([hosts[0].id], pro: false)) + let saved = preferences.values + #expect(!metrics.add(hosts[1...3].map(\.id), pro: false)) + #expect(preferences.values == saved) + #expect(metrics.selections.map(\.id) == [hosts[0].id]) + #expect(metrics.add([hosts[0].id, hosts[1].id, hosts[1].id, hosts[2].id], pro: false)) + #expect(metrics.selections.count == 3) + #expect(metrics.add(hosts[3...4].map(\.id), pro: true)) + #expect(metrics.selections.map(\.id) == hosts.map(\.id)) + } + + @Test + func batchMetricsSaveFailureDoesNotAddAnySelections() { + let hosts = (0..<2).map { Host(name: "Host \($0)", host: "host\($0).invalid") } + let preferences = PickerMetricsPreferences() + preferences.failsSaving = true + let metrics = MobileMetricsModel( + preferences: preferences, hosts: HostLibraryModel(hosts: hosts), services: .testing) + defer { metrics.core.shutdown() } + metrics.pause() + + #expect(!metrics.add(hosts.map(\.id), pro: true)) + #expect(metrics.selections.isEmpty) + #expect(preferences.values.isEmpty) + #expect(metrics.core.storageError != nil) + } +} + +private final class PickerMetricsPreferences: MetricsConfigurationStoring { + var values: [MetricsSelection] = [] + var failsSaving = false + func load() -> [MetricsSelection] { values } + func save(_ selections: [MetricsSelection]) throws { + if failsSaving { throw CocoaError(.fileWriteUnknown) } + values = selections + } +} diff --git a/TFM/Features/HostChooser/HostChooserView.swift b/TFM/Features/HostChooser/HostChooserView.swift index b2d02b2c..f3d5200e 100644 --- a/TFM/Features/HostChooser/HostChooserView.swift +++ b/TFM/Features/HostChooser/HostChooserView.swift @@ -52,7 +52,7 @@ nonisolated enum HostChooserPurpose: Hashable, Sendable, Identifiable { switch self { case .connect: AppStrings.Common.connect case .batchStartup: AppStrings.Vault.batchStartup - case .metrics: AppStrings.Metrics.addServer + case .metrics: AppStrings.Common.addHosts case .snippet: AppStrings.Common.run } } @@ -71,7 +71,7 @@ nonisolated enum HostChooserPurpose: Hashable, Sendable, Identifiable { var confirmLabel: LocalizedStringResource { switch self { case .connect, .batchStartup: AppStrings.Common.connect - case .metrics: AppStrings.Metrics.addServer + case .metrics: AppStrings.Common.addHosts case .snippet: AppStrings.Common.run } } @@ -109,11 +109,11 @@ nonisolated enum HostChooserPurpose: Hashable, Sendable, Identifiable { } } - func connectionChooserShowsConfirmation(for connectionProtocol: HostConnectionProtocol) -> Bool { + func formShowsConfirmation(for connectionProtocol: HostConnectionProtocol) -> Bool { switch self { case .connect: connectionProtocol == .serial - case .batchStartup: true - case .metrics, .snippet: false + case .batchStartup, .metrics: true + case .snippet: false } } } @@ -180,11 +180,12 @@ struct HostChooserView: View { ) } - /// The protocol-choosing variant carries an extra row of controls above the - /// list and needs the room; everything else uses the standard picker width. private var sheetWidth: CGFloat? { guard presentation == .sheet else { return nil } - return purpose.choosesConnectionProtocol ? 520 : Metrics.HostPicker.width + switch purpose { + case .connect, .batchStartup, .metrics: return 520 + case .snippet: return Metrics.HostPicker.width + } } private var query: String { chooser.query } @@ -209,9 +210,9 @@ struct HostChooserView: View { var body: some View { Group { switch purpose { - case .connect, .batchStartup: - connectionChooser - case .metrics, .snippet: + case .connect, .batchStartup, .metrics: + formChooser + case .snippet: managedChooser } } @@ -261,15 +262,15 @@ struct HostChooserView: View { SheetScaffold { managedHeader } content: { - managedContent + hostList } footer: { managedFooter } } - private var connectionChooser: some View { + private var formChooser: some View { VStack(spacing: 0) { - connectionTopBar + formTopBar if connectionProtocol == .serial { serialForm @@ -284,7 +285,14 @@ struct HostChooserView: View { ) .padding(.horizontal, Metrics.Space.xl) - connectionHostList + if purpose == .metrics, app.serverMetrics.remainingCardSlots != nil { + Text(AppStrings.Pro.freeMetricsLimit) + .font(.caption).foregroundStyle(.secondary) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(.horizontal, Metrics.Space.xl) + } + + multiSelectionHostList } .padding(.top, Metrics.Space.xs) } @@ -293,7 +301,7 @@ struct HostChooserView: View { .background(TermindColor.windowSurface) } - private var connectionTopBar: some View { + private var formTopBar: some View { HStack { glassActionButton( AppStrings.Common.cancel, @@ -320,9 +328,13 @@ struct HostChooserView: View { } .frame(maxWidth: .infinity) .overlay { - connectionProtocolPicker - .termindGlassSurface(cornerRadius: Metrics.Radius.medium, isInteractive: true) - .frame(width: protocolPickerWidth, height: 30) + if purpose.choosesConnectionProtocol { + connectionProtocolPicker + .termindGlassSurface(cornerRadius: Metrics.Radius.medium, isInteractive: true) + .frame(width: protocolPickerWidth, height: 30) + } else { + Text(purpose.title).font(.headline) + } } .padding(.horizontal, Metrics.Space.xl) .padding(.top, Metrics.Space.l) @@ -355,7 +367,7 @@ struct HostChooserView: View { } private var showsTopConfirmation: Bool { - purpose.connectionChooserShowsConfirmation(for: connectionProtocol) + purpose.formShowsConfirmation(for: connectionProtocol) } private var managedHeader: some View { @@ -364,23 +376,12 @@ struct HostChooserView: View { symbol: purpose.symbol, prompt: AppStrings.Vault.searchHosts, query: queryBinding, - aboveSearch: chooserOptions, - showsSearch: connectionProtocol != .serial, onSubmit: openSelected, moveCursor: move, toggleCursor: toggleCursor ) } - private var chooserOptions: AnyView? { - if purpose == .metrics, app.serverMetrics.remainingCardSlots != nil { - return AnyView( - Text(AppStrings.Pro.freeMetricsLimit) - .font(.caption).foregroundStyle(.secondary)) - } - return nil - } - private var connectionProtocolPicker: some View { Picker(AppStrings.Vault.connectionProtocol.swiftUIKey, selection: $connectionProtocol) { ForEach(purpose.availableConnectionProtocols) { connectionProtocol in @@ -392,54 +393,7 @@ struct HostChooserView: View { .accessibilityLabel(AppStrings.Vault.connectionProtocolLabel.swiftUIKey) } - @ViewBuilder - private var managedContent: some View { - if app.hostLibrary.hosts.isEmpty { - ContentUnavailableView { - Label(AppStrings.Vault.noHosts.swiftUIKey, systemImage: "server.rack") - } description: { - Text(AppStrings.Vault.addAHostToConnectToIt) - } actions: { - Button(AppStrings.Vault.newHost.swiftUIKey) { - close() - window.openHostEditor(host: nil) - } - } - } else if results.isEmpty { - ContentUnavailableView.search(text: query) - } else { - ScrollViewReader { proxy in - List { - ForEach(results) { host in - Button { - activate(host) - } label: { - HostChooserRow( - host: host, - isCursor: chooser.cursor == host.id, - isChecked: purpose.allowsMultiple - ? chooser.selected.contains(host.id) : nil, - connectionProtocol: connectionProtocol - ) - } - .buttonStyle(.plain) - .accessibilityIdentifier("host-chooser.host.\(host.id)") - .id(host.id) - .listRowBackground( - chooser.cursor == host.id - ? Color.accentColor.opacity(0.11) : Color.clear - ) - } - } - .listStyle(.inset) - .onAppear { scrollCursorIntoView(using: proxy) } - .onChange(of: chooser.cursor) { _, _ in scrollCursorIntoView(using: proxy) } - .onChange(of: results.map(\.id)) { _, _ in scrollCursorIntoView(using: proxy) } - } - } - } - - private var connectionHostList: some View { + private var multiSelectionHostList: some View { VStack(spacing: 0) { if purpose.allowsMultiple { Button(selectAllLabel) { @@ -456,12 +410,12 @@ struct HostChooserView: View { Divider() } - connectionHosts + hostList } } @ViewBuilder - private var connectionHosts: some View { + private var hostList: some View { if app.hostLibrary.hosts.isEmpty { ContentUnavailableView { Label(AppStrings.Vault.noHosts.swiftUIKey, systemImage: "server.rack") diff --git a/TFM/Features/Snippets/SnippetsPageView.swift b/TFM/Features/Snippets/SnippetsPageView.swift index 696f71ef..cdcb70fe 100644 --- a/TFM/Features/Snippets/SnippetsPageView.swift +++ b/TFM/Features/Snippets/SnippetsPageView.swift @@ -665,11 +665,11 @@ struct SnippetEditorSheet: View { Button { route = .targets } label: { - Label(AppStrings.Snippets.addHosts.swiftUIKey, systemImage: "plus") + Label(AppStrings.Common.addHosts.swiftUIKey, systemImage: "plus") } .disabled(app.snippets.hosts.isEmpty) .accessibilityIdentifier("snippet-editor.add-hosts") - .help(AppStrings.Snippets.addHosts.swiftUIKey) + .help(AppStrings.Common.addHosts.swiftUIKey) } } if let error { Text(error).foregroundStyle(.red) } @@ -792,7 +792,7 @@ struct SnippetTargetPickerView: View { var body: some View { VaultDetailScaffold( - title: AppStrings.Snippets.addHosts, symbol: "server.rack", close: close, + title: AppStrings.Common.addHosts, symbol: "server.rack", close: close, closeLabel: AppStrings.Common.back, closeSymbol: "chevron.backward" ) { VStack(spacing: 0) { @@ -816,7 +816,7 @@ struct SnippetTargetPickerView: View { private var header: some View { ManagedChooserHeader( - title: AppStrings.Snippets.addHosts, + title: AppStrings.Common.addHosts, symbol: "server.rack", prompt: AppStrings.Vault.searchHosts, query: queryBinding, diff --git a/TFM/UI/Components/ChromeComponents.swift b/TFM/UI/Components/ChromeComponents.swift index 1bdd8a05..dc1d5e5e 100644 --- a/TFM/UI/Components/ChromeComponents.swift +++ b/TFM/UI/Components/ChromeComponents.swift @@ -141,8 +141,6 @@ struct ManagedChooserHeader: View { let symbol: String let prompt: LocalizedStringResource @Binding var query: String - var aboveSearch: AnyView? = nil - var showsSearch = true var onSubmit: () -> Void = {} var moveCursor: (Int) -> KeyPress.Result = { _ in .ignored } var toggleCursor: () -> KeyPress.Result = { .ignored } @@ -155,29 +153,10 @@ struct ManagedChooserHeader: View { .font(.headline) .labelStyle(.titleOnly) - if let aboveSearch { - aboveSearch - } - - if showsSearch { - searchRow - .transition( - .opacity.combined(with: .move(edge: .top)) - ) - } + searchRow } .padding(Metrics.Space.xl) - .animation(Motion.panel, value: showsSearch) - .onAppear { - if showsSearch { searchFocused = true } - } - .onChange(of: showsSearch) { _, visible in - if visible { - Task { @MainActor in searchFocused = true } - } else { - searchFocused = false - } - } + .onAppear { searchFocused = true } } private var searchRow: some View { diff --git a/TFM/UI/Components/WindowToolbar.swift b/TFM/UI/Components/WindowToolbar.swift index a90392a2..b396c825 100644 --- a/TFM/UI/Components/WindowToolbar.swift +++ b/TFM/UI/Components/WindowToolbar.swift @@ -716,7 +716,7 @@ extension WindowModel { case .metrics: guard metricsHostID == nil else { return [] } return [ - WindowToolbarCreationAction(id: "metrics-server", title: AppStrings.Metrics.addServer, symbol: "plus") { + WindowToolbarCreationAction(id: "metrics-server", title: AppStrings.Common.addHosts, symbol: "plus") { guard self.metricsModel?.requestAddition() ?? true else { return } self.pendingMetricsHostIDs = [] self.presentations.hostChooser = .metrics diff --git a/TFMTests/ChooserInteractionTests.swift b/TFMTests/ChooserInteractionTests.swift index 72de3bf6..0bf02fef 100644 --- a/TFMTests/ChooserInteractionTests.swift +++ b/TFMTests/ChooserInteractionTests.swift @@ -189,6 +189,35 @@ struct ChooserInteractionTests { #expect(workspace.panes.allSatisfy { $0.kind == (target == .sshSession ? .sshSession : .sftpChannel) }) } + @Test(arguments: ["en", "zh-Hans", "zh-Hant"]) + func metricsUsesTheBatchFormAndConfirmsOnlyAfterSelection(_ language: String) async throws { + let fixture = ChooserFixture() + defer { fixture.close() } + fixture.mount(HostChooserView(purpose: .metrics).environment(\.locale, Locale(identifier: language))) + try await fixture.settle() + #expect(!fixture.views.contains { $0 is NSSegmentedControl }) + + _ = try await fixture.quickOpenField() + try fixture.key("\r", code: 36) + try await fixture.settle() + #expect(fixture.model.pendingMetricsHostIDs.isEmpty) + try fixture.key(String(UnicodeScalar(NSDownArrowFunctionKey)!), code: 125) + try fixture.key(" ", code: 49) + try fixture.key(String(UnicodeScalar(NSDownArrowFunctionKey)!), code: 125) + try fixture.key(" ", code: 49) + try await fixture.settle() + #expect(fixture.model.pendingMetricsHostIDs.isEmpty) + #expect(fixture.app.serverMetrics.servers.isEmpty) + let image = try fixture.capture() + Attachment.record(image, named: "metrics-host-picker-\(language).png") + + try fixture.key("\r", code: 36) + try await fixture.settle() + #expect(fixture.model.pendingMetricsHostIDs == fixture.hosts.prefix(2).map(\.id)) + #expect(fixture.model.workspaces.isEmpty) + #expect(fixture.app.serverMetrics.servers.isEmpty) + } + @Test(arguments: ["en", "zh-Hans", "zh-Hant"]) func batchHasOneConnectActionAndCancelCreatesNoSessions(_ language: String) async throws { let fixture = ChooserFixture() diff --git a/TFMTests/ManagedChooserTests.swift b/TFMTests/ManagedChooserTests.swift index 0e3fa7ac..65139039 100644 --- a/TFMTests/ManagedChooserTests.swift +++ b/TFMTests/ManagedChooserTests.swift @@ -77,19 +77,22 @@ struct ManagedChooserTests { func connectAndBatchStartupChooseProtocolsWithExpectedSelectionModes() { #expect(HostChooserPurpose.connect.choosesConnectionProtocol) #expect(HostChooserPurpose.connect.availableConnectionProtocols == [.terminal, .sftp, .serial]) - #expect(!HostChooserPurpose.connect.connectionChooserShowsConfirmation(for: .terminal)) - #expect(!HostChooserPurpose.connect.connectionChooserShowsConfirmation(for: .sftp)) - #expect(HostChooserPurpose.connect.connectionChooserShowsConfirmation(for: .serial)) + #expect(!HostChooserPurpose.connect.formShowsConfirmation(for: .terminal)) + #expect(!HostChooserPurpose.connect.formShowsConfirmation(for: .sftp)) + #expect(HostChooserPurpose.connect.formShowsConfirmation(for: .serial)) #expect(!HostChooserPurpose.connect.allowsMultiple) #expect(HostChooserPurpose.connect.activatesHostImmediately) #expect(HostChooserPurpose.batchStartup.choosesConnectionProtocol) #expect(HostChooserPurpose.batchStartup.availableConnectionProtocols == [.terminal, .sftp]) - #expect(HostChooserPurpose.batchStartup.connectionChooserShowsConfirmation(for: .terminal)) - #expect(HostChooserPurpose.batchStartup.connectionChooserShowsConfirmation(for: .sftp)) + #expect(HostChooserPurpose.batchStartup.formShowsConfirmation(for: .terminal)) + #expect(HostChooserPurpose.batchStartup.formShowsConfirmation(for: .sftp)) #expect(HostChooserPurpose.batchStartup.allowsMultiple) #expect(!HostChooserPurpose.batchStartup.activatesHostImmediately) + #expect(!HostChooserPurpose.metrics.choosesConnectionProtocol) + #expect(HostChooserPurpose.metrics.availableConnectionProtocols.isEmpty) + #expect(HostChooserPurpose.metrics.formShowsConfirmation(for: .terminal)) #expect(HostChooserPurpose.metrics.allowsMultiple) #expect(!HostChooserPurpose.metrics.activatesHostImmediately)