pr suggestions and updated tests

AccessibilityTests/Sources/GeneratedAccessibilityTests.swift

@@ -747,6 +747,10 @@ extension AccessibilityTests {
         try await performAccessibilityAudit(named: "TombstonedAvatarImage_Previews")
     }
 
+    func testToolbarButton() async throws {
+        try await performAccessibilityAudit(named: "ToolbarButton_Previews")
+    }
+
     func testTypingIndicatorView() async throws {
         try await performAccessibilityAudit(named: "TypingIndicatorView_Previews")
     }

ElementX/Sources/FlowCoordinators/SpaceSettingsFlowCoordinator.swift

@@ -1,3 +1,4 @@
+//
 // Copyright 2025 New Vector Ltd.
 //
 // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial

ElementX/Sources/Other/SwiftUI/Views/ToolbarButton.swift

@@ -12,6 +12,7 @@ struct ToolbarButton: View {
     enum Role {
         case cancel
         case done
+        case save
         
         var title: String {
             switch self {
@@ -19,15 +20,29 @@ struct ToolbarButton: View {
                 L10n.actionCancel
             case .done:
                 L10n.actionDone
+            case .save:
+                L10n.actionSave
             }
         }
         
-        var icon: CompoundIcon {
+        @ViewBuilder
+        var icon: some View {
             switch self {
             case .cancel:
                 CompoundIcon(\.close)
-            case .done:
+                    .foregroundStyle(.compound.iconPrimary)
+            case .done, .save:
                 CompoundIcon(\.check)
+                    .foregroundStyle(.compound.iconOnSolidPrimary)
+            }
+        }
+        
+        var tint: Color {
+            switch self {
+            case .cancel:
+                .compound.bgCanvasDefault
+            case .done, .save:
+                .compound.bgAccentRest
             }
         }
     }
@@ -41,8 +56,40 @@ struct ToolbarButton: View {
                 role.icon
                     .accessibilityLabel(role.title)
             }
+            .tint(role.tint)
+            .buttonStyleGlassProminent()
         } else {
             Button(role.title, action: action)
         }
     }
 }
+
+@available(iOS 26, *)
+private extension View {
+    @ViewBuilder
+    func buttonStyleGlassProminent() -> some View {
+        // `.glassProminent` breaks our preview tests so we need to disable it when running tests.
+        // https://github.com/pointfreeco/swift-snapshot-testing/issues/1029#issuecomment-3366942138
+        if ProcessInfo.isRunningTests {
+            self
+        } else {
+            buttonStyle(.glassProminent)
+        }
+    }
+}
+
+struct ToolbarButton_Previews: PreviewProvider, TestablePreview {
+    static var previews: some View {
+        NavigationStack {
+            Color.clear
+                .toolbar {
+                    ToolbarItem(placement: .confirmationAction) {
+                        ToolbarButton(role: .done) { }
+                    }
+                    ToolbarItem(placement: .cancellationAction) {
+                        ToolbarButton(role: .cancel) { }
+                    }
+                }
+        }
+    }
+}

ElementX/Sources/Other/TestablePreview/TestablePreviewsDictionary.swift

