Fix multiple small issues in ConnectionView

mullvad · Jan 9, 2025 · 9925f53 · 9925f53
1 parent 7692f33
commit 9925f53
Show file tree

Hide file tree

Showing 19 changed files with 165 additions and 199 deletions.
diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj
@@ -621,6 +621,7 @@
 		7AA1309F2D007B2500640DF9 /* VisualEffectView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AA1309E2D007B2500640DF9 /* VisualEffectView.swift */; };
 		7AA130A12D01B1E200640DF9 /* SplitMainButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AA130A02D01B1E200640DF9 /* SplitMainButton.swift */; };
 		7AA513862BC91C6B00D081A4 /* LogRotationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AA513852BC91C6B00D081A4 /* LogRotationTests.swift */; };
+		7AA636382D2D3BB0009B2C89 /* View+Conditionals.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AA636372D2D3BAC009B2C89 /* View+Conditionals.swift */; };
 		7AA7046A2C8EFE2B0045699D /* StoredRelays.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AA704682C8EFE050045699D /* StoredRelays.swift */; };
 		7AB2B6702BA1EB8C00B03E3B /* ListCustomListViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AB2B66E2BA1EB8C00B03E3B /* ListCustomListViewController.swift */; };
 		7AB2B6712BA1EB8C00B03E3B /* ListCustomListCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AB2B66F2BA1EB8C00B03E3B /* ListCustomListCoordinator.swift */; };
@@ -661,7 +662,6 @@
 		7AF36A9A2CA2964200E1D497 /* AnyIPAddressTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AF36A992CA2964000E1D497 /* AnyIPAddressTests.swift */; };
 		7AF6E5F02A95051E00F2679D /* RouterBlockDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AF6E5EF2A95051E00F2679D /* RouterBlockDelegate.swift */; };
 		7AF84F462D12C5B000C72690 /* SelectedRelaysStub+Stubs.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AF84F452D12C59F00C72690 /* SelectedRelaysStub+Stubs.swift */; };
-		7AF84F482D12C9D400C72690 /* ConnectionViewPreview.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AF84F472D12C9CF00C72690 /* ConnectionViewPreview.swift */; };
 		7AF9BE882A30C62100DBFEDB /* SelectableSettingsCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A1A264A2A29D65E00B978AA /* SelectableSettingsCell.swift */; };
 		7AF9BE8C2A321D1F00DBFEDB /* RelayFilter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AF9BE8A2A321BEF00DBFEDB /* RelayFilter.swift */; };
 		7AF9BE8E2A331C7B00DBFEDB /* RelayFilterViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AF9BE8D2A331C7B00DBFEDB /* RelayFilterViewModel.swift */; };
@@ -2007,6 +2007,7 @@
 		7AA1309E2D007B2500640DF9 /* VisualEffectView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VisualEffectView.swift; sourceTree = "<group>"; };
 		7AA130A02D01B1E200640DF9 /* SplitMainButton.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SplitMainButton.swift; sourceTree = "<group>"; };
 		7AA513852BC91C6B00D081A4 /* LogRotationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LogRotationTests.swift; sourceTree = "<group>"; };
+		7AA636372D2D3BAC009B2C89 /* View+Conditionals.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "View+Conditionals.swift"; sourceTree = "<group>"; };
 		7AA704682C8EFE050045699D /* StoredRelays.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoredRelays.swift; sourceTree = "<group>"; };
 		7AB2B66E2BA1EB8C00B03E3B /* ListCustomListViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ListCustomListViewController.swift; sourceTree = "<group>"; };
 		7AB2B66F2BA1EB8C00B03E3B /* ListCustomListCoordinator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ListCustomListCoordinator.swift; sourceTree = "<group>"; };
@@ -2040,7 +2041,6 @@
 		7AF36A992CA2964000E1D497 /* AnyIPAddressTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AnyIPAddressTests.swift; sourceTree = "<group>"; };
 		7AF6E5EF2A95051E00F2679D /* RouterBlockDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RouterBlockDelegate.swift; sourceTree = "<group>"; };
 		7AF84F452D12C59F00C72690 /* SelectedRelaysStub+Stubs.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "SelectedRelaysStub+Stubs.swift"; sourceTree = "<group>"; };
-		7AF84F472D12C9CF00C72690 /* ConnectionViewPreview.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConnectionViewPreview.swift; sourceTree = "<group>"; };
 		7AF9BE8A2A321BEF00DBFEDB /* RelayFilter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RelayFilter.swift; sourceTree = "<group>"; };
 		7AF9BE8D2A331C7B00DBFEDB /* RelayFilterViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RelayFilterViewModel.swift; sourceTree = "<group>"; };
 		7AF9BE8F2A39F26000DBFEDB /* Collection+Sorting.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Collection+Sorting.swift"; sourceTree = "<group>"; };
@@ -2705,13 +2705,14 @@
 		4419AA862D28264D001B13C9 /* ConnectionView */ = {
 			isa = PBXGroup;
 			children = (
-				449E9A6B2D2839FD00F8574A /* Preview */,
+				F0ADF1CF2D01B50B00299F09 /* ChipView */,
+				449E9A6E2D283C7400F8574A /* ButtonPanel.swift */,
 				7AA130982CFF365A00640DF9 /* ConnectionView.swift */,
+				449E9A6C2D283A2500F8574A /* ConnectionViewComponentPreview.swift */,
 				7A0EAEA32D06DF8200D3EB8B /* ConnectionViewViewModel.swift */,
 				4419AA882D282687001B13C9 /* DetailsContainer.swift */,
 				4419AA8A2D2826E5001B13C9 /* DetailsView.swift */,
 				4419AA8D2D2828A4001B13C9 /* HeaderView.swift */,
-				449E9A6E2D283C7400F8574A /* ButtonPanel.swift */,
 			);
 			path = ConnectionView;
 			sourceTree = "<group>";
@@ -2756,15 +2757,6 @@
 			path = MullvadSettings;
 			sourceTree = "<group>";
 		};
