diff --git a/AccessibilityTests/Sources/GeneratedAccessibilityTests.swift b/AccessibilityTests/Sources/GeneratedAccessibilityTests.swift index 2825895e6..ced163874 100644 --- a/AccessibilityTests/Sources/GeneratedAccessibilityTests.swift +++ b/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") } diff --git a/ElementX/Sources/FlowCoordinators/SpaceSettingsFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/SpaceSettingsFlowCoordinator.swift index c89d086d6..cfa316526 100644 --- a/ElementX/Sources/FlowCoordinators/SpaceSettingsFlowCoordinator.swift +++ b/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 diff --git a/ElementX/Sources/Other/SwiftUI/Views/ToolbarButton.swift b/ElementX/Sources/Other/SwiftUI/Views/ToolbarButton.swift index 6dfee0eb6..dd733da41 100644 --- a/ElementX/Sources/Other/SwiftUI/Views/ToolbarButton.swift +++ b/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) { } + } + } + } + } +} diff --git a/ElementX/Sources/Other/TestablePreview/TestablePreviewsDictionary.swift b/ElementX/Sources/Other/TestablePreview/TestablePreviewsDictionary.swift index 5c328ccd7..12ff1eeca 100644 --- a/ElementX/Sources/Other/TestablePreview/TestablePreviewsDictionary.swift +++ b/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, diff --git a/ElementX/Sources/Screens/ManageAuthorizedSpacesScreen/ManageAuthorizedSpacesScreenModels.swift b/ElementX/Sources/Screens/ManageAuthorizedSpacesScreen/ManageAuthorizedSpacesScreenModels.swift index dc5c86c01..4124b673a 100644 --- a/ElementX/Sources/Screens/ManageAuthorizedSpacesScreen/ManageAuthorizedSpacesScreenModels.swift +++ b/ElementX/Sources/Screens/ManageAuthorizedSpacesScreen/ManageAuthorizedSpacesScreenModels.swift @@ -14,19 +14,19 @@ enum ManageAuthorizedSpacesScreenViewModelAction { struct ManageAuthorizedSpacesScreenViewState: BindableState { let authorizedSpacesSelection: AuthorizedSpacesSelection - var desiredSelectedIDs: Set + var selectedIDs: Set 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 - let desiredSelectIDs: PassthroughSubject, Never> = .init() + let initialSelectedIDs: Set + let selectedIDs: PassthroughSubject, Never> = .init() } diff --git a/ElementX/Sources/Screens/ManageAuthorizedSpacesScreen/ManageAuthorizedSpacesScreenViewModel.swift b/ElementX/Sources/Screens/ManageAuthorizedSpacesScreen/ManageAuthorizedSpacesScreenViewModel.swift index f24cec5a9..2a8bc81e9 100644 --- a/ElementX/Sources/Screens/ManageAuthorizedSpacesScreen/ManageAuthorizedSpacesScreenViewModel.swift +++ b/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) { - state.desiredSelectedIDs.remove(spaceID) + if state.selectedIDs.contains(spaceID) { + state.selectedIDs.remove(spaceID) } else { - state.desiredSelectedIDs.insert(spaceID) + state.selectedIDs.insert(spaceID) } } } diff --git a/ElementX/Sources/Screens/ManageAuthorizedSpacesScreen/View/ManageAuthorizedSpacesScreen.swift b/ElementX/Sources/Screens/ManageAuthorizedSpacesScreen/View/ManageAuthorizedSpacesScreen.swift index f274f53a7..e90f6b5e9 100644 --- a/ElementX/Sources/Screens/ManageAuthorizedSpacesScreen/View/ManageAuthorizedSpacesScreen.swift +++ b/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())) diff --git a/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenViewModel.swift b/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenViewModel.swift index 17acbb1cf..56d58cf5b 100644 --- a/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenViewModel.swift +++ b/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) - authorizedSpacesSelection.desiredSelectIDs + initialSelectedIDs: selectedIDs) + authorizedSpacesSelection.selectedIDs .sink { [weak self] desiredSelectedIDs in self?.state.bindings.desiredSettings.accessType = .spaceUsers(spaceIDs: desiredSelectedIDs.sorted()) } diff --git a/PreviewTests/Sources/GeneratedPreviewTests.swift b/PreviewTests/Sources/GeneratedPreviewTests.swift index ee222fe8a..3c67019e3 100644 --- a/PreviewTests/Sources/GeneratedPreviewTests.swift +++ b/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) diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/manageAuthorizedSpacesScreen.iPad-en-GB-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/manageAuthorizedSpacesScreen.iPad-en-GB-0.png index f377a66f0..63906d7dd 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/manageAuthorizedSpacesScreen.iPad-en-GB-0.png +++ b/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 -size 180070 +oid sha256:41b3fe504786e50cc23f43b4c5b3695af32fdbcc68981f2809b8c062ce7c12ec +size 180094 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/manageAuthorizedSpacesScreen.iPad-pseudo-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/manageAuthorizedSpacesScreen.iPad-pseudo-0.png index 31c91c54b..908d9bea5 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/manageAuthorizedSpacesScreen.iPad-pseudo-0.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/manageAuthorizedSpacesScreen.iPad-pseudo-0.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:9d4148bdd30d52750164b720a6e8b957cf1912842237729e0b3ef03a5d3d8f75 -size 205274 +oid sha256:40c3dce4e87b7021d3d50841071abe375c8e917abf755a9af3d2de4130ea029a +size 205332 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/toolbarButton.iPad-en-GB-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/toolbarButton.iPad-en-GB-0.png new file mode 100644 index 000000000..5ffdfd455 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/toolbarButton.iPad-en-GB-0.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:468ac87d2e665342d3073b804ef2921ee4468247609e5495fe37e355d5961729 +size 66647 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/toolbarButton.iPad-pseudo-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/toolbarButton.iPad-pseudo-0.png new file mode 100644 index 000000000..5ffdfd455 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/toolbarButton.iPad-pseudo-0.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:468ac87d2e665342d3073b804ef2921ee4468247609e5495fe37e355d5961729 +size 66647 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/toolbarButton.iPhone-en-GB-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/toolbarButton.iPhone-en-GB-0.png new file mode 100644 index 000000000..659f19ba8 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/toolbarButton.iPhone-en-GB-0.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:6b649e830e7ab43a1b8828a83886f65904e4efe808d07edc92aaf1b10fb2c065 +size 17355 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/toolbarButton.iPhone-pseudo-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/toolbarButton.iPhone-pseudo-0.png new file mode 100644 index 000000000..659f19ba8 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/toolbarButton.iPhone-pseudo-0.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:6b649e830e7ab43a1b8828a83886f65904e4efe808d07edc92aaf1b10fb2c065 +size 17355 diff --git a/UnitTests/Sources/SecurityAndPrivacyScreenViewModelTests.swift b/UnitTests/Sources/SecurityAndPrivacyScreenViewModelTests.swift index 27ffc22ad..74296ebd4 100644 --- a/UnitTests/Sources/SecurityAndPrivacyScreenViewModelTests.swift +++ b/UnitTests/Sources/SecurityAndPrivacyScreenViewModelTests.swift @@ -104,21 +104,21 @@ class SecurityAndPrivacyScreenViewModelTests: XCTestCase { return } - var desiredSpaceIDs: PassthroughSubject, Never>! + var selectedIDs: PassthroughSubject, 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, Never>! + var selectedIDs: PassthroughSubject, 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)