diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj index 0f38ac521..5d6bd3bfa 100644 --- a/ElementX.xcodeproj/project.pbxproj +++ b/ElementX.xcodeproj/project.pbxproj @@ -1067,6 +1067,7 @@ BDC4EB54CC3036730475CB8B /* QRCodeLoginScreenViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 25E7E9B7FEAB6169D960C206 /* QRCodeLoginScreenViewModelTests.swift */; }; BDED6DA7AD1E76018C424143 /* LegalInformationScreenViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C34667458773B02AB5FB0B2 /* LegalInformationScreenViewModel.swift */; }; BDFF0AEBF57B5B124062DAEF /* GeneratedAccessibilityTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F4CEB4590CCF70F0E3C0B171 /* GeneratedAccessibilityTests.swift */; }; + BE011C4473B9A8F12CBFE92A /* UserDetailsEditScreenViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61F58D94C936A1B731D78DE1 /* UserDetailsEditScreenViewModelTests.swift */; }; BE8075CA131C5EA3665C9E0D /* SpaceRoomProxyProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = A01F2E9FD8FF95AB89A345E6 /* SpaceRoomProxyProtocol.swift */; }; BE8E5985771DF9137C6CE89A /* ProcessInfo.swift in Sources */ = {isa = PBXBuildFile; fileRef = 077B01C13BBA2996272C5FB5 /* ProcessInfo.swift */; }; BEC6DFEA506085D3027E353C /* MediaEventsTimelineScreen.swift in Sources */ = {isa = PBXBuildFile; fileRef = 002399C6CB875C4EBB01CBC0 /* MediaEventsTimelineScreen.swift */; }; @@ -2031,6 +2032,7 @@ 604A69C081B935D6A38DE6D8 /* UserProfileScreenModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserProfileScreenModels.swift; sourceTree = ""; }; 60C9BAE9F9436B14E4E22E8F /* PinnedItemsBannerView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PinnedItemsBannerView.swift; sourceTree = ""; }; 61B33F23681660E940BA57F4 /* it */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = it; path = it.lproj/SAS.strings; sourceTree = ""; }; + 61F58D94C936A1B731D78DE1 /* UserDetailsEditScreenViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserDetailsEditScreenViewModelTests.swift; sourceTree = ""; }; 622D09D4ECE759189009AEAF /* MapLibreMapView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MapLibreMapView.swift; sourceTree = ""; }; 624244C398804ADC885239AA /* hu */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = hu; path = hu.lproj/Localizable.strings; sourceTree = ""; }; 627A8B5E798CC778C363655E /* KnockRequestsListEmptyStateView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KnockRequestsListEmptyStateView.swift; sourceTree = ""; }; @@ -4723,6 +4725,7 @@ 76310030C831D4610A705603 /* URLComponentsTests.swift */, 3AB34956C87731AB094DB33A /* URLTests.swift */, EB3B237387B8288A5A938F1B /* UserAgentBuilderTests.swift */, + 61F58D94C936A1B731D78DE1 /* UserDetailsEditScreenViewModelTests.swift */, 2429224EB0EEA34D35CE9249 /* UserIndicatorControllerTests.swift */, BA241DEEF7C8A7181C0AEDC9 /* UserPreferenceTests.swift */, 71E2E5103702D13361D09100 /* UserProfileScreenViewModelTests.swift */, @@ -7496,6 +7499,7 @@ 20C16A3F718802B0E4A19C83 /* URLComponentsTests.swift in Sources */, 49BBEC46D523BF6A41400048 /* URLTests.swift in Sources */, 8D3E1FADD78E72504DE0E402 /* UserAgentBuilderTests.swift in Sources */, + BE011C4473B9A8F12CBFE92A /* UserDetailsEditScreenViewModelTests.swift in Sources */, E313BDD2B8813144139B2E00 /* UserDiscoveryServiceTest.swift in Sources */, A1DF0E1E526A981ED6D5DF44 /* UserIndicatorControllerTests.swift in Sources */, 04F17DE71A50206336749BAC /* UserPreferenceTests.swift in Sources */, diff --git a/ElementX/Resources/Localizations/en-US.lproj/Localizable.strings b/ElementX/Resources/Localizations/en-US.lproj/Localizable.strings index 1795a9b7f..d3c5f528c 100644 --- a/ElementX/Resources/Localizations/en-US.lproj/Localizable.strings +++ b/ElementX/Resources/Localizations/en-US.lproj/Localizable.strings @@ -374,6 +374,7 @@ "dialog_default_video_quality_selector_subtitle" = "Select the default quality of videos you upload."; "dialog_title_confirmation" = "Confirmation"; "dialog_title_warning" = "Warning"; +"dialog_unsaved_changes_description" = "You have unsaved changes."; "dialog_unsaved_changes_description_ios" = "Your changes won’t be saved"; "dialog_unsaved_changes_title" = "Save changes?"; "dialog_video_quality_selector_subtitle_file_size" = "The max file size allowed is: %1$@"; @@ -1041,7 +1042,6 @@ "screen_room_change_role_moderators_owner_section_footer" = "Owners automatically have admin privileges."; "screen_room_change_role_moderators_title" = "Edit Moderators"; "screen_room_change_role_owners_title" = "Choose Owners"; -"screen_room_change_role_unsaved_changes_description" = "You have unsaved changes."; "screen_room_details_add_topic_title" = "Add topic"; "screen_room_details_badge_encrypted" = "Encrypted"; "screen_room_details_badge_not_encrypted" = "Not encrypted"; @@ -1425,6 +1425,7 @@ "screen_room_change_role_section_administrators" = "Admins"; "screen_room_change_role_section_moderators" = "Moderators"; "screen_room_change_role_section_users" = "Members"; +"screen_room_change_role_unsaved_changes_description" = "You have unsaved changes."; "screen_room_change_role_unsaved_changes_title" = "Save changes?"; "screen_room_details_badge_public" = "Public room"; "screen_room_details_invite_people_title" = "Invite people"; diff --git a/ElementX/Resources/Localizations/en.lproj/Localizable.strings b/ElementX/Resources/Localizations/en.lproj/Localizable.strings index 9e78b0149..01d9dedb7 100644 --- a/ElementX/Resources/Localizations/en.lproj/Localizable.strings +++ b/ElementX/Resources/Localizations/en.lproj/Localizable.strings @@ -374,6 +374,7 @@ "dialog_default_video_quality_selector_subtitle" = "Select the default quality of videos you upload."; "dialog_title_confirmation" = "Confirmation"; "dialog_title_warning" = "Warning"; +"dialog_unsaved_changes_description" = "You have unsaved changes."; "dialog_unsaved_changes_description_ios" = "Your changes won’t be saved"; "dialog_unsaved_changes_title" = "Save changes?"; "dialog_video_quality_selector_subtitle_file_size" = "The max file size allowed is: %1$@"; @@ -1041,7 +1042,6 @@ "screen_room_change_role_moderators_owner_section_footer" = "Owners automatically have admin privileges."; "screen_room_change_role_moderators_title" = "Edit Moderators"; "screen_room_change_role_owners_title" = "Choose Owners"; -"screen_room_change_role_unsaved_changes_description" = "You have unsaved changes."; "screen_room_details_add_topic_title" = "Add topic"; "screen_room_details_badge_encrypted" = "Encrypted"; "screen_room_details_badge_not_encrypted" = "Not encrypted"; @@ -1425,6 +1425,7 @@ "screen_room_change_role_section_administrators" = "Admins"; "screen_room_change_role_section_moderators" = "Moderators"; "screen_room_change_role_section_users" = "Members"; +"screen_room_change_role_unsaved_changes_description" = "You have unsaved changes."; "screen_room_change_role_unsaved_changes_title" = "Save changes?"; "screen_room_details_badge_public" = "Public room"; "screen_room_details_invite_people_title" = "Invite people"; diff --git a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift index 24f25afe0..d4a679f0e 100644 --- a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift @@ -1355,6 +1355,8 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { switch action { case .displayEditAddressScreen: presentEditAddressScreen() + case .dismiss: + navigationStackCoordinator.pop() } } .store(in: &cancellables) diff --git a/ElementX/Sources/FlowCoordinators/SettingsFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/SettingsFlowCoordinator.swift index b23d354cd..536287035 100644 --- a/ElementX/Sources/FlowCoordinators/SettingsFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/SettingsFlowCoordinator.swift @@ -155,6 +155,14 @@ class SettingsFlowCoordinator: FlowCoordinatorProtocol { navigationStackCoordinator: navigationStackCoordinator, userIndicatorController: flowParameters.userIndicatorController, appSettings: flowParameters.appSettings)) + coordinator.actions + .sink { [weak self] action in + switch action { + case .dismiss: + self?.navigationStackCoordinator.pop() + } + } + .store(in: &cancellables) navigationStackCoordinator.push(coordinator) } diff --git a/ElementX/Sources/FlowCoordinators/SpaceSettingsFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/SpaceSettingsFlowCoordinator.swift index a48abd006..3b4eac322 100644 --- a/ElementX/Sources/FlowCoordinators/SpaceSettingsFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/SpaceSettingsFlowCoordinator.swift @@ -256,6 +256,8 @@ final class SpaceSettingsFlowCoordinator: FlowCoordinatorProtocol { switch action { case .displayEditAddressScreen: self.stateMachine.tryEvent(.presentEditAddress) + case .dismiss: + navigationStackCoordinator.pop() } } .store(in: &cancellables) diff --git a/ElementX/Sources/Generated/Strings.swift b/ElementX/Sources/Generated/Strings.swift index 50f85f233..59240f1e7 100644 --- a/ElementX/Sources/Generated/Strings.swift +++ b/ElementX/Sources/Generated/Strings.swift @@ -854,6 +854,8 @@ internal enum L10n { internal static var dialogTitleSuccess: String { return L10n.tr("Localizable", "dialog_title_success") } /// Warning internal static var dialogTitleWarning: String { return L10n.tr("Localizable", "dialog_title_warning") } + /// You have unsaved changes. + internal static var dialogUnsavedChangesDescription: String { return L10n.tr("Localizable", "dialog_unsaved_changes_description") } /// Your changes won’t be saved internal static var dialogUnsavedChangesDescriptionIos: String { return L10n.tr("Localizable", "dialog_unsaved_changes_description_ios") } /// Save changes? diff --git a/ElementX/Sources/Mocks/ClientProxyMock.swift b/ElementX/Sources/Mocks/ClientProxyMock.swift index 5239ba2be..63d811347 100644 --- a/ElementX/Sources/Mocks/ClientProxyMock.swift +++ b/ElementX/Sources/Mocks/ClientProxyMock.swift @@ -82,9 +82,9 @@ extension ClientProxyMock { uploadMediaReturnValue = .failure(.sdkError(ClientProxyMockError.generic)) loadUserDisplayNameReturnValue = .failure(.sdkError(ClientProxyMockError.generic)) setUserDisplayNameReturnValue = .failure(.sdkError(ClientProxyMockError.generic)) - loadUserAvatarURLReturnValue = .failure(.sdkError(ClientProxyMockError.generic)) - setUserAvatarMediaReturnValue = .failure(.sdkError(ClientProxyMockError.generic)) - removeUserAvatarReturnValue = .failure(.sdkError(ClientProxyMockError.generic)) + loadUserAvatarURLReturnValue = .success(()) + setUserAvatarMediaReturnValue = .success(()) + removeUserAvatarReturnValue = .success(()) isAliasAvailableReturnValue = .success(true) searchUsersSearchTermLimitReturnValue = .success(.init(results: [], limited: false)) profileForReturnValue = .success(.init(userID: "@a:b.com", displayName: "Some user")) diff --git a/ElementX/Sources/Screens/RoomDetailsEditScreen/RoomDetailsEditScreenModels.swift b/ElementX/Sources/Screens/RoomDetailsEditScreen/RoomDetailsEditScreenModels.swift index e89de0d6d..a56623d63 100644 --- a/ElementX/Sources/Screens/RoomDetailsEditScreen/RoomDetailsEditScreenModels.swift +++ b/ElementX/Sources/Screens/RoomDetailsEditScreen/RoomDetailsEditScreenModels.swift @@ -15,12 +15,6 @@ enum RoomDetailsEditScreenViewModelAction { case displayMediaPicker } -struct RoomDetailsEditScreenViewStateBindings { - var name: String - var topic: String - var showMediaSheet = false -} - struct RoomDetailsEditScreenViewState: BindableState { let roomID: String let isSpace: Bool @@ -66,6 +60,18 @@ struct RoomDetailsEditScreenViewState: BindableState { } } +struct RoomDetailsEditScreenViewStateBindings { + var name: String + var topic: String + var showMediaSheet = false + + var alertInfo: AlertInfo? +} + +enum RoomDetailsEditScreenAlertType { + case unsavedChanges +} + enum RoomDetailsEditScreenViewAction { case cancel case save diff --git a/ElementX/Sources/Screens/RoomDetailsEditScreen/RoomDetailsEditScreenViewModel.swift b/ElementX/Sources/Screens/RoomDetailsEditScreen/RoomDetailsEditScreenViewModel.swift index f0fa51a7b..74b0a45b3 100644 --- a/ElementX/Sources/Screens/RoomDetailsEditScreen/RoomDetailsEditScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomDetailsEditScreen/RoomDetailsEditScreenViewModel.swift @@ -60,9 +60,9 @@ class RoomDetailsEditScreenViewModel: RoomDetailsEditScreenViewModelType, RoomDe override func process(viewAction: RoomDetailsEditScreenViewAction) { switch viewAction { case .cancel: - actionsSubject.send(.cancel) + cancel() case .save: - saveRoomDetails() + Task { await saveRoomDetails() } case .presentMediaSource: state.bindings.showMediaSheet = true case .displayCameraPicker: @@ -110,50 +110,60 @@ class RoomDetailsEditScreenViewModel: RoomDetailsEditScreenViewModelType, RoomDe } } - private func saveRoomDetails() { - Task { - let userIndicatorID = UUID().uuidString - defer { - userIndicatorController.retractIndicatorWithId(userIndicatorID) - } - userIndicatorController.submitIndicator(UserIndicator(id: userIndicatorID, - type: .modal(progress: .indeterminate, interactiveDismissDisabled: true, allowsInteraction: false), - title: L10n.screenRoomDetailsUpdatingRoom, - persistent: true)) - - do { - try await withThrowingTaskGroup(of: Void.self) { group in - if state.avatarDidChange { - group.addTask { - if let localMedia = await self.state.localMedia { - try await self.roomProxy.uploadAvatar(media: localMedia).get() - } else if await self.state.avatarURL == nil { - try await self.roomProxy.removeAvatar().get() - } + private func cancel() { + if state.canSave { + state.bindings.alertInfo = .init(id: .unsavedChanges, + title: L10n.dialogUnsavedChangesTitle, + message: L10n.dialogUnsavedChangesDescription, + primaryButton: .init(title: L10n.actionSave) { Task { await self.saveRoomDetails() } }, + secondaryButton: .init(title: L10n.actionDiscard, role: .cancel) { self.actionsSubject.send(.cancel) }) + } else { + actionsSubject.send(.cancel) + } + } + + private func saveRoomDetails() async { + let userIndicatorID = UUID().uuidString + defer { + userIndicatorController.retractIndicatorWithId(userIndicatorID) + } + userIndicatorController.submitIndicator(UserIndicator(id: userIndicatorID, + type: .modal(progress: .indeterminate, interactiveDismissDisabled: true, allowsInteraction: false), + title: L10n.screenRoomDetailsUpdatingRoom, + persistent: true)) + + do { + try await withThrowingTaskGroup(of: Void.self) { group in + if state.avatarDidChange { + group.addTask { + if let localMedia = await self.state.localMedia { + try await self.roomProxy.uploadAvatar(media: localMedia).get() + } else if await self.state.avatarURL == nil { + try await self.roomProxy.removeAvatar().get() } } - - if state.nameDidChange { - group.addTask { - try await self.roomProxy.setName(self.state.bindings.name).get() - } - } - - if state.topicDidChange { - group.addTask { - try await self.roomProxy.setTopic(self.state.bindings.topic).get() - } - } - - try await group.waitForAll() } - actionsSubject.send(.saveFinished) - } catch { - userIndicatorController.alertInfo = .init(id: .init(), - title: L10n.screenRoomDetailsEditionErrorTitle, - message: L10n.screenRoomDetailsEditionError) + if state.nameDidChange { + group.addTask { + try await self.roomProxy.setName(self.state.bindings.name).get() + } + } + + if state.topicDidChange { + group.addTask { + try await self.roomProxy.setTopic(self.state.bindings.topic).get() + } + } + + try await group.waitForAll() } + + actionsSubject.send(.saveFinished) + } catch { + userIndicatorController.alertInfo = .init(id: .init(), + title: L10n.screenRoomDetailsEditionErrorTitle, + message: L10n.screenRoomDetailsEditionError) } } } diff --git a/ElementX/Sources/Screens/RoomDetailsEditScreen/View/RoomDetailsEditScreen.swift b/ElementX/Sources/Screens/RoomDetailsEditScreen/View/RoomDetailsEditScreen.swift index 4c1b250c0..59581acb0 100644 --- a/ElementX/Sources/Screens/RoomDetailsEditScreen/View/RoomDetailsEditScreen.swift +++ b/ElementX/Sources/Screens/RoomDetailsEditScreen/View/RoomDetailsEditScreen.swift @@ -30,6 +30,7 @@ struct RoomDetailsEditScreen: View { .navigationBarTitleDisplayMode(.inline) .toolbar { toolbar } .track(screen: .RoomSettings) + .alert(item: $context.alertInfo) } // MARK: - Private diff --git a/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenCoordinator.swift b/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenCoordinator.swift index 414f592d7..01697746f 100644 --- a/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenCoordinator.swift +++ b/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenCoordinator.swift @@ -18,6 +18,7 @@ struct SecurityAndPrivacyScreenCoordinatorParameters { enum SecurityAndPrivacyScreenCoordinatorAction { case displayEditAddressScreen + case dismiss } final class SecurityAndPrivacyScreenCoordinator: CoordinatorProtocol { @@ -45,6 +46,8 @@ final class SecurityAndPrivacyScreenCoordinator: CoordinatorProtocol { switch action { case .displayEditAddressScreen: actionsSubject.send(.displayEditAddressScreen) + case .dismiss: + actionsSubject.send(.dismiss) } } .store(in: &cancellables) diff --git a/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenModels.swift b/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenModels.swift index 49be05d8e..fd4768cb4 100644 --- a/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenModels.swift +++ b/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenModels.swift @@ -10,6 +10,7 @@ import Foundation enum SecurityAndPrivacyScreenViewModelAction { case displayEditAddressScreen + case dismiss } struct SecurityAndPrivacyScreenViewState: BindableState { @@ -188,9 +189,11 @@ enum SecurityAndPrivacyRoomAccessType: Equatable { enum SecurityAndPrivacyAlertType { case enableEncryption + case unsavedChanges } enum SecurityAndPrivacyScreenViewAction { + case cancel case save case tryUpdatingEncryption(Bool) case editAddress diff --git a/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenViewModel.swift b/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenViewModel.swift index 27d2b3176..e0f2ca10e 100644 --- a/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenViewModel.swift +++ b/ElementX/Sources/Screens/SecurityAndPrivacyScreen/SecurityAndPrivacyScreenViewModel.swift @@ -62,10 +62,10 @@ class SecurityAndPrivacyScreenViewModel: SecurityAndPrivacyScreenViewModelType, MXLog.info("View model: received view action: \(viewAction)") switch viewAction { + case .cancel: + showUnsavedChangesAlert() // The cancel button is only shown when there are unsaved changes. case .save: - Task { - await saveDesiredSettings() - } + Task { await saveDesiredSettings() } case .tryUpdatingEncryption(let updatedValue): if updatedValue { state.bindings.alertInfo = .init(id: .enableEncryption, @@ -168,12 +168,19 @@ class SecurityAndPrivacyScreenViewModel: SecurityAndPrivacyScreenViewModelType, } } - private func saveDesiredSettings() async { + private func showUnsavedChangesAlert() { + state.bindings.alertInfo = .init(id: .unsavedChanges, + title: L10n.dialogUnsavedChangesTitle, + message: L10n.dialogUnsavedChangesDescription, + primaryButton: .init(title: L10n.actionSave) { Task { await self.saveDesiredSettings(shouldDismiss: true) } }, + secondaryButton: .init(title: L10n.actionDiscard, role: .cancel) { self.actionsSubject.send(.dismiss) }) + } + + private func saveDesiredSettings(shouldDismiss: Bool = false) async { showLoadingIndicator() + defer { hideLoadingIndicator() } - defer { - hideLoadingIndicator() - } + var hasFailures = false if state.currentSettings.isEncryptionEnabled != state.bindings.desiredSettings.isEncryptionEnabled { switch await roomProxy.enableEncryption() { @@ -181,6 +188,7 @@ class SecurityAndPrivacyScreenViewModel: SecurityAndPrivacyScreenViewModelType, state.currentSettings.isEncryptionEnabled = state.bindings.desiredSettings.isEncryptionEnabled case .failure: userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown)) + hasFailures = true } } @@ -190,6 +198,7 @@ class SecurityAndPrivacyScreenViewModel: SecurityAndPrivacyScreenViewModelType, state.currentSettings.historyVisibility = state.bindings.desiredSettings.historyVisibility case .failure: userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown)) + hasFailures = true } } @@ -205,6 +214,7 @@ class SecurityAndPrivacyScreenViewModel: SecurityAndPrivacyScreenViewModelType, state.currentSettings.accessType = state.bindings.desiredSettings.accessType case .failure: userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown)) + hasFailures = true } } @@ -216,8 +226,13 @@ class SecurityAndPrivacyScreenViewModel: SecurityAndPrivacyScreenViewModelType, state.currentSettings.isVisibileInRoomDirectory = state.bindings.desiredSettings.isVisibileInRoomDirectory case .failure: userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown)) + hasFailures = true } } + + if shouldDismiss, !hasFailures { + actionsSubject.send(.dismiss) + } } private func handleSelectedSpaceMembersAccess() { diff --git a/ElementX/Sources/Screens/SecurityAndPrivacyScreen/View/SecurityAndPrivacyScreen.swift b/ElementX/Sources/Screens/SecurityAndPrivacyScreen/View/SecurityAndPrivacyScreen.swift index a45399517..af47b395b 100644 --- a/ElementX/Sources/Screens/SecurityAndPrivacyScreen/View/SecurityAndPrivacyScreen.swift +++ b/ElementX/Sources/Screens/SecurityAndPrivacyScreen/View/SecurityAndPrivacyScreen.swift @@ -39,6 +39,7 @@ struct SecurityAndPrivacyScreen: View { .compoundList() .navigationBarTitleDisplayMode(.inline) .navigationTitle(L10n.screenSecurityAndPrivacyTitle) + .navigationBarBackButtonHidden(!context.viewState.isSaveDisabled) .toolbar { toolbar } .alert(item: $context.alertInfo) } @@ -185,6 +186,14 @@ struct SecurityAndPrivacyScreen: View { @ToolbarContentBuilder private var toolbar: some ToolbarContent { + ToolbarItem(placement: .cancellationAction) { + if !context.viewState.isSaveDisabled { + Button(L10n.actionCancel) { + context.send(viewAction: .cancel) + } + } + } + ToolbarItem(placement: .confirmationAction) { Button(L10n.actionSave) { context.send(viewAction: .save) diff --git a/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/UserDetailsEditScreenCoordinator.swift b/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/UserDetailsEditScreenCoordinator.swift index 9e52d6fbd..05ffec2aa 100644 --- a/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/UserDetailsEditScreenCoordinator.swift +++ b/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/UserDetailsEditScreenCoordinator.swift @@ -18,11 +18,20 @@ struct UserDetailsEditScreenCoordinatorParameters { let appSettings: AppSettings } +enum UserDetailsEditScreenCoordinatorAction { + case dismiss +} + final class UserDetailsEditScreenCoordinator: CoordinatorProtocol { private let parameters: UserDetailsEditScreenCoordinatorParameters private var viewModel: UserDetailsEditScreenViewModelProtocol private var cancellables = Set() + private let actionsSubject: PassthroughSubject = .init() + var actions: AnyPublisher { + actionsSubject.eraseToAnyPublisher() + } + init(parameters: UserDetailsEditScreenCoordinatorParameters) { self.parameters = parameters @@ -34,13 +43,17 @@ final class UserDetailsEditScreenCoordinator: CoordinatorProtocol { func start() { viewModel.actions .sink { [weak self] action in + guard let self else { return } + switch action { + case .dismiss: + actionsSubject.send(.dismiss) case .displayCameraPicker: - self?.displayMediaPickerWithMode(.init(source: .camera, selectionType: .single)) + displayMediaPickerWithMode(.init(source: .camera, selectionType: .single)) case .displayMediaPicker: - self?.displayMediaPickerWithMode(.init(source: .photoLibrary, selectionType: .single)) + displayMediaPickerWithMode(.init(source: .photoLibrary, selectionType: .single)) case .displayFilePicker: - self?.displayMediaPickerWithMode(.init(source: .documents, selectionType: .single)) + displayMediaPickerWithMode(.init(source: .documents, selectionType: .single)) } } .store(in: &cancellables) diff --git a/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/UserDetailsEditScreenModels.swift b/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/UserDetailsEditScreenModels.swift index fcb52437e..d920a15e2 100644 --- a/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/UserDetailsEditScreenModels.swift +++ b/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/UserDetailsEditScreenModels.swift @@ -9,6 +9,7 @@ import Foundation enum UserDetailsEditScreenViewModelAction { + case dismiss case displayCameraPicker case displayMediaPicker case displayFilePicker @@ -46,9 +47,16 @@ struct UserDetailsEditScreenViewState: BindableState { struct UserDetailsEditScreenViewStateBindings { var name = "" var showMediaSheet = false + + var alertInfo: AlertInfo? +} + +enum UserDetailsEditScreenAlertType { + case unsavedChanges } enum UserDetailsEditScreenViewAction { + case cancel case save case presentMediaSource case displayCameraPicker diff --git a/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/UserDetailsEditScreenViewModel.swift b/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/UserDetailsEditScreenViewModel.swift index 52c704043..1d76f45d9 100644 --- a/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/UserDetailsEditScreenViewModel.swift +++ b/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/UserDetailsEditScreenViewModel.swift @@ -65,8 +65,10 @@ class UserDetailsEditScreenViewModel: UserDetailsEditScreenViewModelType, UserDe override func process(viewAction: UserDetailsEditScreenViewAction) { switch viewAction { + case .cancel: + showUnsavedChangesAlert() // The cancel button is only shown when there are unsaved changes. case .save: - saveUserDetails() + Task { await saveUserDetails() } case .presentMediaSource: state.bindings.showMediaSheet = true case .displayCameraPicker: @@ -106,42 +108,52 @@ class UserDetailsEditScreenViewModel: UserDetailsEditScreenViewModelType, UserDe // MARK: - Private - private func saveUserDetails() { - Task { - let userIndicatorID = UUID().uuidString - defer { - userIndicatorController.retractIndicatorWithId(userIndicatorID) - } - userIndicatorController.submitIndicator(UserIndicator(id: userIndicatorID, - type: .modal(progress: .indeterminate, interactiveDismissDisabled: true, allowsInteraction: false), - title: L10n.screenEditProfileUpdatingDetails, - persistent: true)) - - do { - try await withThrowingTaskGroup(of: Void.self) { group in - if state.avatarDidChange { - group.addTask { - if let localMedia = await self.state.localMedia { - try await self.clientProxy.setUserAvatar(media: localMedia).get() - } else if await self.state.selectedAvatarURL == nil { - try await self.clientProxy.removeUserAvatar().get() - } + private func showUnsavedChangesAlert() { + state.bindings.alertInfo = .init(id: .unsavedChanges, + title: L10n.dialogUnsavedChangesTitle, + message: L10n.dialogUnsavedChangesDescription, + primaryButton: .init(title: L10n.actionSave) { Task { await self.saveUserDetails(shouldDismiss: true) } }, + secondaryButton: .init(title: L10n.actionDiscard, role: .cancel) { self.actionsSubject.send(.dismiss) }) + } + + private func saveUserDetails(shouldDismiss: Bool = false) async { + let userIndicatorID = UUID().uuidString + defer { + userIndicatorController.retractIndicatorWithId(userIndicatorID) + } + userIndicatorController.submitIndicator(UserIndicator(id: userIndicatorID, + type: .modal(progress: .indeterminate, interactiveDismissDisabled: true, allowsInteraction: false), + title: L10n.screenEditProfileUpdatingDetails, + persistent: true)) + + do { + try await withThrowingTaskGroup(of: Void.self) { group in + if state.avatarDidChange { + group.addTask { + if let localMedia = await self.state.localMedia { + try await self.clientProxy.setUserAvatar(media: localMedia).get() + } else if await self.state.selectedAvatarURL == nil { + try await self.clientProxy.removeUserAvatar().get() } } - - if state.nameDidChange { - group.addTask { - try await self.clientProxy.setUserDisplayName(self.state.bindings.name).get() - } - } - - try await group.waitForAll() } - } catch { - userIndicatorController.alertInfo = .init(id: .init(), - title: L10n.screenEditProfileErrorTitle, - message: L10n.screenEditProfileError) + + if state.nameDidChange { + group.addTask { + try await self.clientProxy.setUserDisplayName(self.state.bindings.name).get() + } + } + + try await group.waitForAll() } + + if shouldDismiss { + actionsSubject.send(.dismiss) + } + } catch { + userIndicatorController.alertInfo = .init(id: .init(), + title: L10n.screenEditProfileErrorTitle, + message: L10n.screenEditProfileError) } } } diff --git a/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/View/UserDetailsEditScreen.swift b/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/View/UserDetailsEditScreen.swift index f924d3568..22f4022e8 100644 --- a/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/View/UserDetailsEditScreen.swift +++ b/ElementX/Sources/Screens/Settings/UserDetailsEditScreen/View/UserDetailsEditScreen.swift @@ -31,13 +31,22 @@ struct UserDetailsEditScreen: View { .scrollDismissesKeyboard(.immediately) .navigationTitle(L10n.screenEditProfileTitle) .navigationBarTitleDisplayMode(.inline) + .navigationBarBackButtonHidden(context.viewState.canSave) .toolbar { toolbar } + .alert(item: $context.alertInfo) } // MARK: - Private @ToolbarContentBuilder private var toolbar: some ToolbarContent { + ToolbarItem(placement: .cancellationAction) { + if context.viewState.canSave { + Button(L10n.actionCancel) { + context.send(viewAction: .cancel) + } + } + } ToolbarItem(placement: .confirmationAction) { Button(L10n.actionSave) { context.send(viewAction: .save) diff --git a/UnitTests/Sources/RoomDetailsEditScreenViewModelTests.swift b/UnitTests/Sources/RoomDetailsEditScreenViewModelTests.swift index 52c2bfcce..eb762a2bf 100644 --- a/UnitTests/Sources/RoomDetailsEditScreenViewModelTests.swift +++ b/UnitTests/Sources/RoomDetailsEditScreenViewModelTests.swift @@ -71,7 +71,7 @@ class RoomDetailsEditScreenViewModelTests: XCTestCase { XCTAssertFalse(context.viewState.canSave) } - func testSaveShowsSheet() { + func testAvatarPickerShowsSheet() { setupViewModel(roomProxyConfiguration: .init(name: "Some room", members: [.mockMeAdmin])) context.name = "name" XCTAssertFalse(context.showMediaSheet) @@ -93,6 +93,47 @@ class RoomDetailsEditScreenViewModelTests: XCTestCase { XCTAssertEqual(action, .saveFinished) } + func testCancelWithoutChanges() async throws { + setupViewModel(roomProxyConfiguration: .init(name: "Some room", members: [.mockMeAdmin])) + XCTAssertFalse(context.viewState.canSave) + XCTAssertNil(context.alertInfo) + + var deferred = deferFulfillment(viewModel.actions) { $0 == .cancel } + context.send(viewAction: .cancel) + try await deferred.fulfill() + XCTAssertNil(context.alertInfo) + } + + func testCancelWithChangesAndDiscard() async throws { + setupViewModel(roomProxyConfiguration: .init(name: "Some room", members: [.mockMeAdmin])) + context.name = "name" + XCTAssertTrue(context.viewState.canSave) + XCTAssertNil(context.alertInfo) + + context.send(viewAction: .cancel) + + XCTAssertNotNil(context.alertInfo) + + let deferred = deferFulfillment(viewModel.actions) { $0 == .cancel } + context.alertInfo?.secondaryButton?.action?() // Discard + try await deferred.fulfill() + } + + func testCancelWithChangesAndSave() async throws { + setupViewModel(roomProxyConfiguration: .init(name: "Some room", members: [.mockMeAdmin])) + context.name = "name" + XCTAssertTrue(context.viewState.canSave) + XCTAssertNil(context.alertInfo) + + context.send(viewAction: .cancel) + + XCTAssertNotNil(context.alertInfo) + + let deferred = deferFulfillment(viewModel.actions) { $0 == .saveFinished } + context.alertInfo?.primaryButton.action?() // Save + try await deferred.fulfill() + } + func testErrorShownOnFailedFetchOfMedia() async throws { setupViewModel(roomProxyConfiguration: .init(name: "Some room", members: [.mockMeAdmin])) viewModel.didSelectMediaUrl(url: .picturesDirectory) diff --git a/UnitTests/Sources/SecurityAndPrivacyScreenViewModelTests.swift b/UnitTests/Sources/SecurityAndPrivacyScreenViewModelTests.swift index 063b37acb..8a69aa6d4 100644 --- a/UnitTests/Sources/SecurityAndPrivacyScreenViewModelTests.swift +++ b/UnitTests/Sources/SecurityAndPrivacyScreenViewModelTests.swift @@ -88,6 +88,67 @@ class SecurityAndPrivacyScreenViewModelTests: XCTestCase { } } + func testSave() async throws { + setupViewModel(isSpaceSettingsEnabled: false, joinedParentSpaces: [], joinRule: .public) + + // Saving shouldn't dismiss this screen (or trigger any other action). + let deferred = deferFailure(viewModel.actionsPublisher, timeout: 1) { _ in true } + + context.desiredSettings.accessType = .inviteOnly + context.send(viewAction: .save) + + try await deferred.fulfill() + } + + func testCancelWithChangesAndDiscard() async throws { + setupViewModel(isSpaceSettingsEnabled: false, joinedParentSpaces: [], joinRule: .public) + context.desiredSettings.accessType = .inviteOnly + XCTAssertFalse(context.viewState.isSaveDisabled) + XCTAssertNil(context.alertInfo) + + context.send(viewAction: .cancel) + + XCTAssertNotNil(context.alertInfo) + + let deferred = deferFulfillment(viewModel.actionsPublisher) { $0 == .dismiss } + context.alertInfo?.secondaryButton?.action?() // Discard + try await deferred.fulfill() + } + + func testCancelWithChangesAndSave() async throws { + setupViewModel(isSpaceSettingsEnabled: false, joinedParentSpaces: [], joinRule: .public) + context.desiredSettings.accessType = .inviteOnly + XCTAssertFalse(context.viewState.isSaveDisabled) + XCTAssertNil(context.alertInfo) + + context.send(viewAction: .cancel) + + XCTAssertNotNil(context.alertInfo) + + let deferred = deferFulfillment(viewModel.actionsPublisher) { $0 == .dismiss } + context.alertInfo?.primaryButton.action?() // Save + try await deferred.fulfill() + } + + func testCancelWithChangesAndSaveWithFailure() async throws { + setupViewModel(isSpaceSettingsEnabled: false, joinedParentSpaces: [], joinRule: .public) + roomProxy.updateJoinRuleReturnValue = .failure(.sdkError(RoomProxyMockError.generic)) + context.desiredSettings.accessType = .inviteOnly + XCTAssertFalse(context.viewState.isSaveDisabled) + XCTAssertNil(context.alertInfo) + + context.send(viewAction: .cancel) + + XCTAssertNotNil(context.alertInfo) + + // The screen should not be dismissed if a failure occurred. + let deferred = deferFailure(viewModel.actionsPublisher, timeout: 1) { _ in true } + context.alertInfo?.primaryButton.action?() // Save + try await deferred.fulfill() + } + + // MARK: - Helpers + private func setupViewModel(isSpaceSettingsEnabled: Bool, joinedParentSpaces: [SpaceRoomProxyProtocol], joinRule: JoinRule) { @@ -98,6 +159,7 @@ class SecurityAndPrivacyScreenViewModelTests: XCTestCase { members: .allMembersAsCreator, joinRule: joinRule, isVisibleInPublicDirectory: true)) + roomProxy.updateJoinRuleReturnValue = .success(()) viewModel = SecurityAndPrivacyScreenViewModel(roomProxy: roomProxy, clientProxy: ClientProxyMock(.init(userIDServerName: "matrix.org", diff --git a/UnitTests/Sources/UserDetailsEditScreenViewModelTests.swift b/UnitTests/Sources/UserDetailsEditScreenViewModelTests.swift new file mode 100644 index 000000000..32994b0eb --- /dev/null +++ b/UnitTests/Sources/UserDetailsEditScreenViewModelTests.swift @@ -0,0 +1,101 @@ +// +// Copyright 2025 Element Creations Ltd. +// Copyright 2022-2025 New Vector Ltd. +// +// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. +// Please see LICENSE files in the repository root for full details. +// + +import Combine +import XCTest + +@testable import ElementX + +@MainActor +class UserDetailsEditScreenViewModelTests: XCTestCase { + var viewModel: UserDetailsEditScreenViewModel! + + var userIndicatorController: UserIndicatorControllerMock! + + var context: UserDetailsEditScreenViewModelType.Context { + viewModel.context + } + + func testCannotSaveOnLanding() { + setupViewModel() + XCTAssertFalse(context.viewState.canSave) + } + + func testNameDidChange() { + setupViewModel() + context.name = "name" + XCTAssertTrue(context.viewState.nameDidChange) + XCTAssertTrue(context.viewState.canSave) + } + + func testEmptyNameCannotBeSaved() { + setupViewModel() + context.name = "" + XCTAssertFalse(context.viewState.canSave) + } + + func testAvatarPickerShowsSheet() { + setupViewModel() + context.name = "name" + XCTAssertFalse(context.showMediaSheet) + context.send(viewAction: .presentMediaSource) + XCTAssertTrue(context.showMediaSheet) + } + + func testSave() async throws { + setupViewModel() + + // Saving shouldn't dismiss this screen (or trigger any other action). + let deferred = deferFailure(viewModel.actions, timeout: 1) { _ in true } + + context.name = "name" + context.send(viewAction: .save) + + try await deferred.fulfill() + } + + func testCancelWithChangesAndDiscard() async throws { + setupViewModel() + context.name = "name" + XCTAssertTrue(context.viewState.canSave) + XCTAssertNil(context.alertInfo) + + context.send(viewAction: .cancel) + + XCTAssertNotNil(context.alertInfo) + + let deferred = deferFulfillment(viewModel.actions) { $0 == .dismiss } + context.alertInfo?.secondaryButton?.action?() // Discard + try await deferred.fulfill() + } + + func testCancelWithChangesAndSave() async throws { + setupViewModel() + context.name = "name" + XCTAssertTrue(context.viewState.canSave) + XCTAssertNil(context.alertInfo) + + context.send(viewAction: .cancel) + + XCTAssertNotNil(context.alertInfo) + + let deferred = deferFulfillment(viewModel.actions) { $0 == .dismiss } + context.alertInfo?.primaryButton.action?() // Save + try await deferred.fulfill() + } + + // MARK: - Private + + private func setupViewModel() { + userIndicatorController = UserIndicatorControllerMock.default + + viewModel = .init(userSession: UserSessionMock(.init()), + mediaUploadingPreprocessor: MediaUploadingPreprocessor(appSettings: ServiceLocator.shared.settings), + userIndicatorController: userIndicatorController) + } +}