-		449E9A6B2D2839FD00F8574A /* Preview */ = {
-			isa = PBXGroup;
-			children = (
-				7AF84F472D12C9CF00C72690 /* ConnectionViewPreview.swift */,
-				449E9A6C2D283A2500F8574A /* ConnectionViewComponentPreview.swift */,
-			);
-			path = Preview;
-			sourceTree = "<group>";
-		};
 		449EBA242B975B7C00DFA4EB /* Protocols */ = {
 			isa = PBXGroup;
 			children = (
@@ -3180,6 +3172,7 @@
 				7A21DACE2A30AA3700A787A9 /* UITextField+Appearance.swift */,
 				5878F4FF29CDA742003D4BE2 /* UIView+AutoLayoutBuilder.swift */,
 				7A516C2D2B6D357500BBD33D /* URL+Scoping.swift */,
+				7AA636372D2D3BAC009B2C89 /* View+Conditionals.swift */,
 				7A0EAE9D2D01BCBF00D3EB8B /* View+Size.swift */,
 				7A8A18FA2CE4B66C000BCB5B /* View+TapAreaSize.swift */,
 			);
@@ -4117,7 +4110,6 @@
 			isa = PBXGroup;
 			children = (
 				4419AA862D28264D001B13C9 /* ConnectionView */,
-				F0ADF1CF2D01B50B00299F09 /* ChipView */,
 				7AFBE3862D084C96002335FC /* ActivityIndicator.swift */,
 				F0B4957B2D03154200CFEC2A /* FeatureIndicatorsView.swift */,
 				F0ADF1D22D01B6B400299F09 /* FeatureIndicatorsViewModel.swift */,
@@ -5954,6 +5946,7 @@
 				F01DAE332C2B032A00521E46 /* RelaySelection.swift in Sources */,
 				58B993B12608A34500BA7811 /* LoginContentView.swift in Sources */,
 				7A516C2E2B6D357500BBD33D /* URL+Scoping.swift in Sources */,
+				7AA636382D2D3BB0009B2C89 /* View+Conditionals.swift in Sources */,
 				5878A27529093A310096FC88 /* StorePaymentEvent.swift in Sources */,
 				7A7AD28D29DC677800480EF1 /* FirstTimeLaunch.swift in Sources */,
 				F062000C2CB7EB5D002E6DB9 /* UIImage+Helpers.swift in Sources */,
@@ -5973,7 +5966,6 @@
 				7A27E3CB2CAE861D0088BCFF /* SettingsViewModel.swift in Sources */,
 				588527B2276B3F0700BAA373 /* LoadTunnelConfigurationOperation.swift in Sources */,
 				7A9F29392CABFAFC005F2089 /* InfoHeaderView.swift in Sources */,
-				7AF84F482D12C9D400C72690 /* ConnectionViewPreview.swift in Sources */,
 				58DFF7D22B0256A300F864E0 /* MarkdownStylingOptions.swift in Sources */,
 				5867770E29096984006F721F /* OutOfTimeInteractor.swift in Sources */,
 				F03580252A13842C00E5DAFD /* IncreasedHitButton.swift in Sources */,

diff --git a/ios/MullvadVPN/Extensions/View+Conditionals.swift b/ios/MullvadVPN/Extensions/View+Conditionals.swift
@@ -0,0 +1,41 @@
+//
+//  View+Conditionals.swift
+//  MullvadVPN
+//
+//  Created by Jon Petersson on 2025-01-07.
+//  Copyright © 2025 Mullvad VPN AB. All rights reserved.
+//
+
+import SwiftUI
+
+extension View {
+    @ViewBuilder func `if`<Content: View>(
+        _ conditional: Bool,
+        @ViewBuilder _ content: (Self) -> Content
+    ) -> some View {
+        if conditional {
+            content(self)
+        } else {
+            self
+        }
+    }
+
+    @ViewBuilder func ifLet<Content: View, T>(
+        _ conditional: T?,
+        @ViewBuilder _ content: (Self, _ value: T) -> Content
+    ) -> some View {
+        if let value = conditional {
+            content(self, value)
+        } else {
+            self
+        }
+    }
+
+    @ViewBuilder func showIf(_ conditional: Bool) -> some View {
+        if conditional {
+            self
+        } else {
+            EmptyView()
+        }
+    }
+}
diff --git a/ios/MullvadVPN/Extensions/View+TapAreaSize.swift b/ios/MullvadVPN/Extensions/View+TapAreaSize.swift
@@ -28,6 +28,5 @@ private struct TappablePadding: ViewModifier {
                 height: max(actualViewSize.height, tappableViewSize.height)
             )
             .contentShape(Rectangle())
-            .frame(width: actualViewSize.width, height: actualViewSize.height)
     }
 }