@@ -194,6 +194,7 @@ enum TestablePreviewsDictionary {
         "TimelineThreadSummaryView_Previews" : TimelineThreadSummaryView_Previews.self,
         "TimelineView_Previews" : TimelineView_Previews.self,
         "TombstonedAvatarImage_Previews" : TombstonedAvatarImage_Previews.self,
+        "ToolbarButton_Previews" : ToolbarButton_Previews.self,
         "TypingIndicatorView_Previews" : TypingIndicatorView_Previews.self,
         "UnsupportedRoomTimelineView_Previews" : UnsupportedRoomTimelineView_Previews.self,
         "UserDetailsEditScreen_Previews" : UserDetailsEditScreen_Previews.self,

ElementX/Sources/Screens/ManageAuthorizedSpacesScreen/ManageAuthorizedSpacesScreenModels.swift

@@ -14,19 +14,19 @@ enum ManageAuthorizedSpacesScreenViewModelAction {
 
 struct ManageAuthorizedSpacesScreenViewState: BindableState {
     let authorizedSpacesSelection: AuthorizedSpacesSelection
-    var desiredSelectedIDs: Set<String>
+    var selectedIDs: Set<String>
     
     var hasChanges: Bool {
-        authorizedSpacesSelection.currentSelectedIDs != desiredSelectedIDs
+        authorizedSpacesSelection.initialSelectedIDs != selectedIDs
     }
     
     var isDoneButtonDisabled: Bool {
-        desiredSelectedIDs.isEmpty || !hasChanges
+        selectedIDs.isEmpty || !hasChanges
     }
     
     init(authorizedSpacesSelection: AuthorizedSpacesSelection) {
         self.authorizedSpacesSelection = authorizedSpacesSelection
-        desiredSelectedIDs = authorizedSpacesSelection.currentSelectedIDs
+        selectedIDs = authorizedSpacesSelection.initialSelectedIDs
     }
 }
 
@@ -39,6 +39,6 @@ enum ManageAuthorizedSpacesScreenViewAction {
 struct AuthorizedSpacesSelection {
     let joinedParentSpaces: [SpaceRoomProxyProtocol]
     let unknownSpacesIDs: [String]
-    let currentSelectedIDs: Set<String>
+    let initialSelectedIDs: Set<String>
-    let desiredSelectIDs: PassthroughSubject<Set<String>, Never> = .init()
+    let selectedIDs: PassthroughSubject<Set<String>, Never> = .init()
 }

ElementX/Sources/Screens/ManageAuthorizedSpacesScreen/ManageAuthorizedSpacesScreenViewModel.swift

@@ -30,13 +30,13 @@ class ManageAuthorizedSpacesScreenViewModel: ManageAuthorizedSpacesScreenViewMod
         case .cancel:
             actionsSubject.send(.dismiss)
         case .done:
-            state.authorizedSpacesSelection.desiredSelectIDs.send(state.desiredSelectedIDs)
+            state.authorizedSpacesSelection.selectedIDs.send(state.selectedIDs)
             actionsSubject.send(.dismiss)
         case .toggle(let spaceID):
-            if state.desiredSelectedIDs.contains(spaceID) {
+            if state.selectedIDs.contains(spaceID) {
-                state.desiredSelectedIDs.remove(spaceID)
+                state.selectedIDs.remove(spaceID)
             } else {
-                state.desiredSelectedIDs.insert(spaceID)
+                state.selectedIDs.insert(spaceID)
             }
         }
     }

ElementX/Sources/Screens/ManageAuthorizedSpacesScreen/View/ManageAuthorizedSpacesScreen.swift

@@ -14,11 +14,13 @@ struct ManageAuthorizedSpacesScreen: View {
     var body: some View {
         Form {
             header
+            
             if !context.viewState.authorizedSpacesSelection.joinedParentSpaces.isEmpty {
                 joinedParentsSection
             }
+            
             if !context.viewState.authorizedSpacesSelection.unknownSpacesIDs.isEmpty {
-                unkwnownSpacesSection
+                unknownSpacesSection
             }
         }
         .compoundList()
@@ -50,7 +52,7 @@ struct ManageAuthorizedSpacesScreen: View {
                 ListRow(label: .avatar(title: space.name,
                                        description: space.canonicalAlias,
                                        icon: avatar(space: space)),
-                        kind: .multiSelection(isSelected: context.viewState.desiredSelectedIDs.contains(space.id)) {
+                        kind: .multiSelection(isSelected: context.viewState.selectedIDs.contains(space.id)) {
                             context.send(viewAction: .toggle(spaceID: space.id))
                         })
             }
@@ -60,12 +62,12 @@ struct ManageAuthorizedSpacesScreen: View {
         }
     }
     
-    private var unkwnownSpacesSection: some View {
+    private var unknownSpacesSection: some View {
         Section {
             ForEach(context.viewState.authorizedSpacesSelection.unknownSpacesIDs, id: \.self) { id in
                 ListRow(label: .plain(title: L10n.screenManageAuthorizedSpacesUnknownSpace,
                                       description: id),
-                        kind: .multiSelection(isSelected: context.viewState.desiredSelectedIDs.contains(id)) {
+                        kind: .multiSelection(isSelected: context.viewState.selectedIDs.contains(id)) {
                             context.send(viewAction: .toggle(spaceID: id))
                         })
             }
@@ -104,7 +106,7 @@ struct ManageAuthorizedSpacesScreen_Previews: PreviewProvider, TestablePreview {
                                                                                                   unknownSpacesIDs: ["!unknown-space-id-1",
                                                                                                                      "!unknown-space-id-2",
                                                                                                                      "!unknown-space-id-3"],
-                                                                                                  currentSelectedIDs: ["space1",
+                                                                                                  initialSelectedIDs: ["space1",
                                                                                                                        "space3",
                                                                                                                        "!unknown-space-id-2"]),
                                                                  mediaProvider: MediaProviderMock(configuration: .init()))

ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenViewModel.swift

@@ -236,6 +236,7 @@ class SecurityAndPrivacyScreenViewModel: SecurityAndPrivacyScreenViewModelType,
     
     private func handleSelectedSpaceMembersAccess() {
         guard !state.bindings.desiredSettings.accessType.isSpaceUsers else {
+            // If the user is tapping the space members access again we do nothing
             return
         }
         
@@ -257,8 +258,8 @@ class SecurityAndPrivacyScreenViewModel: SecurityAndPrivacyScreenViewModelType,
         let selectedIDs = Set(state.bindings.desiredSettings.accessType.spaceIDs)
         let authorizedSpacesSelection = AuthorizedSpacesSelection(joinedParentSpaces: joinedParentSpaces,
                                                                   unknownSpacesIDs: unknownSpaceIDs,
-                                                                  currentSelectedIDs: selectedIDs)
+                                                                  initialSelectedIDs: selectedIDs)
-        authorizedSpacesSelection.desiredSelectIDs
+        authorizedSpacesSelection.selectedIDs
             .sink { [weak self] desiredSelectedIDs in
                 self?.state.bindings.desiredSettings.accessType = .spaceUsers(spaceIDs: desiredSelectedIDs.sorted())
             }

PreviewTests/Sources/GeneratedPreviewTests.swift

@@ -1121,6 +1121,12 @@ extension PreviewTests {
         }
     }
 
+    func testToolbarButton() async throws {
+        for (index, preview) in ToolbarButton_Previews._allPreviews.enumerated() {
+            try await assertSnapshots(matching: preview, step: index)
+        }
+    }
+
     func testTypingIndicatorView() async throws {
         for (index, preview) in TypingIndicatorView_Previews._allPreviews.enumerated() {
             try await assertSnapshots(matching: preview, step: index)

PreviewTests/Sources/__Snapshots__/PreviewTests/manageAuthorizedSpacesScreen.iPad-en-GB-0.png

@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:55244034738390a41d8d0c68510265a3b7a82f2b6464c2960ebc149c3e495635
+oid sha256:41b3fe504786e50cc23f43b4c5b3695af32fdbcc68981f2809b8c062ce7c12ec
-size 180070
+size 180094

PreviewTests/Sources/__Snapshots__/PreviewTests/manageAuthorizedSpacesScreen.iPad-pseudo-0.png

@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:9d4148bdd30d52750164b720a6e8b957cf1912842237729e0b3ef03a5d3d8f75
+oid sha256:40c3dce4e87b7021d3d50841071abe375c8e917abf755a9af3d2de4130ea029a
-size 205274
+size 205332

UnitTests/Sources/SecurityAndPrivacyScreenViewModelTests.swift

@@ -104,21 +104,21 @@ class SecurityAndPrivacyScreenViewModelTests: XCTestCase {
             return
         }
         
-        var desiredSpaceIDs: PassthroughSubject<Set<String>, Never>!
+        var selectedIDs: PassthroughSubject<Set<String>, Never>!
         let deferredAction = deferFulfillment(viewModel.actionsPublisher) { action in
             switch action {
             case .displayManageAuthorizedSpacesScreen(let authorizedSpacesSelection):
-                defer { desiredSpaceIDs = authorizedSpacesSelection.desiredSelectIDs }
+                defer { selectedIDs = authorizedSpacesSelection.selectedIDs }
                 return authorizedSpacesSelection.joinedParentSpaces.map(\.id) == spaces.map(\.id) &&
                     authorizedSpacesSelection.unknownSpacesIDs.isEmpty &&
-                    authorizedSpacesSelection.currentSelectedIDs.isEmpty
+                    authorizedSpacesSelection.initialSelectedIDs.isEmpty
             default:
                 return false
             }
         }
         context.send(viewAction: .selectedSpaceMembersAccess)
         try await deferredAction.fulfill()
-        desiredSpaceIDs.send([spaces[0].id])
+        selectedIDs.send([spaces[0].id])
         XCTAssertEqual(context.desiredSettings.accessType, .spaceUsers(spaceIDs: [spaces[0].id]))
         XCTAssertNotNil(context.viewState.accessSectionFooter)
         XCTAssertFalse(context.viewState.isSaveDisabled)
@@ -150,21 +150,22 @@ class SecurityAndPrivacyScreenViewModelTests: XCTestCase {
             return
         }
         
-        var desiredSpaceIDs: PassthroughSubject<Set<String>, Never>!
+        var selectedIDs: PassthroughSubject<Set<String>, Never>!
         let deferredAction = deferFulfillment(viewModel.actionsPublisher) { action in
             switch action {
             case .displayManageAuthorizedSpacesScreen(let authorizedSpacesSelection):
-                defer { desiredSpaceIDs = authorizedSpacesSelection.desiredSelectIDs }
+                // We need the
+                defer { selectedIDs = authorizedSpacesSelection.selectedIDs }
                 return authorizedSpacesSelection.joinedParentSpaces.map(\.id) == spaces.map(\.id) &&
                     authorizedSpacesSelection.unknownSpacesIDs == ["unknownSpaceID"] &&
-                    authorizedSpacesSelection.currentSelectedIDs == ["unknownSpaceID"]
+                    authorizedSpacesSelection.initialSelectedIDs == ["unknownSpaceID"]
             default:
                 return false
             }
         }
         context.send(viewAction: .manageSpaces)
         try await deferredAction.fulfill()
-        desiredSpaceIDs.send([spaces[0].id, "unknownSpaceID"])
+        selectedIDs.send([spaces[0].id, "unknownSpaceID"])
         XCTAssertEqual(context.desiredSettings.accessType, .spaceUsers(spaceIDs: [spaces[0].id, "unknownSpaceID"]))
         XCTAssertNotNil(context.viewState.accessSectionFooter)
         XCTAssertFalse(context.viewState.isSaveDisabled)