From f523a9674508d197102a547cd08b0c02241bd8b4 Mon Sep 17 00:00:00 2001 From: Jon Petersson Date: Wed, 4 Dec 2024 14:31:06 +0100 Subject: [PATCH] Add trailing status indicators for daita and multihop cells --- .../Settings/SettingsCellFactory.swift | 18 +++++++- .../Settings/SettingsDataSource.swift | 14 ++++--- .../Settings/SettingsInteractor.swift | 41 ++++--------------- .../Settings/SettingsViewController.swift | 4 ++ .../Settings/SettingsViewModel.swift | 6 +-- .../SwiftUI components/SingleChoiceList.swift | 2 +- .../Pages/SettingsPage.swift | 40 ++++++++++++++++++ ios/MullvadVPNUITests/RelayTests.swift | 4 ++ 8 files changed, 84 insertions(+), 45 deletions(-) diff --git a/ios/MullvadVPN/View controllers/Settings/SettingsCellFactory.swift b/ios/MullvadVPN/View controllers/Settings/SettingsCellFactory.swift index 763c3d20ad59..7a00c9336d0b 100644 --- a/ios/MullvadVPN/View controllers/Settings/SettingsCellFactory.swift +++ b/ios/MullvadVPN/View controllers/Settings/SettingsCellFactory.swift @@ -111,7 +111,14 @@ final class SettingsCellFactory: CellFactoryProtocol { value: "DAITA", comment: "" ) - cell.detailTitleLabel.text = nil + + cell.detailTitleLabel.text = NSLocalizedString( + "DAITA_CELL_DETAIL_LABEL", + tableName: "Settings", + value: viewModel.daitaSettings.daitaState.isEnabled ? "On" : "Off", + comment: "" + ) + cell.accessibilityIdentifier = item.accessibilityIdentifier cell.disclosureType = .chevron @@ -124,7 +131,14 @@ final class SettingsCellFactory: CellFactoryProtocol { value: "Multihop", comment: "" ) - cell.detailTitleLabel.text = nil + + cell.detailTitleLabel.text = NSLocalizedString( + "MULTIHOP_CELL_DETAIL_LABEL", + tableName: "Settings", + value: viewModel.multihopState.isEnabled ? "On" : "Off", + comment: "" + ) + cell.accessibilityIdentifier = item.accessibilityIdentifier cell.disclosureType = .chevron } diff --git a/ios/MullvadVPN/View controllers/Settings/SettingsDataSource.swift b/ios/MullvadVPN/View controllers/Settings/SettingsDataSource.swift index 795c4732b045..7d46b67750be 100644 --- a/ios/MullvadVPN/View controllers/Settings/SettingsDataSource.swift +++ b/ios/MullvadVPN/View controllers/Settings/SettingsDataSource.swift @@ -104,6 +104,14 @@ final class SettingsDataSource: UITableViewDiffableDataSource Bool { @@ -171,10 +179,4 @@ extension SettingsDataSource: SettingsCellEventHandler { func showInfo(for button: SettingsInfoButtonItem) { delegate?.showInfo(for: button) } - - private func reloadItem(_ item: Item) { - var snapshot = snapshot() - snapshot.reloadItems([item]) - apply(snapshot, animatingDifferences: false) - } } diff --git a/ios/MullvadVPN/View controllers/Settings/SettingsInteractor.swift b/ios/MullvadVPN/View controllers/Settings/SettingsInteractor.swift index eb21e716ffca..eb0f428afd85 100644 --- a/ios/MullvadVPN/View controllers/Settings/SettingsInteractor.swift +++ b/ios/MullvadVPN/View controllers/Settings/SettingsInteractor.swift @@ -15,6 +15,7 @@ final class SettingsInteractor { private var tunnelObserver: TunnelObserver? var didUpdateDeviceState: ((DeviceState) -> Void)? + var didUpdateTunnelSettings: ((LatestTunnelSettings) -> Void)? var tunnelSettings: LatestTunnelSettings { tunnelManager.settings @@ -28,41 +29,17 @@ final class SettingsInteractor { self.tunnelManager = tunnelManager let tunnelObserver = - TunnelBlockObserver(didUpdateDeviceState: { [weak self] _, deviceState, _ in - self?.didUpdateDeviceState?(deviceState) - }) + TunnelBlockObserver( + didUpdateDeviceState: { [weak self] _, deviceState, _ in + self?.didUpdateDeviceState?(deviceState) + }, + didUpdateTunnelSettings: { [weak self] _, settings in + self?.didUpdateTunnelSettings?(settings) + } + ) tunnelManager.addObserver(tunnelObserver) self.tunnelObserver = tunnelObserver } - - func updateDAITASettings(_ settings: DAITASettings) { - tunnelManager.updateSettings([.daita(settings)]) - } - - func evaluateDaitaSettingsCompatibility(_ settings: DAITASettings) -> DAITASettingsCompatibilityError? { - guard settings.daitaState.isEnabled else { return nil } - - var tunnelSettings = tunnelSettings - tunnelSettings.daita = settings - - var compatibilityError: DAITASettingsCompatibilityError? - - do { - _ = try tunnelManager.selectRelays(tunnelSettings: tunnelSettings) - } catch let error as NoRelaysSatisfyingConstraintsError where error.reason == .noDaitaRelaysFound { - // Return error if no relays could be selected due to DAITA constraints. - compatibilityError = tunnelSettings.tunnelMultihopState.isEnabled ? .multihop : .singlehop - } catch _ as NoRelaysSatisfyingConstraintsError { - // Even if the constraints error is not DAITA specific, if both DAITA and Direct only are enabled, - // we should return a DAITA related error since the current settings would have resulted in the - // relay selector not being able to select a DAITA relay anyway. - if settings.isDirectOnly { - compatibilityError = tunnelSettings.tunnelMultihopState.isEnabled ? .multihop : .singlehop - } - } catch {} - - return compatibilityError - } } diff --git a/ios/MullvadVPN/View controllers/Settings/SettingsViewController.swift b/ios/MullvadVPN/View controllers/Settings/SettingsViewController.swift index ffe9344d163d..351713b100f1 100644 --- a/ios/MullvadVPN/View controllers/Settings/SettingsViewController.swift +++ b/ios/MullvadVPN/View controllers/Settings/SettingsViewController.swift @@ -69,6 +69,10 @@ class SettingsViewController: UITableViewController { dataSource = SettingsDataSource(tableView: tableView, interactor: interactor) dataSource?.delegate = self + + interactor.didUpdateTunnelSettings = { [weak self] newSettings in + self?.dataSource?.reload(from: newSettings) + } } } diff --git a/ios/MullvadVPN/View controllers/Settings/SettingsViewModel.swift b/ios/MullvadVPN/View controllers/Settings/SettingsViewModel.swift index 1e67a4c28500..441e9f4c97d4 100644 --- a/ios/MullvadVPN/View controllers/Settings/SettingsViewModel.swift +++ b/ios/MullvadVPN/View controllers/Settings/SettingsViewModel.swift @@ -10,12 +10,10 @@ import MullvadSettings struct SettingsViewModel { private(set) var daitaSettings: DAITASettings - - mutating func setDAITASettings(_ newSettings: DAITASettings) { - daitaSettings = newSettings - } + private(set) var multihopState: MultihopState init(from tunnelSettings: LatestTunnelSettings = LatestTunnelSettings()) { daitaSettings = tunnelSettings.daita + multihopState = tunnelSettings.tunnelMultihopState } } diff --git a/ios/MullvadVPN/View controllers/Settings/SwiftUI components/SingleChoiceList.swift b/ios/MullvadVPN/View controllers/Settings/SwiftUI components/SingleChoiceList.swift index a01b51b54caf..4f9ce77f263d 100644 --- a/ios/MullvadVPN/View controllers/Settings/SwiftUI components/SingleChoiceList.swift +++ b/ios/MullvadVPN/View controllers/Settings/SwiftUI components/SingleChoiceList.swift @@ -411,4 +411,4 @@ struct SingleChoiceList: View where Value: Equatable { ) } } -} +} // swiftlint:disable:this file_length diff --git a/ios/MullvadVPNUITests/Pages/SettingsPage.swift b/ios/MullvadVPNUITests/Pages/SettingsPage.swift index bf24d767be73..88939bb64f41 100644 --- a/ios/MullvadVPNUITests/Pages/SettingsPage.swift +++ b/ios/MullvadVPNUITests/Pages/SettingsPage.swift @@ -40,6 +40,26 @@ class SettingsPage: Page { return self } + @discardableResult func verifyDAITAOn() -> Self { + let textElement = app.tables[AccessibilityIdentifier.settingsTableView] + .cells[AccessibilityIdentifier.daitaCell] + .staticTexts["On"] + + XCTAssertTrue(textElement.exists) + + return self + } + + @discardableResult func verifyDAITAOff() -> Self { + let textElement = app.tables[AccessibilityIdentifier.settingsTableView] + .cells[AccessibilityIdentifier.daitaCell] + .staticTexts["Off"] + + XCTAssertTrue(textElement.exists) + + return self + } + @discardableResult func tapMultihopCell() -> Self { app.tables[AccessibilityIdentifier.settingsTableView] .cells[AccessibilityIdentifier.multihopCell] @@ -48,6 +68,26 @@ class SettingsPage: Page { return self } + @discardableResult func verifyMultihopOn() -> Self { + let textElement = app.tables[AccessibilityIdentifier.settingsTableView] + .cells[AccessibilityIdentifier.multihopCell] + .staticTexts["On"] + + XCTAssertTrue(textElement.exists) + + return self + } + + @discardableResult func verifyMultihopOff() -> Self { + let textElement = app.tables[AccessibilityIdentifier.settingsTableView] + .cells[AccessibilityIdentifier.multihopCell] + .staticTexts["Off"] + + XCTAssertTrue(textElement.exists) + + return self + } + @discardableResult func tapVPNSettingsCell() -> Self { app.tables[AccessibilityIdentifier.settingsTableView] .cells[AccessibilityIdentifier.vpnSettingsCell] diff --git a/ios/MullvadVPNUITests/RelayTests.swift b/ios/MullvadVPNUITests/RelayTests.swift index b5951eb2f558..9bf9c790421f 100644 --- a/ios/MullvadVPNUITests/RelayTests.swift +++ b/ios/MullvadVPNUITests/RelayTests.swift @@ -303,6 +303,7 @@ class RelayTests: LoggedInWithTimeUITestCase { .tapSettingsButton() SettingsPage(app) + .verifyDAITAOff() .tapDAITACell() DAITAPage(app) @@ -313,6 +314,7 @@ class RelayTests: LoggedInWithTimeUITestCase { .tapBackButton() SettingsPage(app) + .verifyDAITAOn() .tapDoneButton() TunnelControlPage(app) @@ -343,6 +345,7 @@ class RelayTests: LoggedInWithTimeUITestCase { .tapSettingsButton() SettingsPage(app) + .verifyMultihopOff() .tapMultihopCell() MultihopPage(app) @@ -351,6 +354,7 @@ class RelayTests: LoggedInWithTimeUITestCase { .tapBackButton() SettingsPage(app) + .verifyMultihopOn() .tapDoneButton() TunnelControlPage(app)