diff --git a/ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView/ButtonPanel.swift b/ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView/ButtonPanel.swift
@@ -12,7 +12,7 @@ extension ConnectionView {
     internal struct ButtonPanel: View {
         typealias Action = (ConnectionViewViewModel.TunnelAction) -> Void
 
-        @StateObject var viewModel: ConnectionViewViewModel
+        @ObservedObject var viewModel: ConnectionViewViewModel
         var action: Action?
 
         var body: some View {
@@ -32,10 +32,10 @@ extension ConnectionView {
                     text: viewModel.localizedTitleForSelectLocationButton,
                     image: .iconReload,
                     style: .default,
+                    accessibilityId: .selectLocationButton,
                     primaryAction: { action?(.selectLocation) },
                     secondaryAction: { action?(.reconnect) }
                 )
-                .accessibilityIdentifier(AccessibilityIdentifier.selectLocationButton.asString)
             case .disconnecting, .pendingReconnect, .disconnected:
                 MainButton(
                     text: viewModel.localizedTitleForSelectLocationButton,

diff --git a/...dicators/ChipView/ChipContainerView.swift → ...tionView/ChipView/ChipContainerView.swift b/...dicators/ChipView/ChipContainerView.swift → ...tionView/ChipView/ChipContainerView.swift
@@ -30,15 +30,14 @@ struct ChipContainerView<ViewModel>: View where ViewModel: ChipViewModelProtocol
                     createChipViews(chips: chipsToAdd, containerWidth: containerWidth)
                 }
 
-                if showMoreButton {
-                    Text(LocalizedStringKey("\(viewModel.chips.count - chipsToAdd.count) more..."))
-                        .font(.subheadline)
-                        .lineLimit(1)
-                        .foregroundStyle(UIColor.primaryTextColor.color)
-                        .onTapGesture {
-                            isExpanded.toggle()
-                        }
-                }
+                Text(LocalizedStringKey("\(viewModel.chips.count - chipsToAdd.count) more..."))
+                    .font(.subheadline)
+                    .lineLimit(1)
+                    .foregroundStyle(UIColor.primaryTextColor.color)
+                    .onTapGesture {
+                        isExpanded.toggle()
+                    }
+                    .showIf(showMoreButton)
 
                 Spacer()
             }

diff --git a/...tureIndicators/ChipView/ChipFeature.swift → ...ConnectionView/ChipView/ChipFeature.swift b/...tureIndicators/ChipView/ChipFeature.swift → ...ConnectionView/ChipView/ChipFeature.swift
@@ -8,6 +8,10 @@
 import MullvadSettings
 import SwiftUI
 
+// Opting to use NSLocalizedString instead of LocalizedStringKey here in order
+// to be able to fetch the string value at a later point (eg. in ChipViewModelProtocol,
+// when calculating the text widths of the chips).
+
 protocol ChipFeature {
     var isEnabled: Bool { get }
     var name: String { get }

diff --git a/...eatureIndicators/ChipView/ChipModel.swift → ...s/ConnectionView/ChipView/ChipModel.swift b/...eatureIndicators/ChipView/ChipModel.swift → ...s/ConnectionView/ChipView/ChipModel.swift
diff --git a/...FeatureIndicators/ChipView/ChipView.swift → ...rs/ConnectionView/ChipView/ChipView.swift b/...FeatureIndicators/ChipView/ChipView.swift → ...rs/ConnectionView/ChipView/ChipView.swift
@@ -11,7 +11,7 @@ import SwiftUI
 struct ChipView: View {
     let item: ChipModel
     var body: some View {
-        Text(LocalizedStringKey(item.name))
+        Text(item.name)
             .font(.subheadline)
             .lineLimit(1)
             .foregroundStyle(UIColor.primaryTextColor.color)

diff --git a/...tors/ChipView/ChipViewModelProtocol.swift → ...View/ChipView/ChipViewModelProtocol.swift b/...tors/ChipView/ChipViewModelProtocol.swift → ...View/ChipView/ChipViewModelProtocol.swift
@@ -13,12 +13,19 @@ protocol ChipViewModelProtocol: ObservableObject {
 }
 
 extension ChipViewModelProtocol {
-    func chipsToAdd(forContainerWidth containerWidth: CGFloat) -> (chips: [ChipModel], chipsWillOverflow: Bool) {
+    func chipsToAdd(forContainerWidth containerWidth: CGFloat) -> (chips: [ChipModel], isOverflowing: Bool) {
         var chipsToAdd = [ChipModel]()
-        var chipsWillOverflow = false
+        var isOverflowing = false
 
-        let moreTextWidth = "\(chips.count) more..."
-            .width(using: .preferredFont(forTextStyle: .subheadline)) + 4 // Some extra to be safe.
+        let moreTextWidth = String(
+            format: NSLocalizedString(
+                "CONNECTION_VIEW_CHIPS_MORE",
+                tableName: "ConnectionView",
+                value: "@d more...",
+                comment: ""
+            ), arguments: [chips.count]
+        )
+        .width(using: .preferredFont(forTextStyle: .subheadline)) + 4 // Some extra to be safe.
         var totalChipsWidth: CGFloat = 0
 
         for (index, chip) in chips.enumerated() {
@@ -33,20 +40,15 @@ extension ChipViewModelProtocol {
             let chipWillFitWithMoreText = (totalChipsWidth + moreTextWidth) <= containerWidth
             let chipWillFit = totalChipsWidth <= containerWidth
 
-            if chipWillFitWithMoreText {
-                // If a chip can fit together with the "more" text, add it.
-                chipsToAdd.append(chip)
-                chipsWillOverflow = !isLastChip
-            } else if chipWillFit && isLastChip {
-                // If a chip can fit and it's the last one, add it.
-                chipsToAdd.append(chip)
-                chipsWillOverflow = false
-            } else {
+            guard (chipWillFit && isLastChip) || chipWillFitWithMoreText else {
+                isOverflowing = true
                 break
             }
+
+            chipsToAdd.append(chip)
         }
 
-        return (chipsToAdd, chipsWillOverflow)
+        return (chipsToAdd, isOverflowing)
     }
 }
 

diff --git a/ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView/ConnectionView.swift b/ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView/ConnectionView.swift
@@ -9,8 +9,8 @@
 import SwiftUI
 
 struct ConnectionView: View {
-    @StateObject var connectionViewModel: ConnectionViewViewModel
-    @StateObject var indicatorsViewModel: FeatureIndicatorsViewModel
+    @ObservedObject var connectionViewModel: ConnectionViewViewModel
+    @ObservedObject var indicatorsViewModel: FeatureIndicatorsViewModel
 
     @State private(set) var isExpanded = false
 
@@ -19,53 +19,71 @@ struct ConnectionView: View {
 
     var body: some View {
         Spacer()
+            .accessibilityIdentifier(AccessibilityIdentifier.connectionView.asString)
+
         VStack(spacing: 22) {
-            if connectionViewModel.showsActivityIndicator {
-                CustomProgressView(style: .large)
-            }
+            CustomProgressView(style: .large)
+                .showIf(connectionViewModel.showsActivityIndicator)
 
             ZStack {
                 BlurView(style: .dark)
 
-                VStack(alignment: .leading, spacing: 16) {
+                VStack(alignment: .leading, spacing: 0) {
                     HeaderView(viewModel: connectionViewModel, isExpanded: $isExpanded)
+                        .padding(.bottom, headerViewBottomPadding)
 
-                    if connectionViewModel.showConnectionDetails {
-                        DetailsContainer(
-                            viewModel: connectionViewModel,
-                            indicatorsViewModel: indicatorsViewModel,
-                            isExpanded: $isExpanded
-                        )
-                    }
+                    DetailsContainer(
+                        connectionViewModel: connectionViewModel,
+                        indicatorsViewModel: indicatorsViewModel,
+                        isExpanded: $isExpanded
+                    )
+                    .showIf(connectionViewModel.showConnectionDetails)
 
                     ButtonPanel(viewModel: connectionViewModel, action: action)
+                        .padding(.top, 16)
                 }
                 .padding(16)
             }
             .cornerRadius(12)
             .padding(16)
         }
-        .padding(.bottom, 8) // Adding some spacing so as not to overlap with the map legal link.
-        .accessibilityIdentifier(AccessibilityIdentifier.connectionView.asString)
+        .padding(.bottom, 8) // Some spacing to avoid overlap with the map legal link.
         .onChange(of: isExpanded) { _ in
             onContentUpdate?()
         }
         .onReceive(connectionViewModel.combinedState) { _, _ in
-            onContentUpdate?()
-
+            // Only update expanded state when connections details should be hidden.
+            // This will contract the view on eg. disconnect, but leave it as-is on
+            // eg. connect.
             if !connectionViewModel.showConnectionDetails {
-//                withAnimation {
                 isExpanded = false
-//                }
+                return
             }
+
+            onContentUpdate?()
         }
     }
 }
 
+extension ConnectionView {
+    var headerViewBottomPadding: CGFloat {
+        let showConnectionDetails = connectionViewModel.showConnectionDetails
+        let hasIndicators = !indicatorsViewModel.chips.isEmpty
+
+        return isExpanded
+            ? 16
+            : hasIndicators && showConnectionDetails ? 16 : 0
+    }
+}
+
 #Preview("ConnectionView (Indicators)") {
-    ConnectionViewPreview(configuration: .normal).make()
+    ConnectionViewComponentPreview(showIndicators: true, isExpanded: true) { indicatorModel, viewModel, _ in
+        ConnectionView(connectionViewModel: viewModel, indicatorsViewModel: indicatorModel)
+    }
 }
 
 #Preview("ConnectionView (No indicators)") {
-    ConnectionViewPreview(configuration: .normalNoIndicators).make()
+    ConnectionViewComponentPreview(showIndicators: false, isExpanded: true) { indicatorModel, viewModel, _ in
+        ConnectionView(connectionViewModel: viewModel, indicatorsViewModel: indicatorModel)
+    }
 }
diff --git a/...view/ConnectionViewComponentPreview.swift → ...View/ConnectionViewComponentPreview.swift b/...view/ConnectionViewComponentPreview.swift → ...View/ConnectionViewComponentPreview.swift
@@ -55,15 +55,14 @@ struct ConnectionViewComponentPreview<Content: View>: View {
     }
 
     var body: some View {
-        VStack {
-            content(
-                FeatureIndicatorsViewModel(
-                    tunnelSettings: tunnelSettings,
-                    ipOverrides: []
-                ),
-                viewModel,
-                $isExpanded
-            )
-        }.background(UIColor.secondaryColor.color)
+        content(
+            FeatureIndicatorsViewModel(
+                tunnelSettings: tunnelSettings,
+                ipOverrides: []
+            ),
+            viewModel,
+            $isExpanded
+        )
+        .background(UIColor.secondaryColor.color)
     }
 }